From 8655e5b0251f355c36e5f470beae145d8a1db1e0 Mon Sep 17 00:00:00 2001 From: Gabriel Jablonski Date: Fri, 2 Jan 2026 23:59:08 -0300 Subject: [PATCH] feat(webhook): retry webhook job on any error (#180) --- lib/webhooks/trigger.rb | 7 +- spec/lib/webhooks/trigger_spec.rb | 120 ++---------------------------- 2 files changed, 10 insertions(+), 117 deletions(-) diff --git a/lib/webhooks/trigger.rb b/lib/webhooks/trigger.rb index 3e2dac455..830aa7058 100644 --- a/lib/webhooks/trigger.rb +++ b/lib/webhooks/trigger.rb @@ -11,12 +11,9 @@ 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 - Webhooks::ErrorHandler.perform(@payload, @webhook_type, e) - Rails.logger.warn "Exception: Invalid webhook URL #{@url} : #{e.message}" + Rails.logger.warn "Webhook request failed for #{@url}: #{e.message}" + raise CustomExceptions::Webhook::RetriableError, "Webhook request failed: #{e.message}" end private diff --git a/spec/lib/webhooks/trigger_spec.rb b/spec/lib/webhooks/trigger_spec.rb index 8d345abfc..6627a7474 100644 --- a/spec/lib/webhooks/trigger_spec.rb +++ b/spec/lib/webhooks/trigger_spec.rb @@ -41,7 +41,7 @@ describe Webhooks::Trigger do trigger.execute(url, payload, webhook_type) end - it 'updates message status if webhook fails for message-created event' do + it 'raises RetriableError when webhook fails' do payload = { event: 'message_created', conversation: { id: conversation.id }, id: message.id } expect(RestClient::Request).to receive(:execute) @@ -53,88 +53,18 @@ describe Webhooks::Trigger do timeout: webhook_timeout ).and_raise(RestClient::ExceptionWithResponse.new('error', 500)).once - expect { trigger.execute(url, payload, webhook_type) }.to change { message.reload.status }.from('sent').to('failed') + expect { trigger.execute(url, payload, webhook_type) }.to raise_error(CustomExceptions::Webhook::RetriableError) end - it 'updates message status if webhook fails for message-updated event' do - payload = { event: 'message_updated', conversation: { id: conversation.id }, id: message.id } + it 'does not call ErrorHandler directly (deferred to job discard)' 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::ExceptionWithResponse.new('error', 500)).once - expect { trigger.execute(url, payload, webhook_type) }.to change { message.reload.status }.from('sent').to('failed') + .and_raise(RestClient::ExceptionWithResponse.new('error', 500)) + + expect(Webhooks::ErrorHandler).not_to receive(:perform) + expect { trigger.execute(url, payload, webhook_type) }.to raise_error(CustomExceptions::Webhook::RetriableError) end - - context 'when webhook type is agent bot' do - let(:webhook_type) { :agent_bot_webhook } - - it 'reopens conversation and enqueues activity message if pending' do - conversation.update!(status: :pending) - 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::ExceptionWithResponse.new('error', 500)).once - - expect do - perform_enqueued_jobs do - trigger.execute(url, payload, webhook_type) - end - end.not_to(change { message.reload.status }) - - expect(conversation.reload.status).to eq('open') - - activity_message = conversation.reload.messages.order(:created_at).last - expect(activity_message.message_type).to eq('activity') - expect(activity_message.content).to eq(agent_bot_error_content) - end - - it 'does not change message status or enqueue activity when conversation is not pending' 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::ExceptionWithResponse.new('error', 500)).once - - expect do - trigger.execute(url, payload, webhook_type) - end.not_to(change { message.reload.status }) - - expect(Conversations::ActivityMessageJob).not_to have_been_enqueued - - expect(conversation.reload.status).to eq('open') - end - 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: webhook_timeout - ).and_raise(RestClient::ExceptionWithResponse.new('error', 500)).once - - expect { trigger.execute(url, payload, webhook_type) }.not_to(change { message.reload.status }) end context 'when webhook timeout configuration is blank' do @@ -174,38 +104,4 @@ 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