Merge pull request rails#54541 from Shopify/report-option-retry-on

Add `report:` option to `ActiveJob::Base#retry_on` and `#discard_on`

Author: byroot

activejob/CHANGELOG.md

@@ -1,3 +1,10 @@
+*   Add `report:` option to `ActiveJob::Base#retry_on` and `#discard_on`
+
+    When the `report:` option is passed, errors will be reported to the error reporter
+    before being retried / discarded.
+
+    *Andrew Novoselac*
+
 *   Accept a block for `ActiveJob::ConfiguredJob#perform_later`.
 
     This was inconsistent with a regular `ActiveJob::Base#perform_later`.

activejob/lib/active_job/exceptions.rb

@@ -34,6 +34,7 @@ module ClassMethods
       # * <tt>:queue</tt> - Re-enqueues the job on a different queue
       # * <tt>:priority</tt> - Re-enqueues the job with a different priority
       # * <tt>:jitter</tt> - A random delay of wait time used when calculating backoff. The default is 15% (0.15) which represents the upper bound of possible wait time (expressed as a percentage)
+      # * <tt>:report</tt> - Errors will be reported to the Rails.error reporter before being retried
       #
       # ==== Examples
       #
@@ -49,8 +50,9 @@ module ClassMethods
       #    # retry_on Net::ReadTimeout, wait: 5.seconds, jitter: 0.30, attempts: 10
       #    # retry_on Timeout::Error, wait: :polynomially_longer, attempts: 10
       #
-      #    retry_on(YetAnotherCustomAppException) do |job, error|
-      #      ExceptionNotifier.caught(error)
+      #    retry_on YetAnotherCustomAppException, report: true
+      #    retry_on EvenWorseCustomAppException do |job, error|
+      #      CustomErrorHandlingCode.handle(job, error)
       #    end
       #
       #    def perform(*args)
@@ -59,10 +61,11 @@ module ClassMethods
       #      # Might raise Net::OpenTimeout or Timeout::Error when the remote service is down
       #    end
       #  end
-      def retry_on(*exceptions, wait: 3.seconds, attempts: 5, queue: nil, priority: nil, jitter: JITTER_DEFAULT)
+      def retry_on(*exceptions, wait: 3.seconds, attempts: 5, queue: nil, priority: nil, jitter: JITTER_DEFAULT, report: false)
         rescue_from(*exceptions) do |error|
           executions = executions_for(exceptions)
           if attempts == :unlimited || executions < attempts
+            ActiveSupport.error_reporter.report(error, source: "application.active_job") if report
             retry_job wait: determine_delay(seconds_or_duration_or_algorithm: wait, executions: executions, jitter: jitter), queue: queue, priority: priority, error: error
           else
             if block_given?
@@ -82,6 +85,8 @@ def retry_on(*exceptions, wait: 3.seconds, attempts: 5, queue: nil, priority: ni
       # Discard the job with no attempts to retry, if the exception is raised. This is useful when the subject of the job,
       # like an Active Record, is no longer available, and the job is thus no longer relevant.
       #
+      # Passing the <tt>:report</tt> option reporter the error through the error reporter before discarding the job.
+      #
       # You can also pass a block that'll be invoked. This block is yielded with the job instance as the first and the error instance as the second parameter.
       #
       # +retry_on+ and +discard_on+ handlers are searched from bottom to top, and up the class hierarchy. The handler of the first class for
@@ -91,18 +96,20 @@ def retry_on(*exceptions, wait: 3.seconds, attempts: 5, queue: nil, priority: ni
       #
       #  class SearchIndexingJob < ActiveJob::Base
       #    discard_on ActiveJob::DeserializationError
-      #    discard_on(CustomAppException) do |job, error|
-      #      ExceptionNotifier.caught(error)
+      #    discard_on CustomAppException, report: true
+      #    discard_on(AnotherCustomAppException) do |job, error|
+      #      CustomErrorHandlingCode.handle(job, error)
       #    end
       #
       #    def perform(record)
       #      # Will raise ActiveJob::DeserializationError if the record can't be deserialized
       #      # Might raise CustomAppException for something domain specific
       #    end
       #  end
-      def discard_on(*exceptions)
+      def discard_on(*exceptions, report: true)
         rescue_from(*exceptions) do |error|
           instrument :discard, error: error do
+            ActiveSupport.error_reporter.report(error, source: "application.active_job") if report
             yield self, error if block_given?
             run_after_discard_procs(error)
           end

activejob/test/cases/exceptions_test.rb

@@ -343,6 +343,18 @@ def adapter_skips_scheduling?(queue_adapter)
       assert_equal ["Raised DefaultsError for the 5th time"], JobBuffer.values
     end
 
+    test "retrying a job reports error when report: true" do
+      assert_error_reported(ReportedError) do
+        RetryJob.perform_later("ReportedError", 2)
+      end
+    end
+
+    test "discarding a job reports error when report: true" do
+      assert_error_reported(AfterDiscardRetryJob::ReportedError) do
+        AfterDiscardRetryJob.perform_later("AfterDiscardRetryJob::ReportedError", 2)
+      end
+    end
+
     test "#after_discard block is run when an unhandled error is raised" do
       assert_raises(AfterDiscardRetryJob::UnhandledError) do
         AfterDiscardRetryJob.perform_later("AfterDiscardRetryJob::UnhandledError", 2)

activejob/test/jobs/after_discard_retry_job.rb

@@ -11,6 +11,7 @@ class DiscardableError < StandardError; end
   class CustomDiscardableError < StandardError; end
   class AfterDiscardError < StandardError; end
   class ChildAfterDiscardError < AfterDiscardError; end
+  class ReportedError < StandardError; end
 
   retry_on DefaultsError
   retry_on(CustomCatchError) { |job, error| JobBuffer.add("Dealt with a job that failed to retry in a custom way after #{job.arguments.second} attempts. Message: #{error.message}") }
@@ -19,6 +20,7 @@ class ChildAfterDiscardError < AfterDiscardError; end
 
   discard_on DiscardableError
   discard_on(CustomDiscardableError) { |_job, error| JobBuffer.add("Dealt with a job that was discarded in a custom way. Message: #{error.message}") }
+  discard_on(ReportedError, report: true)
 
   after_discard { |_job, error| JobBuffer.add("Ran after_discard for job. Message: #{error.message}") }
 

activejob/test/jobs/retry_job.rb

@@ -18,6 +18,7 @@ class FirstDiscardableErrorOfTwo < StandardError; end
 class SecondDiscardableErrorOfTwo < StandardError; end
 class CustomDiscardableError < StandardError; end
 class UnlimitedRetryError < StandardError; end
+class ReportedError < StandardError; end
 
 class RetryJob < ActiveJob::Base
   retry_on DefaultsError
@@ -31,6 +32,7 @@ class RetryJob < ActiveJob::Base
   retry_on(CustomCatchError) { |job, error| JobBuffer.add("Dealt with a job that failed to retry in a custom way after #{job.arguments.second} attempts. Message: #{error.message}") }
   retry_on(ActiveJob::DeserializationError) { |job, error| JobBuffer.add("Raised #{error.class} for the #{job.executions} time") }
   retry_on UnlimitedRetryError, attempts: :unlimited
+  retry_on ReportedError, report: true
 
   discard_on DiscardableError
   discard_on FirstDiscardableErrorOfTwo, SecondDiscardableErrorOfTwo