From 594674703c20c1ef7380af3c2a3aad18f3570667 Mon Sep 17 00:00:00 2001 From: Arpit Jalan Date: Mon, 15 Apr 2019 12:19:48 +0530 Subject: [PATCH] FIX: properly log webhook errors in UI on rescue (#7376) --- app/jobs/regular/emit_web_hook_event.rb | 8 ++++++-- app/models/web_hook_event.rb | 1 - spec/jobs/emit_web_hook_event_spec.rb | 14 ++++++++++++++ 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/app/jobs/regular/emit_web_hook_event.rb b/app/jobs/regular/emit_web_hook_event.rb index 64d680ddd53..cdba6da734a 100644 --- a/app/jobs/regular/emit_web_hook_event.rb +++ b/app/jobs/regular/emit_web_hook_event.rb @@ -113,8 +113,12 @@ module Jobs duration: ((Time.zone.now - now) * 1000).to_i ) rescue => e - web_hook_event.update!(headers: MultiJson.dump(headers)) - Rails.logger.error("Webhook event failed: #{e}") + web_hook_event.update!( + headers: MultiJson.dump(headers), + status: -1, + response_headers: MultiJson.dump(error: e), + duration: ((Time.zone.now - now) * 1000).to_i + ) end MessageBus.publish("/web_hook_events/#{web_hook.id}", { diff --git a/app/models/web_hook_event.rb b/app/models/web_hook_event.rb index 204efba8aaf..2ab764f161a 100644 --- a/app/models/web_hook_event.rb +++ b/app/models/web_hook_event.rb @@ -19,7 +19,6 @@ class WebHookEvent < ActiveRecord::Base else WebHook.last_delivery_statuses[:failed] end - web_hook.save! end end diff --git a/spec/jobs/emit_web_hook_event_spec.rb b/spec/jobs/emit_web_hook_event_spec.rb index 41de9f0f072..ceb3f84df0a 100644 --- a/spec/jobs/emit_web_hook_event_spec.rb +++ b/spec/jobs/emit_web_hook_event_spec.rb @@ -78,6 +78,20 @@ describe Jobs::EmitWebHookEvent do ) end.to change { Jobs::EmitWebHookEvent.jobs.size }.by(0) end + + it 'properly logs error on rescue' do + stub_request(:post, post_hook.payload_url).to_raise("connection error") + subject.execute( + web_hook_id: post_hook.id, + event_type: described_class::PING_EVENT + ) + + event = WebHookEvent.last + expect(event.payload).to eq(MultiJson.dump(ping: 'OK')) + expect(event.status).to eq(-1) + expect(MultiJson.load(event.response_headers)['error']).to eq('connection error') + end + end it 'does not raise an error for a ping event without payload' do