From b6b5a7ac524a8c22d9ca162e53d752141f974651 Mon Sep 17 00:00:00 2001
From: bogdanvlviv <bogdanvlviv@gmail.com>
Date: Sun, 16 Sep 2018 17:39:55 +0300
Subject: `retry_job` should publish `enqueue_retry.active_job` notification

Also this commit removes `:wait` from payload of
`retry_stopped.active_job`.

Related to https://github.com/rails/rails/pull/33751#discussion_r214140008
Follow up #33751

/cc @kaspth, @rafaelfranca
---
 activejob/lib/active_job/exceptions.rb | 28 +++++++++++++++++-----------
 activejob/lib/active_job/logging.rb    |  2 +-
 activejob/test/cases/logging_test.rb   |  5 +++++
 3 files changed, 23 insertions(+), 12 deletions(-)

(limited to 'activejob')

diff --git a/activejob/lib/active_job/exceptions.rb b/activejob/lib/active_job/exceptions.rb
index d8384c81b6..bb25afbca6 100644
--- a/activejob/lib/active_job/exceptions.rb
+++ b/activejob/lib/active_job/exceptions.rb
@@ -46,18 +46,15 @@ module ActiveJob
       #  end
       def retry_on(*exceptions, wait: 3.seconds, attempts: 5, queue: nil, priority: nil)
         rescue_from(*exceptions) do |error|
-          payload = {
-            job: self,
-            adapter: self.class.queue_adapter,
-            error: error,
-            wait: wait
-          }
-
           if executions < attempts
-            ActiveSupport::Notifications.instrument("enqueue_retry.active_job", payload) do
-              retry_job wait: determine_delay(wait), queue: queue, priority: priority
-            end
+            retry_job wait: determine_delay(wait), queue: queue, priority: priority, error: error
           else
+            payload = {
+              job: self,
+              adapter: self.class.queue_adapter,
+              error: error
+            }
+
             if block_given?
               ActiveSupport::Notifications.instrument("retry_stopped.active_job", payload) do
                 yield self, error
@@ -127,7 +124,16 @@ module ActiveJob
     #    end
     #  end
     def retry_job(options = {})
-      enqueue options
+      payload = {
+        job: self,
+        adapter: self.class.queue_adapter,
+        error: options[:error],
+        wait: options[:wait]
+      }
+
+      ActiveSupport::Notifications.instrument("enqueue_retry.active_job", payload) do
+        enqueue options
+      end
     end
 
     private
diff --git a/activejob/lib/active_job/logging.rb b/activejob/lib/active_job/logging.rb
index 96a3e6bf48..0abee4ed98 100644
--- a/activejob/lib/active_job/logging.rb
+++ b/activejob/lib/active_job/logging.rb
@@ -94,7 +94,7 @@ module ActiveJob
           wait = event.payload[:wait]
 
           error do
-            "Retrying #{job.class} in #{wait} seconds, due to a #{ex.class}. The original exception was #{ex.cause.inspect}."
+            "Retrying #{job.class} in #{wait.inspect} seconds, due to a #{ex&.class.inspect}. The original exception was #{ex&.cause.inspect}."
           end
         end
 
diff --git a/activejob/test/cases/logging_test.rb b/activejob/test/cases/logging_test.rb
index 2e8d2ef7c0..b5bf40c83b 100644
--- a/activejob/test/cases/logging_test.rb
+++ b/activejob/test/cases/logging_test.rb
@@ -173,6 +173,11 @@ class LoggingTest < ActiveSupport::TestCase
     end
   end
 
+  def test_enqueue_retry_logging_on_retry_job
+    perform_enqueued_jobs { RescueJob.perform_later "david" }
+    assert_match(/Retrying RescueJob in nil seconds, due to a nil\. The original exception was nil\./, @logger.messages)
+  end
+
   def test_retry_stopped_logging
     perform_enqueued_jobs do
       RetryJob.perform_later "CustomCatchError", 6
-- 
cgit v1.2.3