Merge pull request #1813 from avalancheau/fix-activerecord-retry-on-restart

Improved (and more effective) fix for catching and ignoring exceptions caused by Sidekiq::Shutdown

lib/sidekiq/middleware/server/retry_jobs.rb

@@ -64,11 +64,8 @@ module Sidekiq
           # ignore, will be pushed back onto queue during hard_shutdown
           raise
         rescue Exception => e
-          # In Ruby 2.1.0 only, check if exception is a result of shutdown.
+          # ignore, will be pushed back onto queue during hard_shutdown
-          # If so, will be pushed back onto queue during hard_shutdown.
+          raise Sidekiq::Shutdown if exception_caused_by_shutdown?(e)
-          if defined?(e.cause) && e.cause.class == Sidekiq::Shutdown
-            raise Sidekiq::Shutdown
-          end
 
           raise e unless msg['retry']
           max_retry_attempts = retry_attempts_from(msg['retry'], @max_retries)
@@ -169,6 +166,14 @@ module Sidekiq
           end
         end
 
+        def exception_caused_by_shutdown?(e)
+          # In Ruby 2.1.0 only, check if exception is a result of shutdown.
+          return false unless defined?(e.cause)
+
+          e.cause.instance_of?(Sidekiq::Shutdown) ||
+            exception_caused_by_shutdown?(e.cause)
+        end
+
       end
     end
   end

test/test_retry.rb

@@ -117,6 +117,64 @@ class TestRetry < Sidekiq::Test
       @redis.verify
     end
 
+    it 'shuts down without retrying work-in-progress, which will resume' do
+      @redis.expect :zadd, 1, ['retry', String, String]
+      msg = { 'class' => 'Bob', 'args' => [1,2,'foo'], 'retry' => true }
+      handler = Sidekiq::Middleware::Server::RetryJobs.new
+      assert_raises Sidekiq::Shutdown do
+        handler.call(worker, msg, 'default') do
+          raise Sidekiq::Shutdown
+        end
+      end
+      assert_raises(MockExpectationError, "zadd should not be called") do
+        @redis.verify
+      end
+    end
+
+    it 'shuts down cleanly when shutdown causes exception' do
+      skip('Not supported in Ruby < 2.1.0') if RUBY_VERSION < '2.1.0'
+
+      @redis.expect :zadd, 1, ['retry', String, String]
+      msg = { 'class' => 'Bob', 'args' => [1,2,'foo'], 'retry' => true }
+      handler = Sidekiq::Middleware::Server::RetryJobs.new
+      assert_raises Sidekiq::Shutdown do
+        handler.call(worker, msg, 'default') do
+          begin
+            raise Sidekiq::Shutdown
+          rescue Interrupt
+            raise "kerblammo!"
+          end
+        end
+      end
+      assert_raises(MockExpectationError, "zadd should not be called") do
+        @redis.verify
+      end
+    end
+
+    it 'shuts down cleanly when shutdown causes chained exceptions' do
+      skip('Not supported in Ruby < 2.1.0') if RUBY_VERSION < '2.1.0'
+
+      @redis.expect :zadd, 1, ['retry', String, String]
+      msg = { 'class' => 'Bob', 'args' => [1,2,'foo'], 'retry' => true }
+      handler = Sidekiq::Middleware::Server::RetryJobs.new
+      assert_raises Sidekiq::Shutdown do
+        handler.call(worker, msg, 'default') do
+          begin
+            raise Sidekiq::Shutdown
+          rescue Interrupt
+            begin
+              raise "kerblammo!"
+            rescue
+              raise "kablooie!"
+            end
+          end
+        end
+      end
+      assert_raises(MockExpectationError, "zadd should not be called") do
+        @redis.verify
+      end
+    end
+
     it 'allows a retry queue' do
       @redis.expect :zadd, 1, ['retry', String, String]
       msg = { 'class' => 'Bob', 'args' => [1,2,'foo'], 'retry' => true, 'retry_queue' => 'retry' }