support new sidekiq context structure

lib/rollbar/plugins/sidekiq/plugin.rb

@@ -12,16 +12,16 @@ def call(worker, msg, queue)
       end
     end
 
-    def self.handle_exception(msg_or_context, e)
-      return if skip_report?(msg_or_context, e)
+    def self.handle_exception(ctx_hash, e)
+      job_hash = ctx_hash && (ctx_hash[:job] || ctx_hash)
+      return if skip_report?(job_hash, e)
 
-      params = msg_or_context.reject{ |k| PARAM_BLACKLIST.include?(k) }
-      scrubbed_params = scrub_params(params)
       scope = {
-        :request => { :params => scrubbed_params },
         :framework => "Sidekiq: #{::Sidekiq::VERSION}"
       }
-      if params.is_a?(Hash)
+      unless job_hash.nil?
+        params = job_hash.reject { |k| PARAM_BLACKLIST.include?(k) }
+        scope[:request] = { :params => scrub_params(params) }
         scope[:context] = params['class']
         scope[:queue] = params['queue']
       end
@@ -38,9 +38,9 @@ def self.scrub_params(params)
       Rollbar::Scrubbers::Params.call(options)
     end
 
-    def self.skip_report?(msg_or_context, e)
-      msg_or_context.is_a?(Hash) && msg_or_context['retry'] &&
-        msg_or_context['retry_count'] && msg_or_context['retry_count'] < ::Rollbar.configuration.sidekiq_threshold
+    def self.skip_report?(job_hash, e)
+      !job_hash.nil? && (job_hash['retry'] && job_hash['retry_count'] &&
+        job_hash['retry_count'] < ::Rollbar.configuration.sidekiq_threshold)
     end
 
     def call(worker, msg, queue)

spec/rollbar/plugins/sidekiq_spec.rb

@@ -1,83 +1,111 @@
 require 'spec_helper'
 
-unless RUBY_VERSION == '1.8.7'
-  require 'sidekiq'
-end
+require 'sidekiq' unless RUBY_VERSION == '1.8.7'
 
 Rollbar.plugins.load!
 
 describe Rollbar::Sidekiq, :reconfigure_notifier => false do
   describe '.handle_exception' do
-    let(:msg_or_context) { ['hello', 'error_backtrace', 'backtrace', 'goodbye'] }
     let(:exception) { StandardError.new('oh noes') }
     let(:rollbar) { double }
-    let(:expected_args) do
+
+    let(:job_hash) do
       {
-        :request => { :params => ['hello', 'goodbye'] },
-        :framework => "Sidekiq: #{Sidekiq::VERSION}"
+        'class' => 'FooWorker',
+        'args' => %w(foo bar),
+        'queue' => 'default',
+        'jid' => '96aa59723946616dff537e97',
+        'enqueued_at' => Time.now.to_f,
+        'error_message' => exception.message,
+        'error_class' => exception.class,
+        'created_at' => Time.now.to_f,
+        'failed_at' => Time.now.to_f,
+        'retry' => 3,
+        'retry_count' => 0
       }
     end
 
-    subject { described_class }
+    let(:ctx_hash) do
+      { :context => 'Job raised exception', :job => job_hash }
+    end
+
+    let(:expected_scope) do
+      {
+        :request => {
+          :params => job_hash.reject { |k| described_class::PARAM_BLACKLIST.include?(k) }
+        },
+        :framework => "Sidekiq: #{Sidekiq::VERSION}",
+        :context => job_hash['class'],
+        :queue => job_hash['queue']
+      }
+    end
 
-    it 'constructs scope from filtered params' do
+    it 'constructs scope from ctx hash' do
       allow(rollbar).to receive(:error)
-      expect(Rollbar).to receive(:scope).with(expected_args) { rollbar }
+      expect(Rollbar).to receive(:scope).with(expected_scope) { rollbar }
+
+      described_class.handle_exception(ctx_hash, exception)
+    end
+
+    context 'sidekiq < 4.2.3 ctx hash' do
+      let(:ctx_hash) { job_hash }
+
+      it 'constructs scope from ctx hash' do
+        allow(rollbar).to receive(:error)
+        expect(Rollbar).to receive(:scope).with(expected_scope) { rollbar }
+
+        described_class.handle_exception(ctx_hash, exception)
+      end
+    end
+
+    context 'sidekiq < 4.0.0 nil ctx hash from Launcher#actor_died' do
+      let(:ctx_hash) { nil }
+
+      it 'constructs scope from ctx hash' do
+        allow(rollbar).to receive(:error)
+        expect(Rollbar).to receive(:scope).with(
+          :framework => "Sidekiq: #{Sidekiq::VERSION}"
+        ) { rollbar }
 
