From e49fb7921e94d533ed39c197cf0287120d389b28 Mon Sep 17 00:00:00 2001
From: "yuuji.yaginuma" <yuuji.yaginuma@gmail.com>
Date: Wed, 25 Jul 2018 17:30:38 +0900
Subject: [PATCH] Increment execution count before deserialize arguments

Currently, the execution count increments after deserializes arguments.
Therefore, if an error occurs with deserialize, it retries indefinitely.

In order to prevent this, the count is moved before deserialize.
Fixes #33344.
---
 activejob/lib/active_job/execution.rb   | 6 +++---
 activejob/test/cases/exceptions_test.rb | 8 ++++++++
 activejob/test/jobs/retry_job.rb        | 1 +
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/activejob/lib/active_job/execution.rb b/activejob/lib/active_job/execution.rb
index d75be376ec..f5a343311f 100644
--- a/activejob/lib/active_job/execution.rb
+++ b/activejob/lib/active_job/execution.rb
@@ -31,11 +31,11 @@ module ActiveJob
     #
     #   MyJob.new(*args).perform_now
     def perform_now
+      # Guard against jobs that were persisted before we started counting executions by zeroing out nil counters
+      self.executions = (executions || 0) + 1
+
       deserialize_arguments_if_needed
       run_callbacks :perform do
-        # Guard against jobs that were persisted before we started counting executions by zeroing out nil counters
-        self.executions = (executions || 0) + 1
-
         perform(*arguments)
       end
     rescue => exception
diff --git a/activejob/test/cases/exceptions_test.rb b/activejob/test/cases/exceptions_test.rb
index 47d4e3c0c2..37bb65538a 100644
--- a/activejob/test/cases/exceptions_test.rb
+++ b/activejob/test/cases/exceptions_test.rb
@@ -2,6 +2,7 @@
 
 require "helper"
 require "jobs/retry_job"
+require "models/person"
 
 class ExceptionsTest < ActiveJob::TestCase
   setup do
@@ -131,4 +132,11 @@ class ExceptionsTest < ActiveJob::TestCase
       assert_equal [ "Raised SecondDiscardableErrorOfTwo for the 1st time" ], JobBuffer.values
     end
   end
+
+  test "successfully retry job throwing DeserializationError" do
+    perform_enqueued_jobs do
+      RetryJob.perform_later Person.new(404), 5
+      assert_equal ["Raised ActiveJob::DeserializationError for the 5 time"], JobBuffer.values
+    end
+  end
 end
diff --git a/activejob/test/jobs/retry_job.rb b/activejob/test/jobs/retry_job.rb
index 1383fffd7d..68dc17e16c 100644
--- a/activejob/test/jobs/retry_job.rb
+++ b/activejob/test/jobs/retry_job.rb
@@ -24,6 +24,7 @@ class RetryJob < ActiveJob::Base
   retry_on ExponentialWaitTenAttemptsError, wait: :exponentially_longer, attempts: 10
   retry_on CustomWaitTenAttemptsError, wait: ->(executions) { executions * 2 }, attempts: 10
   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") }
 
   discard_on DiscardableError
   discard_on FirstDiscardableErrorOfTwo, SecondDiscardableErrorOfTwo