feat(webhook): retry job on 404 (#179)

* feat(webhook): retry job on 404

* feat(webhook): enhance retry logic for 404 errors and update message status

* feat(webhook): update supported message events to use dynamic error handling

* chore: refactor logic

* feat(webhook): simplify RetriableError initialization and improve error handling for 404 responses

app/jobs/webhook_job.rb

@@ -1,5 +1,15 @@
 class WebhookJob < ApplicationJob
   queue_as :medium
+
+  retry_on CustomExceptions::Webhook::RetriableError, wait: :polynomially_longer, attempts: 5
+  discard_on CustomExceptions::Webhook::RetriableError do |job, error|
+    payload = job.arguments[1]
+    webhook_type = job.arguments[2] || :account_webhook
+
+    Rails.logger.warn "Webhook retries exhausted for #{payload[:event]}: #{error.message}"
+    Webhooks::ErrorHandler.perform(payload, webhook_type, error)
+  end
+
   #  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)

lib/custom_exceptions/webhook.rb

@@ -0,0 +1,11 @@
+module CustomExceptions::Webhook # rubocop:disable Style/ClassAndModuleChildren
+  class RetriableError < CustomExceptions::Base
+    def initialize(message)
+      super(message: message)
+    end
+
+    def message
+      @data[:message]
+    end
+  end
+end

lib/webhooks/error_handler.rb

@@ -0,0 +1,58 @@
+class Webhooks::ErrorHandler
+  SUPPORTED_EVENTS = %w[message_created message_updated].freeze
+
+  def initialize(payload, webhook_type, error)
+    @payload = payload
+    @webhook_type = webhook_type
+    @error = error
+  end
+
+  def self.perform(payload, webhook_type, error)
+    new(payload, webhook_type, error).perform
+  end
+
+  def perform
+    return unless SUPPORTED_EVENTS.include?(@payload[:event])
+    return unless message
+
+    case @webhook_type
+    when :agent_bot_webhook
+      handle_agent_bot_error
+    when :api_inbox_webhook
+      handle_api_inbox_error
+    end
+  end
+
+  private
+
+  def handle_agent_bot_error
+    conversation = message.conversation
+    return unless conversation&.pending?
+
+    conversation.open!
+    Conversations::ActivityMessageJob.perform_later(conversation, activity_message_params(conversation))
+  end
+
+  def handle_api_inbox_error
+    Messages::StatusUpdateService.new(message, 'failed', @error.message).perform
+  end
+
+  def activity_message_params(conversation)
+    {
+      account_id: conversation.account_id,
+      inbox_id: conversation.inbox_id,
+      message_type: :activity,
+      content: I18n.t('conversations.activity.agent_bot.error_moved_to_open')
+    }
+  end
+
+  def message
+    return if message_id.blank?
+
+    @message ||= Message.find_by(id: message_id)
+  end
+
+  def message_id
+    @payload[:id]
+  end
+end

lib/webhooks/trigger.rb

@@ -1,6 +1,4 @@
 class Webhooks::Trigger
-  SUPPORTED_ERROR_HANDLE_EVENTS = %w[message_created message_updated].freeze
-
   def initialize(url, payload, webhook_type)
     @url = url
     @payload = payload
@@ -13,8 +11,11 @@ class Webhooks::Trigger
 
   def execute
     perform_request
+  rescue RestClient::NotFound
+    Rails.logger.warn "Webhook returned 404: #{@url}"
+    raise CustomExceptions::Webhook::RetriableError, "Webhook endpoint not found: #{@url}"
   rescue StandardError => e
-    handle_error(e)
+    Webhooks::ErrorHandler.perform(@payload, @webhook_type, e)
     Rails.logger.warn "Exception: Invalid webhook URL #{@url} : #{e.message}"
   end
 
@@ -30,50 +31,6 @@ class Webhooks::Trigger
     )
   end
 
-  def handle_error(error)
-    return unless SUPPORTED_ERROR_HANDLE_EVENTS.include?(@payload[:event])
-    return unless message
-
-    case @webhook_type
-    when :agent_bot_webhook
-      conversation = message.conversation
-      return unless conversation&.pending?
-
-      conversation.open!
-      create_agent_bot_error_activity(conversation)
-    when :api_inbox_webhook
-      update_message_status(error)
-    end
-  end
-
-  def create_agent_bot_error_activity(conversation)
-    content = I18n.t('conversations.activity.agent_bot.error_moved_to_open')
-    Conversations::ActivityMessageJob.perform_later(conversation, activity_message_params(conversation, content))
-  end
-
-  def activity_message_params(conversation, content)
-    {
-      account_id: conversation.account_id,
-      inbox_id: conversation.inbox_id,
-      message_type: :activity,
-      content: content
-    }
-  end
-
-  def update_message_status(error)
-    Messages::StatusUpdateService.new(message, 'failed', error.message).perform
-  end
-
-  def message
-    return if message_id.blank?
-
-    @message ||= Message.find_by(id: message_id)
-  end
-
-  def message_id
-    @payload[:id]
-  end
-
   def webhook_timeout
     raw_timeout = GlobalConfig.get_value('WEBHOOK_TIMEOUT')
     timeout = raw_timeout.presence&.to_i

spec/jobs/webhook_job_spec.rb

@@ -28,4 +28,137 @@ RSpec.describe WebhookJob do
       perform_enqueued_jobs { job }
     end
   end
