Handle errors raised by the reloader (#3212)

The reloader might raise an error instead of yielding control. If this
happens, the job will silently fail to run, without being reenqueued and
without logging any indication of what happened.

This change doesn't prevent the job from being lost, but at least if the
error handlers are called it will be possible to diagnose the problem.

lib/sidekiq/processor.rb

@@ -119,9 +119,9 @@ module Sidekiq
       jobstr = work.job
       queue = work.queue_name
 
-      @reloader.call do
-        ack = false
-        begin
+      ack = false
+      begin
+        @reloader.call do
           job = Sidekiq.load_json(jobstr)
           klass  = job['class'.freeze].constantize
           worker = klass.new
@@ -137,17 +137,17 @@ module Sidekiq
             end
           end
           ack = true
-        rescue Sidekiq::Shutdown
-          # Had to force kill this job because it didn't finish
-          # within the timeout.  Don't acknowledge the work since
-          # we didn't properly finish it.
-          ack = false
-        rescue Exception => ex
-          handle_exception(ex, { :context => "Job raised exception", :job => job, :jobstr => jobstr })
-          raise
-        ensure
-          work.acknowledge if ack
         end
+      rescue Sidekiq::Shutdown
+        # Had to force kill this job because it didn't finish
+        # within the timeout.  Don't acknowledge the work since
+        # we didn't properly finish it.
+        ack = false
+      rescue Exception => ex
+        handle_exception(ex, { :context => "Job raised exception", :job => job, :jobstr => jobstr })
+        raise
+      ensure
+        work.acknowledge if ack
       end
     end
 

test/test_processor.rb

@@ -65,6 +65,40 @@ class TestProcessor < Sidekiq::Test
       assert_equal [['myarg']], msg['args']
     end
 
+    describe 'exception handling' do
+      let(:errors) { [] }
+      let(:error_handler) { proc { |ex, _| errors << ex } }
+
+      before do
+        Sidekiq.error_handlers << error_handler
+      end
+
+      after do
+        Sidekiq.error_handlers.pop
+      end
+
+      it 'handles exceptions raised by the job' do
+        msg = Sidekiq.dump_json({ 'class' => MockWorker.to_s, 'args' => ['boom'] })
+        begin
+          @processor.process(work(msg))
+        rescue TestException
+        end
+        assert_equal 1, errors.count
+        assert_instance_of TestException, errors.first
+      end
+
+      it 'handles exceptions raised by the reloader' do
+        msg = Sidekiq.dump_json({ 'class' => MockWorker.to_s, 'args' => ['myarg'] })
+        @processor.instance_variable_set(:'@reloader', proc { raise TEST_EXCEPTION })
+        begin
+          @processor.process(work(msg))
+        rescue TestException
+        end
+        assert_equal 1, errors.count
+        assert_instance_of TestException, errors.first
+      end
+    end
+
     describe 'acknowledgement' do
       class ExceptionRaisingMiddleware
         def initialize(raise_before_yield, raise_after_yield, skip)