fix: compatibility fixes for ActionMailer plugin (#1146)

Author: waltjones

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

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