Merge pull request #36057 from jhawthorn/activejob_retry_logic

Use ActiveJob 5.2 retry logic for old jobs

2 files changed, 37 insertions, 5 deletions

diff --git a/activejob/lib/active_job/exceptions.rb b/activejob/lib/active_job/exceptions.rb
index 35c1476368..8e83246303 100644
--- a/activejob/lib/active_job/exceptions.rb
+++ b/activejob/lib/active_job/exceptions.rb
@@ -49,12 +49,10 @@ module ActiveJob
       #  end
       def retry_on(*exceptions, wait: 3.seconds, attempts: 5, queue: nil, priority: nil)
         rescue_from(*exceptions) do |error|
-          # Guard against jobs that were persisted before we started having individual executions counters per retry_on
-          self.exception_executions ||= {}
-          self.exception_executions[exceptions.to_s] = (exception_executions[exceptions.to_s] || 0) + 1
+          executions = executions_for(exceptions)
 
-          if exception_executions[exceptions.to_s] < attempts
-            retry_job wait: determine_delay(seconds_or_duration_or_algorithm: wait, executions: exception_executions[exceptions.to_s]), queue: queue, priority: priority, error: error
+          if executions < attempts
+            retry_job wait: determine_delay(seconds_or_duration_or_algorithm: wait, executions: executions), queue: queue, priority: priority, error: error
           else
             if block_given?
               instrument :retry_stopped, error: error do
@@ -146,5 +144,14 @@ module ActiveJob
 
         ActiveSupport::Notifications.instrument("#{name}.active_job", payload, &block)
       end
+
+      def executions_for(exceptions)
+        if exception_executions
+          exception_executions[exceptions.to_s] = (exception_executions[exceptions.to_s] || 0) + 1
+        else
+          # Guard against jobs that were persisted before we started having individual executions counters per retry_on
+          executions
+        end
+      end
   end
 end
diff --git a/activejob/test/cases/exceptions_test.rb b/activejob/test/cases/exceptions_test.rb
index 840f4d40b5..1f07b7b294 100644
--- a/activejob/test/cases/exceptions_test.rb
+++ b/activejob/test/cases/exceptions_test.rb
@@ -179,6 +179,31 @@ class ExceptionsTest < ActiveSupport::TestCase
     assert_equal ["Raised ActiveJob::DeserializationError for the 5 time"], JobBuffer.values
   end
 
+  test "running a job enqueued by AJ 5.2" do
+    job = RetryJob.new("DefaultsError", 6)
+    job.exception_executions = nil # This is how jobs from Rails 5.2 will look
+
+    assert_raises DefaultsError do
+      job.enqueue
+    end
+
+    assert_equal 5, JobBuffer.values.count
+  end
+
+  test "running a job enqueued and attempted under AJ 5.2" do
+    job = RetryJob.new("DefaultsError", 6)
+
+    # Fake 4 previous executions under AJ 5.2
+    job.exception_executions = nil
+    job.executions = 4
+
+    assert_raises DefaultsError do
+      job.enqueue
+    end
+
+    assert_equal ["Raised DefaultsError for the 5th time"], JobBuffer.values
+  end
+
   private
     def adapter_skips_scheduling?(queue_adapter)
       [