diff --git a/app/jobs/webhook_job.rb b/app/jobs/webhook_job.rb index 61b5e8daf..57d3739b7 100644 --- a/app/jobs/webhook_job.rb +++ b/app/jobs/webhook_job.rb @@ -1,7 +1,7 @@ class WebhookJob < ApplicationJob queue_as :medium - - def perform(url, payload) - Webhooks::Trigger.execute(url, payload) + # There are 3 types of webhooks, account, inbox and agent_bot + def perform(url, payload, webhook_type = :account_webhook) + Webhooks::Trigger.execute(url, payload, webhook_type) end end diff --git a/app/listeners/webhook_listener.rb b/app/listeners/webhook_listener.rb index 45c04629f..3fdd823b8 100644 --- a/app/listeners/webhook_listener.rb +++ b/app/listeners/webhook_listener.rb @@ -97,7 +97,7 @@ class WebhookListener < BaseListener return unless inbox.channel_type == 'Channel::Api' return if inbox.channel.webhook_url.blank? - WebhookJob.perform_later(inbox.channel.webhook_url, payload) + WebhookJob.perform_later(inbox.channel.webhook_url, payload, :api_inbox_webhook) end def deliver_webhook_payloads(payload, inbox) diff --git a/lib/webhooks/trigger.rb b/lib/webhooks/trigger.rb index 228d34b7f..42ec29717 100644 --- a/lib/webhooks/trigger.rb +++ b/lib/webhooks/trigger.rb @@ -1,13 +1,57 @@ class Webhooks::Trigger - def self.execute(url, payload) - response = RestClient::Request.execute( + SUPPORTED_ERROR_HANDLE_EVENTS = %w[message_created message_updated].freeze + + def initialize(url, payload, webhook_type) + @url = url + @payload = payload + @webhook_type = webhook_type + end + + def self.execute(url, payload, webhook_type) + new(url, payload, webhook_type).execute + end + + def execute + perform_request + rescue RestClient::Exceptions::Timeout, RestClient::ExceptionWithResponse => e + handle_error(e) + Rails.logger.warn "Exception: Invalid webhook URL #{@url} : #{e.message}" + end + + private + + def perform_request + RestClient::Request.execute( method: :post, - url: url, payload: payload.to_json, + url: @url, + payload: @payload.to_json, headers: { content_type: :json, accept: :json }, timeout: 5 ) - Rails.logger.info "Performed Request: Code - #{response.code}" - rescue StandardError => e - Rails.logger.warn "Exception: invalid webhook url #{url} : #{e.message}" + end + + def handle_error(error) + return unless should_handle_error? + return unless message + + update_message_status(error) + end + + def should_handle_error? + @webhook_type == :api_inbox_webhook && SUPPORTED_ERROR_HANDLE_EVENTS.include?(@payload[:event]) + end + + def update_message_status(error) + message.update!(status: :failed, external_error: error.message) + end + + def message + return if message_id.blank? + + @message ||= Message.find_by(id: message_id) + end + + def message_id + @payload[:id] end end diff --git a/spec/jobs/agent_bots/webhook_job_spec.rb b/spec/jobs/agent_bots/webhook_job_spec.rb index f4c78c953..a8117d84e 100644 --- a/spec/jobs/agent_bots/webhook_job_spec.rb +++ b/spec/jobs/agent_bots/webhook_job_spec.rb @@ -3,19 +3,20 @@ require 'rails_helper' RSpec.describe AgentBots::WebhookJob do include ActiveJob::TestHelper - subject(:job) { described_class.perform_later(url, payload) } + subject(:job) { described_class.perform_later(url, payload, webhook_type) } let(:url) { 'https://test.com' } let(:payload) { { name: 'test' } } + let(:webhook_type) { :agent_bot_webhook } it 'queues the job' do expect { job }.to have_enqueued_job(described_class) - .with(url, payload) + .with(url, payload, webhook_type) .on_queue('high') end it 'executes perform' do - expect(Webhooks::Trigger).to receive(:execute).with(url, payload) + expect(Webhooks::Trigger).to receive(:execute).with(url, payload, webhook_type) perform_enqueued_jobs { job } end end diff --git a/spec/jobs/webhook_job_spec.rb b/spec/jobs/webhook_job_spec.rb index 23ddb6cf1..81802a3c0 100644 --- a/spec/jobs/webhook_job_spec.rb +++ b/spec/jobs/webhook_job_spec.rb @@ -3,19 +3,29 @@ require 'rails_helper' RSpec.describe WebhookJob do include ActiveJob::TestHelper - subject(:job) { described_class.perform_later(url, payload) } + subject(:job) { described_class.perform_later(url, payload, webhook_type) } let(:url) { 'https://test.chatwoot.com' } let(:payload) { { name: 'test' } } + let(:webhook_type) { :account_webhook } it 'queues the job' do expect { job }.to have_enqueued_job(described_class) - .with(url, payload) + .with(url, payload, webhook_type) .on_queue('medium') end - it 'executes perform' do - expect(Webhooks::Trigger).to receive(:execute).with(url, payload) + it 'executes perform with default webhook type' do + expect(Webhooks::Trigger).to receive(:execute).with(url, payload, webhook_type) perform_enqueued_jobs { job } end + + context 'with custom webhook type' do + let(:webhook_type) { :api_inbox_webhook } + + it 'executes perform with inbox webhook type' do + expect(Webhooks::Trigger).to receive(:execute).with(url, payload, webhook_type) + perform_enqueued_jobs { job } + end + end end diff --git a/spec/lib/webhooks/trigger_spec.rb b/spec/lib/webhooks/trigger_spec.rb index b5a2dcbd2..8ff2a21a5 100644 --- a/spec/lib/webhooks/trigger_spec.rb +++ b/spec/lib/webhooks/trigger_spec.rb @@ -3,10 +3,17 @@ require 'rails_helper' describe Webhooks::Trigger do subject(:trigger) { described_class } + let!(:account) { create(:account) } + let!(:inbox) { create(:inbox, account: account) } + let!(:conversation) { create(:conversation, inbox: inbox) } + let!(:message) { create(:message, account: account, inbox: inbox, conversation: conversation) } + + let!(:webhook_type) { :api_inbox_webhook } + let!(:url) { 'https://test.com' } + describe '#execute' do it 'triggers webhook' do payload = { hello: :hello } - url = 'https://test.com' expect(RestClient::Request).to receive(:execute) .with( @@ -16,7 +23,51 @@ describe Webhooks::Trigger do headers: { content_type: :json, accept: :json }, timeout: 5 ).once - trigger.execute(url, payload) + trigger.execute(url, payload, webhook_type) + end + + it 'updates message status if webhook fails for message-created event' do + payload = { event: 'message_created', conversation: { id: conversation.id }, id: message.id } + + expect(RestClient::Request).to receive(:execute) + .with( + method: :post, + url: url, + payload: payload.to_json, + headers: { content_type: :json, accept: :json }, + timeout: 5 + ).and_raise(RestClient::ExceptionWithResponse.new('error', 500)).once + + expect { trigger.execute(url, payload, webhook_type) }.to change { message.reload.status }.from('sent').to('failed') + end + + it 'updates message status if webhook fails for message-updated event' do + payload = { event: 'message_updated', conversation: { id: conversation.id }, id: message.id } + + expect(RestClient::Request).to receive(:execute) + .with( + method: :post, + url: url, + payload: payload.to_json, + headers: { content_type: :json, accept: :json }, + timeout: 5 + ).and_raise(RestClient::ExceptionWithResponse.new('error', 500)).once + expect { trigger.execute(url, payload, webhook_type) }.to change { message.reload.status }.from('sent').to('failed') end end + + it 'does not update message status if webhook fails for other events' do + payload = { event: 'conversation_created', conversation: { id: conversation.id }, id: message.id } + + expect(RestClient::Request).to receive(:execute) + .with( + method: :post, + url: url, + payload: payload.to_json, + headers: { content_type: :json, accept: :json }, + timeout: 5 + ).and_raise(RestClient::ExceptionWithResponse.new('error', 500)).once + + expect { trigger.execute(url, payload, webhook_type) }.not_to(change { message.reload.status }) + end end diff --git a/spec/listeners/webhook_listener_spec.rb b/spec/listeners/webhook_listener_spec.rb index 332cde731..736db07d0 100644 --- a/spec/listeners/webhook_listener_spec.rb +++ b/spec/listeners/webhook_listener_spec.rb @@ -54,7 +54,8 @@ describe WebhookListener do conversation: api_conversation ) api_event = Events::Base.new(event_name, Time.zone.now, message: api_message) - expect(WebhookJob).to receive(:perform_later).with(channel_api.webhook_url, api_message.webhook_data.merge(event: 'message_created')).once + expect(WebhookJob).to receive(:perform_later).with(channel_api.webhook_url, api_message.webhook_data.merge(event: 'message_created'), + :api_inbox_webhook).once listener.message_created(api_event) end @@ -101,7 +102,8 @@ describe WebhookListener do api_conversation = create(:conversation, account: account, inbox: api_inbox, assignee: user) api_event = Events::Base.new(event_name, Time.zone.now, conversation: api_conversation) expect(WebhookJob).to receive(:perform_later).with(channel_api.webhook_url, - api_conversation.webhook_data.merge(event: 'conversation_created')).once + api_conversation.webhook_data.merge(event: 'conversation_created'), + :api_inbox_webhook).once listener.conversation_created(api_event) end