diff --git a/lib/rollbar/plugins/sidekiq/plugin.rb b/lib/rollbar/plugins/sidekiq/plugin.rb index ef0b90d7..02a3cc34 100644 --- a/lib/rollbar/plugins/sidekiq/plugin.rb +++ b/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) diff --git a/spec/rollbar/plugins/sidekiq_spec.rb b/spec/rollbar/plugins/sidekiq_spec.rb index 89c2ee23..644964bc 100644 --- a/spec/rollbar/plugins/sidekiq_spec.rb +++ b/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