From 99cfa304b053950366bb5870e762c58d419332b3 Mon Sep 17 00:00:00 2001 From: Walt Jones Date: Wed, 10 Jan 2024 20:28:56 -0500 Subject: [PATCH] fix: compatibility fixes for ActionMailer plugin --- lib/rollbar/plugins/active_job.rb | 32 ++++++++----- spec/rollbar/plugins/active_job_spec.rb | 61 ++++++++++++++----------- 2 files changed, 56 insertions(+), 37 deletions(-) diff --git a/lib/rollbar/plugins/active_job.rb b/lib/rollbar/plugins/active_job.rb index a9dc3d7d..503ff103 100644 --- a/lib/rollbar/plugins/active_job.rb +++ b/lib/rollbar/plugins/active_job.rb @@ -8,22 +8,32 @@ module Rollbar module ActiveJob def self.included(base) base.send :rescue_from, Exception do |exception| - args = if self.class.respond_to?(:log_arguments?) && !self.class.log_arguments? - arguments.map(&Rollbar::Scrubbers.method(:scrub_value)) - else - arguments - end - job_data = { :job => self.class.name, - :use_exception_level_filters => true, - :arguments => args + :use_exception_level_filters => true } - # job_id isn't present when this is a mailer class. - job_data[:job_id] = job_id if defined?(job_id) + # When included in ActionMailer, the handler is called twice. + # This detects the execution that has the expected state. + if defined?(ActionMailer::Base) && self.class.ancestors.include?(ActionMailer::Base) + job_data[:action] = action_name + job_data[:params] = @params + + Rollbar.error(exception, job_data) + + # This detects other supported integrations. + elsif defined?(arguments) + job_data[:arguments] = \ + if self.class.respond_to?(:log_arguments?) && !self.class.log_arguments? + arguments.map(&Rollbar::Scrubbers.method(:scrub_value)) + else + arguments + end + job_data[:job_id] = job_id if defined?(job_id) + + Rollbar.error(exception, job_data) + end - Rollbar.error(exception, job_data) raise exception end end diff --git a/spec/rollbar/plugins/active_job_spec.rb b/spec/rollbar/plugins/active_job_spec.rb index 6e62a6f7..9595ca4e 100644 --- a/spec/rollbar/plugins/active_job_spec.rb +++ b/spec/rollbar/plugins/active_job_spec.rb @@ -22,15 +22,6 @@ def perform(exception, job_id) end end - class TestMailer < ActionMailer::Base - attr_accessor :arguments - - def test_email(*_arguments) - error = StandardError.new('oh no') - raise(error) - end - end - before { reconfigure_notifier } let(:exception) { StandardError.new('oh no') } @@ -52,19 +43,6 @@ def test_email(*_arguments) end.to raise_error exception end - it 'scrubs all arguments if job has `log_arguments` disabled' do - allow(TestJob).to receive(:log_arguments?).and_return(false) - - expected_params[:job_id] = job_id - expected_params[:arguments] = ['******', '******', '******'] - expect(Rollbar).to receive(:error).with(exception, expected_params) - begin - TestJob.new(1, 2, 3).perform(exception, job_id) - rescue StandardError - nil - end - end - it 'job is created' do ActiveJob::Base.queue_adapter = :test expect do @@ -88,6 +66,15 @@ def test_email(*_arguments) end context 'using ActionMailer::DeliveryJob', :if => Gem::Version.new(Rails.version) < Gem::Version.new('6.0') do + class TestMailer < ActionMailer::Base + attr_accessor :arguments + + def test_email(*_arguments) + error = StandardError.new('oh no') + raise(error) + end + end + include ActiveJob::TestHelper if defined?(ActiveJob::TestHelper) it_behaves_like 'an ActiveMailer plugin' @@ -118,6 +105,19 @@ def test_email(*_arguments) end end + it 'scrubs all arguments if job has `log_arguments` disabled' do + allow(TestJob).to receive(:log_arguments?).and_return(false) + + expected_params[:job_id] = job_id + expected_params[:arguments] = ['******', '******', '******'] + expect(Rollbar).to receive(:error).with(exception, expected_params) + begin + TestJob.new(1, 2, 3).perform(exception, job_id) + rescue StandardError + nil + end + end + it 'scrubs job arguments hash' do Rollbar.configure do |config| config.scrub_fields |= ['user_id'] @@ -155,12 +155,22 @@ def test_email(*_arguments) end context 'using ActionMailer::MailDeliveryJob', :if => Gem::Version.new(Rails.version) >= Gem::Version.new('6.0') do + class ParentMailer < ActionMailer::Base; end + + class TestMailer2 < ParentMailer + def test_email(*_arguments) + error = StandardError.new('oh no') + raise(error) + end + end + include ActiveJob::TestHelper if defined?(ActiveJob::TestHelper) let(:expected_params) do { :job => 'TestJob', - :use_exception_level_filters => true + :use_exception_level_filters => true, + :action => 'test_email' } end @@ -169,13 +179,12 @@ def test_email(*_arguments) it 'reports the error to Rollbar' do expected_params.delete(:job) - # In 6+, the re-raise in the plugin will cause the rescue_from to be called twice. - expect(Rollbar).to receive(:error).twice.with(kind_of(StandardError), + expect(Rollbar).to receive(:error).with(kind_of(StandardError), hash_including(expected_params)) perform_enqueued_jobs do begin - TestMailer.test_email(argument).deliver_later + TestMailer2.test_email(argument).deliver_later rescue StandardError => e nil end