feat: Customizable webhook timeout configuration (#12777)
## Summary - Ability to configure the webhook timeout for Chatwoot self hosted installations fixes: https://github.com/chatwoot/chatwoot/issues/12754
This commit is contained in:
@@ -47,7 +47,7 @@ class SuperAdmin::AppConfigsController < SuperAdmin::ApplicationController
|
|||||||
|
|
||||||
@allowed_configs = mapping.fetch(
|
@allowed_configs = mapping.fetch(
|
||||||
@config,
|
@config,
|
||||||
%w[ENABLE_ACCOUNT_SIGNUP FIREBASE_PROJECT_ID FIREBASE_CREDENTIALS MAXIMUM_FILE_UPLOAD_SIZE]
|
%w[ENABLE_ACCOUNT_SIGNUP FIREBASE_PROJECT_ID FIREBASE_CREDENTIALS WEBHOOK_TIMEOUT MAXIMUM_FILE_UPLOAD_SIZE]
|
||||||
)
|
)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -79,6 +79,11 @@
|
|||||||
display_title: 'System events Webhook URL'
|
display_title: 'System events Webhook URL'
|
||||||
description: 'The URL to which the system events like new accounts created will be sent'
|
description: 'The URL to which the system events like new accounts created will be sent'
|
||||||
locked: false
|
locked: false
|
||||||
|
- name: WEBHOOK_TIMEOUT
|
||||||
|
value: 5
|
||||||
|
display_title: 'Webhook request timeout (seconds)'
|
||||||
|
description: 'Maximum time Chatwoot waits for a webhook response before failing the request'
|
||||||
|
locked: false
|
||||||
- name: DIRECT_UPLOADS_ENABLED
|
- name: DIRECT_UPLOADS_ENABLED
|
||||||
type: boolean
|
type: boolean
|
||||||
value: false
|
value: false
|
||||||
|
|||||||
@@ -26,7 +26,7 @@ class Webhooks::Trigger
|
|||||||
url: @url,
|
url: @url,
|
||||||
payload: @payload.to_json,
|
payload: @payload.to_json,
|
||||||
headers: { content_type: :json, accept: :json },
|
headers: { content_type: :json, accept: :json },
|
||||||
timeout: 5
|
timeout: webhook_timeout
|
||||||
)
|
)
|
||||||
end
|
end
|
||||||
|
|
||||||
@@ -73,4 +73,11 @@ class Webhooks::Trigger
|
|||||||
def message_id
|
def message_id
|
||||||
@payload[:id]
|
@payload[:id]
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def webhook_timeout
|
||||||
|
raw_timeout = GlobalConfig.get_value('WEBHOOK_TIMEOUT')
|
||||||
|
timeout = raw_timeout.presence&.to_i
|
||||||
|
|
||||||
|
timeout&.positive? ? timeout : 5
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -13,9 +13,12 @@ describe Webhooks::Trigger do
|
|||||||
let(:webhook_type) { :api_inbox_webhook }
|
let(:webhook_type) { :api_inbox_webhook }
|
||||||
let!(:url) { 'https://test.com' }
|
let!(:url) { 'https://test.com' }
|
||||||
let(:agent_bot_error_content) { I18n.t('conversations.activity.agent_bot.error_moved_to_open') }
|
let(:agent_bot_error_content) { I18n.t('conversations.activity.agent_bot.error_moved_to_open') }
|
||||||
|
let(:default_timeout) { 5 }
|
||||||
|
let(:webhook_timeout) { default_timeout }
|
||||||
|
|
||||||
before do
|
before do
|
||||||
ActiveJob::Base.queue_adapter = :test
|
ActiveJob::Base.queue_adapter = :test
|
||||||
|
allow(GlobalConfig).to receive(:get_value).with('WEBHOOK_TIMEOUT').and_return(webhook_timeout)
|
||||||
end
|
end
|
||||||
|
|
||||||
after do
|
after do
|
||||||
@@ -33,7 +36,7 @@ describe Webhooks::Trigger do
|
|||||||
url: url,
|
url: url,
|
||||||
payload: payload.to_json,
|
payload: payload.to_json,
|
||||||
headers: { content_type: :json, accept: :json },
|
headers: { content_type: :json, accept: :json },
|
||||||
timeout: 5
|
timeout: webhook_timeout
|
||||||
).once
|
).once
|
||||||
trigger.execute(url, payload, webhook_type)
|
trigger.execute(url, payload, webhook_type)
|
||||||
end
|
end
|
||||||
@@ -47,7 +50,7 @@ describe Webhooks::Trigger do
|
|||||||
url: url,
|
url: url,
|
||||||
payload: payload.to_json,
|
payload: payload.to_json,
|
||||||
headers: { content_type: :json, accept: :json },
|
headers: { content_type: :json, accept: :json },
|
||||||
timeout: 5
|
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 change { message.reload.status }.from('sent').to('failed')
|
||||||
@@ -62,7 +65,7 @@ describe Webhooks::Trigger do
|
|||||||
url: url,
|
url: url,
|
||||||
payload: payload.to_json,
|
payload: payload.to_json,
|
||||||
headers: { content_type: :json, accept: :json },
|
headers: { content_type: :json, accept: :json },
|
||||||
timeout: 5
|
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 change { message.reload.status }.from('sent').to('failed')
|
||||||
end
|
end
|
||||||
@@ -80,7 +83,7 @@ describe Webhooks::Trigger do
|
|||||||
url: url,
|
url: url,
|
||||||
payload: payload.to_json,
|
payload: payload.to_json,
|
||||||
headers: { content_type: :json, accept: :json },
|
headers: { content_type: :json, accept: :json },
|
||||||
timeout: 5
|
timeout: webhook_timeout
|
||||||
).and_raise(RestClient::ExceptionWithResponse.new('error', 500)).once
|
).and_raise(RestClient::ExceptionWithResponse.new('error', 500)).once
|
||||||
|
|
||||||
expect do
|
expect do
|
||||||
@@ -105,7 +108,7 @@ describe Webhooks::Trigger do
|
|||||||
url: url,
|
url: url,
|
||||||
payload: payload.to_json,
|
payload: payload.to_json,
|
||||||
headers: { content_type: :json, accept: :json },
|
headers: { content_type: :json, accept: :json },
|
||||||
timeout: 5
|
timeout: webhook_timeout
|
||||||
).and_raise(RestClient::ExceptionWithResponse.new('error', 500)).once
|
).and_raise(RestClient::ExceptionWithResponse.new('error', 500)).once
|
||||||
|
|
||||||
expect do
|
expect do
|
||||||
@@ -128,9 +131,47 @@ describe Webhooks::Trigger do
|
|||||||
url: url,
|
url: url,
|
||||||
payload: payload.to_json,
|
payload: payload.to_json,
|
||||||
headers: { content_type: :json, accept: :json },
|
headers: { content_type: :json, accept: :json },
|
||||||
timeout: 5
|
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) }.not_to(change { message.reload.status })
|
expect { trigger.execute(url, payload, webhook_type) }.not_to(change { message.reload.status })
|
||||||
end
|
end
|
||||||
|
|
||||||
|
context 'when webhook timeout configuration is blank' do
|
||||||
|
let(:webhook_timeout) { nil }
|
||||||
|
|
||||||
|
it 'falls back to default timeout' 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: default_timeout
|
||||||
|
).once
|
||||||
|
|
||||||
|
trigger.execute(url, payload, webhook_type)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when webhook timeout configuration is invalid' do
|
||||||
|
let(:webhook_timeout) { -1 }
|
||||||
|
|
||||||
|
it 'falls back to default timeout' 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: default_timeout
|
||||||
|
).once
|
||||||
|
|
||||||
|
trigger.execute(url, payload, webhook_type)
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
Reference in New Issue
Block a user