feat: Mark the messages as failed if the API channel webhooks fail for any reason. (#8277)

app/jobs/webhook_job.rb

@@ -1,7 +1,7 @@
 class WebhookJob < ApplicationJob
   queue_as :medium
-
+  #  There are 3 types of webhooks, account, inbox and agent_bot
-  def perform(url, payload)
+  def perform(url, payload, webhook_type = :account_webhook)
-    Webhooks::Trigger.execute(url, payload)
+    Webhooks::Trigger.execute(url, payload, webhook_type)
   end
 end

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)

lib/webhooks/trigger.rb

@@ -1,13 +1,57 @@
 class Webhooks::Trigger
-  def self.execute(url, payload)
+  SUPPORTED_ERROR_HANDLE_EVENTS = %w[message_created message_updated].freeze
-    response = RestClient::Request.execute(
+
+  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}"
+  end
-  rescue StandardError => e
+
-    Rails.logger.warn "Exception: invalid webhook url #{url} : #{e.message}"
+  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

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

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
+  it 'executes perform with default webhook type' 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
+
+  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

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

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