+
+  describe 'retry behavior for 404 errors' do
+    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(:payload) { { event: 'message_created', id: message.id } }
+    let(:webhook_type) { :api_inbox_webhook }
+
+    it 'is configured to retry on CustomExceptions::Webhook::RetriableError' do
+      retry_handler = described_class.rescue_handlers.find do |handler|
+        handler[0] == 'CustomExceptions::Webhook::RetriableError'
+      end
+
+      expect(retry_handler).to be_present
+    end
+
+    context 'when webhook type is api_inbox_webhook' do
+      let(:webhook_type) { :api_inbox_webhook }
+
+      it 'marks message as failed after retries are exhausted' do
+        allow(Webhooks::Trigger).to receive(:execute).and_raise(
+          CustomExceptions::Webhook::RetriableError.new('Webhook endpoint not found')
+        )
+
+        perform_enqueued_jobs { job }
+
+        expect(message.reload.status).to eq('failed')
+      end
+    end
+
+    context 'when webhook type is agent_bot_webhook' do
+      let(:webhook_type) { :agent_bot_webhook }
+
+      before { conversation.update!(status: :pending) }
+
+      it 'opens conversation and creates activity message after retries are exhausted' do
+        allow(Webhooks::Trigger).to receive(:execute).and_raise(
+          CustomExceptions::Webhook::RetriableError.new('Webhook endpoint not found')
+        )
+
+        perform_enqueued_jobs { job }
+
+        expect(conversation.reload.status).to eq('open')
+        activity_message = conversation.messages.where(message_type: :activity).last
+        expect(activity_message.content).to eq(I18n.t('conversations.activity.agent_bot.error_moved_to_open'))
+      end
+
+      it 'does not change message status' do
+        allow(Webhooks::Trigger).to receive(:execute).and_raise(
+          CustomExceptions::Webhook::RetriableError.new('Webhook endpoint not found')
+        )
+
+        expect { perform_enqueued_jobs { job } }.not_to(change { message.reload.status })
+      end
+
+      context 'when conversation is not pending' do
+        before { conversation.update!(status: :open) }
+
+        it 'does not create activity message' do
+          allow(Webhooks::Trigger).to receive(:execute).and_raise(
+            CustomExceptions::Webhook::RetriableError.new('Webhook endpoint not found')
+          )
+
+          expect(Conversations::ActivityMessageJob).not_to receive(:perform_later)
+
+          perform_enqueued_jobs { job }
+        end
+      end
+    end
+
+    context 'when webhook type is account_webhook' do
+      let(:webhook_type) { :account_webhook }
+
+      it 'does not update message status' do
+        allow(Webhooks::Trigger).to receive(:execute).and_raise(
+          CustomExceptions::Webhook::RetriableError.new('Webhook endpoint not found')
+        )
+
+        expect { perform_enqueued_jobs { job } }.not_to(change { message.reload.status })
+      end
+
+      it 'does not create activity message' do
+        allow(Webhooks::Trigger).to receive(:execute).and_raise(
+          CustomExceptions::Webhook::RetriableError.new('Webhook endpoint not found')
+        )
+
+        expect(Conversations::ActivityMessageJob).not_to receive(:perform_later)
+
+        perform_enqueued_jobs { job }
+      end
+    end
+
+    context 'when event is not message_created or message_updated' do
+      let(:payload) { { event: 'conversation_created', id: conversation.id } }
+
+      it 'does not update any message status' do
+        allow(Webhooks::Trigger).to receive(:execute).and_raise(
+          CustomExceptions::Webhook::RetriableError.new('Webhook endpoint not found')
+        )
+
+        expect(Messages::StatusUpdateService).not_to receive(:new)
+
+        perform_enqueued_jobs { job }
+      end
+    end
+
+    context 'when payload has no id' do
+      let(:payload) { { event: 'message_created' } }
+
+      it 'does not attempt to update message status' do
+        allow(Webhooks::Trigger).to receive(:execute).and_raise(
+          CustomExceptions::Webhook::RetriableError.new('Webhook endpoint not found')
+        )
+
+        expect(Messages::StatusUpdateService).not_to receive(:new)
+
+        perform_enqueued_jobs { job }
+      end
+    end
+
+    context 'when message does not exist' do
+      let(:payload) { { event: 'message_created', id: -1 } }
+
+      it 'handles gracefully without raising' do
+        allow(Webhooks::Trigger).to receive(:execute).and_raise(
+          CustomExceptions::Webhook::RetriableError.new('Webhook endpoint not found')
+        )
+
+        expect { perform_enqueued_jobs { job } }.not_to raise_error
+      end
+    end
+  end
 end

spec/lib/webhooks/trigger_spec.rb

@@ -174,4 +174,38 @@ describe Webhooks::Trigger do
       trigger.execute(url, payload, webhook_type)
     end
   end
+
+  context 'when webhook returns 404' do
+    it 'raises CustomExceptions::Webhook::RetriableError' do
+      payload = { hello: :hello }
+
+      expect(RestClient::Request).to receive(:execute)
+        .with(
+          method: :post,
+          url: url,
+          payload: payload.to_json,
+          headers: { content_type: :json, accept: :json },
+          timeout: webhook_timeout
+        ).and_raise(RestClient::NotFound.new)
+
+      expect { trigger.execute(url, payload, webhook_type) }.to raise_error(CustomExceptions::Webhook::RetriableError)
+    end
+
+    it 'does not call handle_error for 404 responses' 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: webhook_timeout
+        ).and_raise(RestClient::NotFound.new)
+
+      expect(Messages::StatusUpdateService).not_to receive(:new)
+      expect { trigger.execute(url, payload, webhook_type) }.to raise_error(CustomExceptions::Webhook::RetriableError)
+      expect(message.reload.status).to eq('sent')
+    end
+  end
 end