From 95d9c3b3d6828b8ce37591e68d4239ce8c18460b Mon Sep 17 00:00:00 2001
From: Alberto Almagro <albertoalmagro@gmail.com>
Date: Fri, 23 Nov 2018 20:31:14 +0100
Subject: Keep executions for each specific exception (#34352)

* Keep executions for each specific declaration

Fixes #34337

ActiveJob used the global executions counter to control the number of
times a job should be retried. The problem with this approach was that
in case a job raised different exceptions during its executions they
weren't retried the number of times defined by their `attemps` number.

**Example:**

Having the following job:
```ruby
class BuggyJob < ActiveJob::Base
  retry_on CustomException, attemps: 3
  retry_on OtherException, attempts: 3
end
```
If the job raised `CustomException` in the first two executions and then
it raised `OtherException`, the job wasn't retried anymore because the
global executions counter was already indicating 3 attempts.

With this patch each `retry_on` declaration has its specific counter so
that the first two executions that raise `CustomException` don't affect
the retries count that future exceptions may have.

* Revert "clarifies documentation around the attempts arugment to retry_on"

This reverts commit 86aa8f8c5631f77ed9a208e5107003c01512133e.
---
 activejob/test/cases/exceptions_test.rb | 22 ++++++++++++++++++++++
 activejob/test/jobs/retry_job.rb        |  9 +++++++++
 2 files changed, 31 insertions(+)

(limited to 'activejob/test')

diff --git a/activejob/test/cases/exceptions_test.rb b/activejob/test/cases/exceptions_test.rb
index 37bb65538a..b5328b8d6a 100644
--- a/activejob/test/cases/exceptions_test.rb
+++ b/activejob/test/cases/exceptions_test.rb
@@ -30,6 +30,28 @@ class ExceptionsTest < ActiveJob::TestCase
     end
   end
 
+  test "keeps the same attempts counter when several exceptions are listed in the same declaration" do
+    exceptions_to_raise = %w(FirstRetryableErrorOfTwo FirstRetryableErrorOfTwo FirstRetryableErrorOfTwo
+                             SecondRetryableErrorOfTwo SecondRetryableErrorOfTwo)
+
+    assert_raises SecondRetryableErrorOfTwo do
+      perform_enqueued_jobs do
+        ExceptionRetryJob.perform_later(exceptions_to_raise)
+      end
+    end
+  end
+
+  test "keeps a separate attempts counter for each individual declaration" do
+    exceptions_to_raise = %w(FirstRetryableErrorOfTwo FirstRetryableErrorOfTwo FirstRetryableErrorOfTwo
+                             DefaultsError DefaultsError)
+
+    assert_nothing_raised do
+      perform_enqueued_jobs do
+        ExceptionRetryJob.perform_later(exceptions_to_raise)
+      end
+    end
+  end
+
   test "failed retry job when exception kept occurring against defaults" do
     perform_enqueued_jobs do
       begin
diff --git a/activejob/test/jobs/retry_job.rb b/activejob/test/jobs/retry_job.rb
index 68dc17e16c..2d19d4c41e 100644
--- a/activejob/test/jobs/retry_job.rb
+++ b/activejob/test/jobs/retry_job.rb
@@ -39,3 +39,12 @@ class RetryJob < ActiveJob::Base
     end
   end
 end
+
+class ExceptionRetryJob < ActiveJob::Base
+  retry_on FirstRetryableErrorOfTwo, SecondRetryableErrorOfTwo, attempts: 4
+  retry_on DefaultsError
+
+  def perform(exceptions)
+    raise exceptions.shift.constantize.new unless exceptions.empty?
+  end
+end
-- 
cgit v1.2.3