-      described_class.handle_exception(msg_or_context, exception)
+        described_class.handle_exception(ctx_hash, exception)
+      end
     end
 
     it 'sends the passed-in error to rollbar' do
       allow(Rollbar).to receive(:scope).and_return(rollbar)
       expect(rollbar).to receive(:error).with(exception, :use_exception_level_filters => true)
 
-      described_class.handle_exception(msg_or_context, exception)
+      described_class.handle_exception(ctx_hash, exception)
     end
 
-    context 'with fields in params to be scrubbed' do
-      let(:msg_or_context) do
-        {
-          :foo => 'bar',
-          :secret => 'foo',
-          :password => 'foo',
-          :password_confirmation => 'foo'
-        }
-      end
-      let(:expected_params) do
+    context 'with fields in job hash to be scrubbed' do
+      let(:ctx_hash) do
         {
-          :foo => 'bar',
-          :secret => /\*+/,
-          :password => /\*+/,
-          :password_confirmation => /\*+/
+          :context => 'Job raised exception',
+          :job => job_hash.merge(
+            'foo' => 'bar',
+            'secret' => 'foo',
+            'password' => 'foo',
+            'password_confirmation' => 'foo'
+          )
         }
       end
 
       before { reconfigure_notifier }
 
       it 'sends a report with the scrubbed fields' do
-        described_class.handle_exception(msg_or_context, exception)
-
-        expect(Rollbar.last_report[:request][:params]).to be_eql_hash_with_regexes(expected_params)
-      end
-    end
-
-    context 'when a sidekiq worker class is set' do
-      it 'adds the sidekiq#queue-name to the error report context' do
-        msg_or_context = {"retry" => true, "retry_count" => 1, 'queue' => 'default', 'class' => 'MyWorkerClass'}
-        expected_args = {
-          :request => { :params => msg_or_context },
-          :framework => "Sidekiq: #{Sidekiq::VERSION}",
-          :context => 'MyWorkerClass',
-          :queue => 'default'
-        }
-
-        allow(rollbar).to receive(:error)
-        allow(Rollbar).to receive(:scope).with(expected_args).and_return(rollbar)
-        described_class.handle_exception(msg_or_context, exception)
+        described_class.handle_exception(ctx_hash, exception)
+
+        expect(Rollbar.last_report[:request][:params]).to be_eql_hash_with_regexes(
+          'foo' => 'bar',
+          'secret' => /\*+/,
+          'password' => /\*+/,
+          'password_confirmation' => /\*+/
+        )
       end
     end
 
-    context 'when set a sidekiq_threshold' do
+    context 'with a sidekiq_threshold set' do
       before do
         Rollbar.configuration.sidekiq_threshold = 2
       end
@@ -86,38 +114,42 @@
         allow(Rollbar).to receive(:scope).and_return(rollbar)
         expect(rollbar).to receive(:error).never
 
-        msg_or_context = {"retry" => true, "retry_count" => 1}
-
-        described_class.handle_exception(msg_or_context, exception)
+        described_class.handle_exception(
+          { :job => { 'retry' => true, 'retry_count' => 1 } },
+          exception
+        )
       end
 
       it 'sends the error to rollbar above the threshold' do
         allow(Rollbar).to receive(:scope).and_return(rollbar)
         expect(rollbar).to receive(:error)
 
-        msg_or_context = {"retry" => true, "retry_count" => 2}
-
-        described_class.handle_exception(msg_or_context, exception)
+        described_class.handle_exception(
+          { :job => { 'retry' => true, 'retry_count' => 2 } },
+          exception
+        )
       end
 
       it 'sends the error to rollbar if not retry' do
         allow(Rollbar).to receive(:scope).and_return(rollbar)
         expect(rollbar).to receive(:error)
 
-        msg_or_context = {"retry" => false}
-
-        described_class.handle_exception(msg_or_context, exception)
+        described_class.handle_exception(
+          { :job => { 'retry' => false } },
+          exception
+        )
       end
 
       it 'does not blow up and sends the error to rollbar if retry is true but there is no retry count' do
         allow(Rollbar).to receive(:scope).and_return(rollbar)
         expect(rollbar).to receive(:error)
 
-        msg_or_context = {"retry" => true}
-
-        expect {
-          described_class.handle_exception(msg_or_context, exception)
-        }.to_not raise_error
+        expect do
+          described_class.handle_exception(
+            { :job => { 'retry' => true } },
+            exception
+          )
+        end.to_not raise_error
       end
     end
   end