[active_job] patch retry_job to report all errors during retries

CHANGELOG.md

@@ -22,6 +22,7 @@
   - Fixes [#2478](https://github.com/getsentry/sentry-ruby/issues/2478)
 - Fix compatibility issues with sidekiq-cron 2.2.0 ([#2591](https://github.com/getsentry/sentry-ruby/pull/2591))
 - Update sentry-sidekiq to work correctly with Sidekiq 8.0 and its new timestamp format ([#2570](https://github.com/getsentry/sentry-ruby/pull/2570))
+- Ensure we capture exceptions after each job retry ([#2597](https://github.com/getsentry/sentry-ruby/pull/2597))
 
 ### Internal
 

sentry-rails/lib/sentry/rails/active_job.rb

@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require "set"
+
 module Sentry
   module Rails
     module ActiveJobExtensions
@@ -20,7 +22,11 @@ def already_supported_by_sentry_integration?
       class SentryReporter
         OP_NAME = "queue.active_job"
         SPAN_ORIGIN = "auto.queue.active_job"
-        NOTIFICATION_NAME = "retry_stopped.active_job"
+
+        EVENT_HANDLERS = {
+          "enqueue_retry.active_job" => :retry_handler,
+          "retry_stopped.active_job" => :retry_stopped_handler
+        }
 
         class << self
           def record(job, &block)
@@ -47,15 +53,19 @@ def record(job, &block)
               rescue Exception => e # rubocop:disable Lint/RescueException
                 finish_sentry_transaction(transaction, 500)
 
-                unless Sentry.configuration.rails.active_job_report_after_job_retries
-                  capture_exception(job, e)
-                end
+                maybe_capture_exception(job, e)
 
                 raise
               end
             end
           end
 
+          def maybe_capture_exception(job, e)
+            unless Sentry.configuration.rails.active_job_report_after_job_retries
+              capture_exception(job, e)
+            end
+          end
+
           def capture_exception(job, e)
             Sentry::Rails.capture_exception(
               e,
@@ -67,29 +77,47 @@ def capture_exception(job, e)
             )
           end
 
-          def register_retry_stopped_subscriber
-            unless @retry_stopped_subscriber
-              @retry_stopped_subscriber = ActiveSupport::Notifications.subscribe(NOTIFICATION_NAME) do |*args|
-                retry_stopped_handler(*args)
+          def register_event_handlers
+            EVENT_HANDLERS.each do |name, handler|
+              subscribers << ActiveSupport::Notifications.subscribe(name) do |*args|
+                public_send(handler, *args)
               end
             end
           end
 
-          def detach_retry_stopped_subscriber
-            if @retry_stopped_subscriber
-              ActiveSupport::Notifications.unsubscribe(@retry_stopped_subscriber)
-              @retry_stopped_subscriber = nil
+          def detach_event_handlers
+            subscribers.each do |subscriber|
+              ActiveSupport::Notifications.unsubscribe(subscriber)
+            end
+            subscribers.clear
+          end
+
+          # This handler does not capture error when `active_job_report_after_job_retries` is true
+          # because in such case only the last retry that failed will capture exception
+          def retry_handler(*args)
+            handle_error_event(*args) do |job, error|
+              return if !Sentry.initialized? || job.already_supported_by_sentry_integration?
+              return if Sentry.configuration.rails.active_job_report_after_job_retries
+
+              capture_exception(job, error)
             end
           end
 
+          # This handler does not capture error when `active_job_report_after_job_retries` is false
+          # because in such cases regular execution flow that failed will capture it in `record`
+          # method
           def retry_stopped_handler(*args)
-            event = ActiveSupport::Notifications::Event.new(*args)
-            job = event.payload[:job]
-            error = event.payload[:error]
+            handle_error_event(*args) do |job, error|
+              return if !Sentry.initialized? || job.already_supported_by_sentry_integration?
+              return unless Sentry.configuration.rails.active_job_report_after_job_retries
 
-            return if !Sentry.initialized? || job.already_supported_by_sentry_integration?
-            return unless Sentry.configuration.rails.active_job_report_after_job_retries
-            capture_exception(job, error)
+              capture_exception(job, error)
+            end
+          end
+
+          def handle_error_event(*args)
+            event = ActiveSupport::Notifications::Event.new(*args)
+            yield(event.payload[:job], event.payload[:error])
           end
 
           def finish_sentry_transaction(transaction, status)
@@ -128,6 +156,12 @@ def sentry_serialize_arguments(argument)
               argument
             end
           end
+
+          private
+
+          def subscribers
+            @__subscribers__ ||= Set.new
+          end
         end
       end
     end

sentry-rails/lib/sentry/rails/railtie.rb

@@ -53,7 +53,9 @@ class Railtie < ::Rails::Railtie
       register_error_subscriber(app) if ::Rails.version.to_f >= 7.0 && Sentry.configuration.rails.register_error_subscriber
 
       # Presence of ActiveJob is no longer a reliable cue
-      register_retry_stopped_subscriber if defined?(Sentry::Rails::ActiveJobExtensions)
+      if defined?(Sentry::Rails::ActiveJobExtensions)
+        Sentry::Rails::ActiveJobExtensions::SentryReporter.register_event_handlers
+      end
     end
 
     runner do
@@ -140,9 +142,5 @@ def register_error_subscriber(app)
       require "sentry/rails/error_subscriber"
       app.executor.error_reporter.subscribe(Sentry::Rails::ErrorSubscriber.new)
     end
-
-    def register_retry_stopped_subscriber
-      Sentry::Rails::ActiveJobExtensions::SentryReporter.register_retry_stopped_subscriber
-    end
   end
 end

sentry-rails/spec/sentry/rails/activejob_spec.rb

@@ -446,9 +446,11 @@ def perform(event, hint)
         allow(Sentry::Rails::ActiveJobExtensions::SentryReporter)
           .to receive(:capture_exception).and_call_original
 
-        assert_performed_jobs 3 do
-          FailedJobWithRetryOn.perform_later rescue nil
-        end
+        FailedJobWithRetryOn.perform_later rescue nil
+
+        perform_enqueued_jobs
+        perform_enqueued_jobs
+        perform_enqueued_jobs rescue nil
 
         expect(Sentry::Rails::ActiveJobExtensions::SentryReporter)
           .to have_received(:capture_exception)
@@ -469,9 +471,11 @@ def perform(event, hint)
         allow(Sentry::Rails::ActiveJobExtensions::SentryReporter)
           .to receive(:capture_exception).and_call_original
 
-        assert_performed_jobs 3 do
-          FailedJobWithRetryOn.perform_later rescue nil
-        end
+        FailedJobWithRetryOn.perform_later rescue nil
+
+        perform_enqueued_jobs
+        perform_enqueued_jobs
+        perform_enqueued_jobs rescue nil
 
         expect(Sentry::Rails::ActiveJobExtensions::SentryReporter)
           .to have_received(:capture_exception)

sentry-rails/spec/spec_helper.rb

@@ -55,7 +55,7 @@
     Sentry::Rails::Tracing.remove_active_support_notifications_patch
 
     if defined?(Sentry::Rails::ActiveJobExtensions)
-      Sentry::Rails::ActiveJobExtensions::SentryReporter.detach_retry_stopped_subscriber
+      Sentry::Rails::ActiveJobExtensions::SentryReporter.detach_event_handlers
     end
 
     reset_sentry_globals!