From f8e91906179afa387cfc7bd8d432e058e46a169a Mon Sep 17 00:00:00 2001 From: Vinoth Kannan Date: Wed, 30 May 2018 16:57:40 +0530 Subject: [PATCH] FEATURE: Retry web hook when it is failed --- app/jobs/regular/emit_web_hook_event.rb | 15 +++++++++ config/locales/server.en.yml | 1 + config/site_settings.yml | 2 ++ spec/jobs/emit_web_hook_event_spec.rb | 42 +++++++++++++++++++++++++ 4 files changed, 60 insertions(+) diff --git a/app/jobs/regular/emit_web_hook_event.rb b/app/jobs/regular/emit_web_hook_event.rb index db90dd04019..742c3b7f2b3 100644 --- a/app/jobs/regular/emit_web_hook_event.rb +++ b/app/jobs/regular/emit_web_hook_event.rb @@ -3,6 +3,8 @@ require 'excon' module Jobs class EmitWebHookEvent < Jobs::Base PING_EVENT = 'ping'.freeze + MAX_RETRY_COUNT = 4.freeze + RETRY_BACKOFF = 5 def execute(args) %i{ @@ -12,6 +14,8 @@ module Jobs raise Discourse::InvalidParameters.new(key) unless args[key].present? end + @orig_args = args.dup + web_hook = WebHook.find_by(id: args[:web_hook_id]) raise Discourse::InvalidParameters.new(:web_hook_id) if web_hook.blank? @@ -113,6 +117,17 @@ module Jobs rescue web_hook_event.destroy! end + + retry_web_hook if response.status != 200 + end + + def retry_web_hook + if SiteSetting.retry_web_hook_events? + @orig_args[:retry_count] = (@orig_args[:retry_count] || 0) + 1 + return if @orig_args[:retry_count] > MAX_RETRY_COUNT + delay = RETRY_BACKOFF**(@orig_args[:retry_count] - 1) + Jobs.enqueue_in(delay.minutes, :emit_web_hook_event, @orig_args) + end end end end diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index d9b757d94ca..d893454430e 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1746,6 +1746,7 @@ en: default_categories_watching_first_post: "List of categories in which first post in each new topic will be watched by default." retain_web_hook_events_period_days: "Number of days to retain web hook event records." + retry_web_hook_events: "Automatically retry failed web hook events for 4 times. Time gaps between the retries are 1, 5, 25 and 125 minutes." allow_user_api_keys: "Allow generation of user API keys" allow_user_api_key_scopes: "List of scopes allowed for user API keys" diff --git a/config/site_settings.yml b/config/site_settings.yml index e808918656f..b3a480dd457 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1633,6 +1633,8 @@ user_preferences: api: retain_web_hook_events_period_days: default: 30 + retry_web_hook_events: + default: false user_api: allow_user_api_keys: diff --git a/spec/jobs/emit_web_hook_event_spec.rb b/spec/jobs/emit_web_hook_event_spec.rb index 97a86f66838..acc5961f3f4 100644 --- a/spec/jobs/emit_web_hook_event_spec.rb +++ b/spec/jobs/emit_web_hook_event_spec.rb @@ -24,6 +24,48 @@ describe Jobs::EmitWebHookEvent do end.to raise_error(Discourse::InvalidParameters) end + context 'when the web hook is failed' do + before do + SiteSetting.retry_web_hook_events = true + stub_request(:post, "https://meta.discourse.org/webhook_listener") + .to_return(body: 'Invalid Access', status: 403) + end + + it 'retry if site setting is enabled' do + expect do + subject.execute( + web_hook_id: post_hook.id, + event_type: described_class::PING_EVENT + ) + end.to change { Jobs::EmitWebHookEvent.jobs.size }.by(1) + + job = Jobs::EmitWebHookEvent.jobs.first + args = job["args"].first + expect(args["retry_count"]).to eq(1) + end + + it 'does not retry for more than maximum allowed times' do + expect do + subject.execute( + web_hook_id: post_hook.id, + event_type: described_class::PING_EVENT, + retry_count: described_class::MAX_RETRY_COUNT + ) + end.to_not change { Jobs::EmitWebHookEvent.jobs.size } + end + + it 'does not retry if site setting is disabled' do + SiteSetting.retry_web_hook_events = false + + expect do + subject.execute( + web_hook_id: post_hook.id, + event_type: described_class::PING_EVENT + ) + end.to change { Jobs::EmitWebHookEvent.jobs.size }.by(0) + end + end + it 'does not raise an error for a ping event without payload' do stub_request(:post, "https://meta.discourse.org/webhook_listener") .to_return(body: 'OK', status: 200)