Remove duplicate retry handling to due extra raise, fixes #3356

lib/sidekiq/job_retry.rb

@@ -70,30 +70,36 @@ module Sidekiq
     # require the worker to be instantiated.
     def global(msg, queue)
       yield
-    rescue Skip
-      raise
-    rescue Sidekiq::Shutdown
+    rescue Skip => ex
+      raise ex
+    rescue Sidekiq::Shutdown => ey
       # ignore, will be pushed back onto queue during hard_shutdown
-      raise
+      raise ey
     rescue Exception => e
       # ignore, will be pushed back onto queue during hard_shutdown
       raise Sidekiq::Shutdown if exception_caused_by_shutdown?(e)
 
       raise e unless msg['retry']
       attempt_retry(nil, msg, queue, e)
+      raise e
     end
 
 
     # The local retry support means that any errors that occur within
     # this block can be associated with the given worker instance.
     # This is required to support the `sidekiq_retries_exhausted` block.
+    #
+    # Note that any exception from the block is wrapped in the Skip
+    # exception so the global block does not reprocess the error.  The
+    # Skip exception is unwrapped within Sidekiq::Processor#process before
+    # calling the handle_exception handlers.
     def local(worker, msg, queue)
       yield
-    rescue Skip
-      raise
-    rescue Sidekiq::Shutdown
+    rescue Skip => ex
+      raise ex
+    rescue Sidekiq::Shutdown => ey
       # ignore, will be pushed back onto queue during hard_shutdown
-      raise
+      raise ey
     rescue Exception => e
       # ignore, will be pushed back onto queue during hard_shutdown
       raise Sidekiq::Shutdown if exception_caused_by_shutdown?(e)
@@ -161,8 +167,6 @@ module Sidekiq
         # Goodbye dear message, you (re)tried your best I'm sure.
         retries_exhausted(worker, msg, exception)
       end
-
-      raise exception
     end
 
     def retries_exhausted(worker, msg, exception)
@@ -217,8 +221,7 @@ module Sidekiq
     end
 
     def exception_caused_by_shutdown?(e, checked_causes = [])
-      # In Ruby 2.1.0 only, check if exception is a result of shutdown.
-      return false unless defined?(e.cause)
+      return false unless e.cause
 
       # Handle circular causes
       checked_causes << e.object_id

lib/sidekiq/processor.rb

@@ -179,8 +179,9 @@ module Sidekiq
         # we didn't properly finish it.
         ack = false
       rescue Exception => ex
-        handle_exception(ex, { :context => "Job raised exception", :job => job_hash, :jobstr => jobstr })
-        raise
+        e = ex.is_a?(::Sidekiq::JobRetry::Skip) && ex.cause ? ex.cause : ex
+        handle_exception(e, { :context => "Job raised exception", :job => job_hash, :jobstr => jobstr })
+        raise e
       ensure
         work.acknowledge if ack
       end

test/test_retry_exhausted.rb

@@ -102,6 +102,7 @@ class TestRetryExhausted < Sidekiq::Test
           raise 'kerblammo!'
         end
       end
+      raised_error = raised_error.cause
 
       assert new_worker.exhausted_called?
       assert_equal raised_error.message, new_worker.exhausted_job['error_message']
@@ -114,6 +115,7 @@ class TestRetryExhausted < Sidekiq::Test
           raise 'kerblammo!'
         end
       end
+      raised_error = raised_error.cause
 
       assert old_worker.exhausted_called?
       assert_equal raised_error.message, old_worker.exhausted_job['error_message']
@@ -138,6 +140,7 @@ class TestRetryExhausted < Sidekiq::Test
             raise 'kerblammo!'
           end
         end
+        raised_error = raised_error.cause
 
         assert exhausted_job
         assert_equal raised_error, exhausted_exception