feat(webhook): retry webhook job on any error (#180)
This commit is contained in:
parent
3c2d535de2
commit
8655e5b025
@ -11,12 +11,9 @@ class Webhooks::Trigger
|
|||||||
|
|
||||||
def execute
|
def execute
|
||||||
perform_request
|
perform_request
|
||||||
rescue RestClient::NotFound
|
|
||||||
Rails.logger.warn "Webhook returned 404: #{@url}"
|
|
||||||
raise CustomExceptions::Webhook::RetriableError, "Webhook endpoint not found: #{@url}"
|
|
||||||
rescue StandardError => e
|
rescue StandardError => e
|
||||||
Webhooks::ErrorHandler.perform(@payload, @webhook_type, e)
|
Rails.logger.warn "Webhook request failed for #{@url}: #{e.message}"
|
||||||
Rails.logger.warn "Exception: Invalid webhook URL #{@url} : #{e.message}"
|
raise CustomExceptions::Webhook::RetriableError, "Webhook request failed: #{e.message}"
|
||||||
end
|
end
|
||||||
|
|
||||||
private
|
private
|
||||||
|
|||||||
@ -41,7 +41,7 @@ describe Webhooks::Trigger do
|
|||||||
trigger.execute(url, payload, webhook_type)
|
trigger.execute(url, payload, webhook_type)
|
||||||
end
|
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 }
|
payload = { event: 'message_created', conversation: { id: conversation.id }, id: message.id }
|
||||||
|
|
||||||
expect(RestClient::Request).to receive(:execute)
|
expect(RestClient::Request).to receive(:execute)
|
||||||
@ -53,88 +53,18 @@ describe Webhooks::Trigger do
|
|||||||
timeout: webhook_timeout
|
timeout: webhook_timeout
|
||||||
).and_raise(RestClient::ExceptionWithResponse.new('error', 500)).once
|
).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
|
end
|
||||||
|
|
||||||
it 'updates message status if webhook fails for message-updated event' do
|
it 'does not call ErrorHandler directly (deferred to job discard)' do
|
||||||
payload = { event: 'message_updated', conversation: { id: conversation.id }, id: message.id }
|
payload = { event: 'message_created', conversation: { id: conversation.id }, id: message.id }
|
||||||
|
|
||||||
expect(RestClient::Request).to receive(:execute)
|
expect(RestClient::Request).to receive(:execute)
|
||||||
.with(
|
.and_raise(RestClient::ExceptionWithResponse.new('error', 500))
|
||||||
method: :post,
|
|
||||||
url: url,
|
expect(Webhooks::ErrorHandler).not_to receive(:perform)
|
||||||
payload: payload.to_json,
|
expect { trigger.execute(url, payload, webhook_type) }.to raise_error(CustomExceptions::Webhook::RetriableError)
|
||||||
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')
|
|
||||||
end
|
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
|
end
|
||||||
|
|
||||||
context 'when webhook timeout configuration is blank' do
|
context 'when webhook timeout configuration is blank' do
|
||||||
@ -174,38 +104,4 @@ describe Webhooks::Trigger do
|
|||||||
trigger.execute(url, payload, webhook_type)
|
trigger.execute(url, payload, webhook_type)
|
||||||
end
|
end
|
||||||
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
|
end
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user