From 901d2cca2bb410850c1a8dc7cc3b7509293b61b9 Mon Sep 17 00:00:00 2001 From: Jan Krutisch Date: Mon, 23 Oct 2023 12:44:20 +0200 Subject: [PATCH 1/7] WIP: Allow for exception tracking on async adapter. --- lib/honeybadger/plugins/active_job.rb | 49 +++++++++++++++++++ spec/fixtures/rails/config/application.rb | 3 ++ spec/fixtures/rails/config/queue_adapter.rb | 18 +++++++ .../rails/async_queue_adapter_spec.rb | 19 +++++++ 4 files changed, 89 insertions(+) create mode 100644 lib/honeybadger/plugins/active_job.rb create mode 100644 spec/fixtures/rails/config/queue_adapter.rb create mode 100644 spec/integration/rails/async_queue_adapter_spec.rb diff --git a/lib/honeybadger/plugins/active_job.rb b/lib/honeybadger/plugins/active_job.rb new file mode 100644 index 000000000..59bcb12ab --- /dev/null +++ b/lib/honeybadger/plugins/active_job.rb @@ -0,0 +1,49 @@ +module Honeybadger + module Plugins + module ActiveJob + + module Installer + def self.included(base) + pp "base #{base}" + base.send(:alias_method, :perform, :perform_without_honeybadger) + base.send(:alias_method, :perform_with_honeybadger, :perform) + end + + def perforn_with_honeybadger + perform_without_honeybadger + rescue => exception + pp "WE HERE" + Honeybadger.notify(e, parameters: { job_arguments: @job_data }, sync: true) + raise + end + + end + + + Plugin.register { + requirement { defined?(::Rails.application) && ::Rails.application } + requirement { + ::Rails.application.config.active_job[:queue_adapter] == :async # || ::Rails.application.config.active_job[:queue_adapter] == :inline || ::Rails.application.config.active_job[:queue_adapter] == :test + } + + execution { + adapter = ::Rails.application.config.active_job[:queue_adapter] + pp "ADAPTER: #{adapter.inspect}" + if adapter == :async + pp "HELLO" + begin + ::ActiveJob::Base.set_callback :execute, :around do |param, block| + pp param, block + end + rescue => e + pp e.stacktrace + end + pp "AFTER" + # ::ActiveJob::QueueAdapters::AsyncAdapter::JobWrapper.send(:include, Installer) + end + } + } + end + end + +end \ No newline at end of file diff --git a/spec/fixtures/rails/config/application.rb b/spec/fixtures/rails/config/application.rb index 27414d52f..aa4f772a1 100644 --- a/spec/fixtures/rails/config/application.rb +++ b/spec/fixtures/rails/config/application.rb @@ -33,6 +33,8 @@ class RailsApp < Rails::Application config.serve_static_files = false config.consider_all_requests_local = false + config.active_job.queue_adapter = :async + routes.append do get '/runtime_error', :to => 'rails#runtime_error' get '/record_not_found', :to => 'rails#record_not_found' @@ -62,3 +64,4 @@ def index Rails.logger = Logger.new(File::NULL) require_relative './breadcrumbs' +require_relative './queue_adapter' diff --git a/spec/fixtures/rails/config/queue_adapter.rb b/spec/fixtures/rails/config/queue_adapter.rb new file mode 100644 index 000000000..c8cf99e01 --- /dev/null +++ b/spec/fixtures/rails/config/queue_adapter.rb @@ -0,0 +1,18 @@ +class ErrorJob < ActiveJob::Base + def perform + pp "PERFORM #{::Rails.application.config.active_job}", caller + raise "exception raised in job" + end +end + +class ErrorJobController < ApplicationController + def enqueue_error_job + ErrorJob.perform_later + head 200 + end +end + +Rails.application.routes.append do + post "/enqueue_error_job", to: "error_job#enqueue_error_job" +end + diff --git a/spec/integration/rails/async_queue_adapter_spec.rb b/spec/integration/rails/async_queue_adapter_spec.rb new file mode 100644 index 000000000..d9dc342e1 --- /dev/null +++ b/spec/integration/rails/async_queue_adapter_spec.rb @@ -0,0 +1,19 @@ +require_relative '../rails_helper' + +describe "Rails Async Queue Adapter Test", if: RAILS_PRESENT, type: :request do + load_rails_hooks(self) + + it "reports exceptions" do + #include ActiveJob::TestHelper + + Honeybadger.flush do + post "/enqueue_error_job" + expect(response.status).to eq(200) + end + + # expect { perform_enqueued_jobs }.to raise_error(RuntimeError) + + expect(Honeybadger::Backend::Test.notifications[:notices].size).to eq(1) + end + +end From 065f5527efafe60f9b10469b5094c13552324d00 Mon Sep 17 00:00:00 2001 From: Jan Krutisch Date: Wed, 25 Oct 2023 09:06:21 +0200 Subject: [PATCH 2/7] Using perform_around callback to catch exceptions for ActiveJob --- lib/honeybadger/plugins/active_job.rb | 46 ++++++------------- spec/fixtures/rails/config/queue_adapter.rb | 5 +- .../rails/async_queue_adapter_spec.rb | 5 +- 3 files changed, 18 insertions(+), 38 deletions(-) diff --git a/lib/honeybadger/plugins/active_job.rb b/lib/honeybadger/plugins/active_job.rb index 59bcb12ab..624e85a53 100644 --- a/lib/honeybadger/plugins/active_job.rb +++ b/lib/honeybadger/plugins/active_job.rb @@ -1,46 +1,26 @@ module Honeybadger module Plugins module ActiveJob - - module Installer - def self.included(base) - pp "base #{base}" - base.send(:alias_method, :perform, :perform_without_honeybadger) - base.send(:alias_method, :perform_with_honeybadger, :perform) - end - - def perforn_with_honeybadger - perform_without_honeybadger - rescue => exception - pp "WE HERE" - Honeybadger.notify(e, parameters: { job_arguments: @job_data }, sync: true) - raise - end - - end - - + Plugin.register { requirement { defined?(::Rails.application) && ::Rails.application } requirement { - ::Rails.application.config.active_job[:queue_adapter] == :async # || ::Rails.application.config.active_job[:queue_adapter] == :inline || ::Rails.application.config.active_job[:queue_adapter] == :test + ::Rails.application.config.active_job[:queue_adapter] == :async } - + execution { - adapter = ::Rails.application.config.active_job[:queue_adapter] - pp "ADAPTER: #{adapter.inspect}" - if adapter == :async - pp "HELLO" - begin - ::ActiveJob::Base.set_callback :execute, :around do |param, block| - pp param, block + ::ActiveJob::Base.class_eval do |base| + base.set_callback :perform, :around do |param, block| + begin + Honeybadger.flush { + block.call + } + rescue => e + Honeybadger.notify(e, parameters: { job_arguments: self.arguments }, sync: true) + raise end - rescue => e - pp e.stacktrace end - pp "AFTER" - # ::ActiveJob::QueueAdapters::AsyncAdapter::JobWrapper.send(:include, Installer) - end + end } } end diff --git a/spec/fixtures/rails/config/queue_adapter.rb b/spec/fixtures/rails/config/queue_adapter.rb index c8cf99e01..54d4187c5 100644 --- a/spec/fixtures/rails/config/queue_adapter.rb +++ b/spec/fixtures/rails/config/queue_adapter.rb @@ -1,13 +1,12 @@ class ErrorJob < ActiveJob::Base - def perform - pp "PERFORM #{::Rails.application.config.active_job}", caller + def perform(opts={}) raise "exception raised in job" end end class ErrorJobController < ApplicationController def enqueue_error_job - ErrorJob.perform_later + ErrorJob.perform_later({some: "data"}) head 200 end end diff --git a/spec/integration/rails/async_queue_adapter_spec.rb b/spec/integration/rails/async_queue_adapter_spec.rb index d9dc342e1..bb8437747 100644 --- a/spec/integration/rails/async_queue_adapter_spec.rb +++ b/spec/integration/rails/async_queue_adapter_spec.rb @@ -4,16 +4,17 @@ load_rails_hooks(self) it "reports exceptions" do - #include ActiveJob::TestHelper + include ActiveJob::TestHelper Honeybadger.flush do post "/enqueue_error_job" expect(response.status).to eq(200) end - # expect { perform_enqueued_jobs }.to raise_error(RuntimeError) + expect { perform_enqueued_jobs }.to raise_error(RuntimeError) expect(Honeybadger::Backend::Test.notifications[:notices].size).to eq(1) + expect(Honeybadger::Backend::Test.notifications[:notices][0].params[:job_arguments][0]).to eq({some: 'data'}) end end From 899e4d72d4b0f77d513018852556a7e8f0434379 Mon Sep 17 00:00:00 2001 From: Jan Krutisch Date: Fri, 27 Oct 2023 13:06:20 +0200 Subject: [PATCH 3/7] Restructure test for older rails versions --- spec/integration/rails/async_queue_adapter_spec.rb | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/spec/integration/rails/async_queue_adapter_spec.rb b/spec/integration/rails/async_queue_adapter_spec.rb index bb8437747..1307d364a 100644 --- a/spec/integration/rails/async_queue_adapter_spec.rb +++ b/spec/integration/rails/async_queue_adapter_spec.rb @@ -1,18 +1,16 @@ require_relative '../rails_helper' describe "Rails Async Queue Adapter Test", if: RAILS_PRESENT, type: :request do + include ActiveJob::TestHelper load_rails_hooks(self) it "reports exceptions" do - include ActiveJob::TestHelper - Honeybadger.flush do - post "/enqueue_error_job" - expect(response.status).to eq(200) + perform_enqueued_jobs do + post "/enqueue_error_job" + end + expect(response.status).to eq(500) end - - expect { perform_enqueued_jobs }.to raise_error(RuntimeError) - expect(Honeybadger::Backend::Test.notifications[:notices].size).to eq(1) expect(Honeybadger::Backend::Test.notifications[:notices][0].params[:job_arguments][0]).to eq({some: 'data'}) end From 5aed4273b3d37eb6c9501d94d5ceb115a7cb99ff Mon Sep 17 00:00:00 2001 From: Jan Krutisch Date: Fri, 27 Oct 2023 14:52:33 +0200 Subject: [PATCH 4/7] Make sure ActiveJob::TestHelper is only included when Rails is there --- spec/integration/rails/async_queue_adapter_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/integration/rails/async_queue_adapter_spec.rb b/spec/integration/rails/async_queue_adapter_spec.rb index 1307d364a..5ad143150 100644 --- a/spec/integration/rails/async_queue_adapter_spec.rb +++ b/spec/integration/rails/async_queue_adapter_spec.rb @@ -1,7 +1,7 @@ require_relative '../rails_helper' describe "Rails Async Queue Adapter Test", if: RAILS_PRESENT, type: :request do - include ActiveJob::TestHelper + include ActiveJob::TestHelper if RAILS_PRESENT load_rails_hooks(self) it "reports exceptions" do From 99abc5d2e91983657155fbc21f83a9188d5311c6 Mon Sep 17 00:00:00 2001 From: Jan Krutisch Date: Fri, 27 Oct 2023 15:40:12 +0200 Subject: [PATCH 5/7] Test with around filter --- spec/fixtures/rails/config/queue_adapter.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/spec/fixtures/rails/config/queue_adapter.rb b/spec/fixtures/rails/config/queue_adapter.rb index 54d4187c5..017dfb40f 100644 --- a/spec/fixtures/rails/config/queue_adapter.rb +++ b/spec/fixtures/rails/config/queue_adapter.rb @@ -1,7 +1,13 @@ class ErrorJob < ActiveJob::Base + around_perform :around_for_testing + def perform(opts={}) raise "exception raised in job" end + + def around_for_testing(*args) + yield + end end class ErrorJobController < ApplicationController From 6683e0e3edbefa84f9bf040b83acb7ee4c9c7fb9 Mon Sep 17 00:00:00 2001 From: Jan Krutisch Date: Mon, 6 Nov 2023 16:40:48 +0100 Subject: [PATCH 6/7] Remove unnecessary test controller --- lib/honeybadger/plugins/active_job.rb | 10 ++++------ spec/fixtures/rails/config/queue_adapter.rb | 10 ---------- spec/integration/rails/async_queue_adapter_spec.rb | 6 ++++-- 3 files changed, 8 insertions(+), 18 deletions(-) diff --git a/lib/honeybadger/plugins/active_job.rb b/lib/honeybadger/plugins/active_job.rb index 624e85a53..9ac7a3032 100644 --- a/lib/honeybadger/plugins/active_job.rb +++ b/lib/honeybadger/plugins/active_job.rb @@ -12,12 +12,10 @@ module ActiveJob ::ActiveJob::Base.class_eval do |base| base.set_callback :perform, :around do |param, block| begin - Honeybadger.flush { - block.call - } - rescue => e - Honeybadger.notify(e, parameters: { job_arguments: self.arguments }, sync: true) - raise + block.call + rescue => error + Honeybadger.notify(error, parameters: { job_arguments: self.arguments }) + raise error end end end diff --git a/spec/fixtures/rails/config/queue_adapter.rb b/spec/fixtures/rails/config/queue_adapter.rb index 017dfb40f..7dd28929a 100644 --- a/spec/fixtures/rails/config/queue_adapter.rb +++ b/spec/fixtures/rails/config/queue_adapter.rb @@ -10,14 +10,4 @@ def around_for_testing(*args) end end -class ErrorJobController < ApplicationController - def enqueue_error_job - ErrorJob.perform_later({some: "data"}) - head 200 - end -end - -Rails.application.routes.append do - post "/enqueue_error_job", to: "error_job#enqueue_error_job" -end diff --git a/spec/integration/rails/async_queue_adapter_spec.rb b/spec/integration/rails/async_queue_adapter_spec.rb index 5ad143150..871f85683 100644 --- a/spec/integration/rails/async_queue_adapter_spec.rb +++ b/spec/integration/rails/async_queue_adapter_spec.rb @@ -7,10 +7,12 @@ it "reports exceptions" do Honeybadger.flush do perform_enqueued_jobs do - post "/enqueue_error_job" + expect { + ErrorJob.perform_later({some: 'data'}) + }.to raise_error(StandardError) end - expect(response.status).to eq(500) end + expect(Honeybadger::Backend::Test.notifications[:notices].size).to eq(1) expect(Honeybadger::Backend::Test.notifications[:notices][0].params[:job_arguments][0]).to eq({some: 'data'}) end From b36bb5f146e0bdd5f4d098a581369a4f7481184c Mon Sep 17 00:00:00 2001 From: Jan Krutisch Date: Mon, 27 Nov 2023 12:36:48 +0100 Subject: [PATCH 7/7] call Honeybadger.clear! before executing job --- lib/honeybadger/plugins/active_job.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/honeybadger/plugins/active_job.rb b/lib/honeybadger/plugins/active_job.rb index 9ac7a3032..b191423d3 100644 --- a/lib/honeybadger/plugins/active_job.rb +++ b/lib/honeybadger/plugins/active_job.rb @@ -11,6 +11,7 @@ module ActiveJob execution { ::ActiveJob::Base.class_eval do |base| base.set_callback :perform, :around do |param, block| + Honeybadger.clear! begin block.call rescue => error