From 5b7d1327bb2da81a749dc0548703361b5603a7f3 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Mon, 18 Oct 2021 09:19:08 +0100 Subject: [PATCH 01/12] Loosen assertions on Time.now calls Sometimes Time.now is called more than these tests allowed, making them flakey on CI I haven't been able to debug why this happens, but it's likely not even Bugsnag code calling it --- spec/breadcrumbs/breadcrumb_spec.rb | 2 +- spec/bugsnag_spec.rb | 2 +- spec/report_spec.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/breadcrumbs/breadcrumb_spec.rb b/spec/breadcrumbs/breadcrumb_spec.rb index b4bd4301..aa35effb 100644 --- a/spec/breadcrumbs/breadcrumb_spec.rb +++ b/spec/breadcrumbs/breadcrumb_spec.rb @@ -101,7 +101,7 @@ describe "#to_h" do it "outputs as a hash" do fake_now = Time.gm(2020, 1, 2, 3, 4, 5, 123456) - expect(Time).to receive(:now).and_return(fake_now) + expect(Time).to receive(:now).at_least(:once).and_return(fake_now) breadcrumb = Bugsnag::Breadcrumbs::Breadcrumb.new( "my message", diff --git a/spec/bugsnag_spec.rb b/spec/bugsnag_spec.rb index 99cc0656..6f5af3b4 100644 --- a/spec/bugsnag_spec.rb +++ b/spec/bugsnag_spec.rb @@ -580,7 +580,7 @@ module Kernel describe "request headers" do it "Bugsnag-Sent-At should use the current time" do fake_now = Time.gm(2020, 1, 2, 3, 4, 5, 123456) - expect(Time).to receive(:now).at_most(6).times.and_return(fake_now) + expect(Time).to receive(:now).at_least(6).times.and_return(fake_now) Bugsnag.notify(BugsnagTestException.new("It crashed")) diff --git a/spec/report_spec.rb b/spec/report_spec.rb index 7ab5dea7..20c3cd3b 100644 --- a/spec/report_spec.rb +++ b/spec/report_spec.rb @@ -49,7 +49,7 @@ def gloops it "#headers should return the correct request headers" do fake_now = Time.gm(2020, 1, 2, 3, 4, 5, 123456) - expect(Time).to receive(:now).twice.and_return(fake_now) + expect(Time).to receive(:now).at_least(:twice).and_return(fake_now) report_or_event = class_to_test.new( BugsnagTestException.new("It crashed"), From 001a34a5bcd974ca9ed2756cee2c306a7c0ff035 Mon Sep 17 00:00:00 2001 From: Kevan Carstensen Date: Wed, 17 Nov 2021 17:06:43 -0800 Subject: [PATCH 02/12] resque: be more specific when handling ActiveJob wrapped jobs --- lib/bugsnag/integrations/resque.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/bugsnag/integrations/resque.rb b/lib/bugsnag/integrations/resque.rb index 560dcfb0..022d0662 100644 --- a/lib/bugsnag/integrations/resque.rb +++ b/lib/bugsnag/integrations/resque.rb @@ -49,7 +49,7 @@ def save # when using Active Job the payload "class" will always be the Resque # "JobWrapper", not the actual job class so we need to fix this here - if metadata['args'] && metadata['args'][0] && metadata['args'][0]['job_class'] + if class_name == 'ActiveJob::QueueAdapters::ResqueAdapter::JobWrapper' && metadata['args'] && metadata['args'][0] && metadata['args'][0]['job_class'] class_name = metadata['args'][0]['job_class'] metadata['wrapped'] ||= class_name end From bbca376f71af6d4663e2fd5d3c85502dbe596b91 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 25 Nov 2021 16:45:06 +0000 Subject: [PATCH 03/12] Allow using rescue as a modifier --- .rubocop.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.rubocop.yml b/.rubocop.yml index cdf3009c..9e74055b 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -19,6 +19,9 @@ Metrics/ClassLength: Exclude: - 'lib/bugsnag/configuration.rb' +Style/RescueModifier: + Enabled: false + # We can't use ".freeze" on our constants in case users are monkey patching # them — this would be a BC break Style/MutableConstant: From 8ca46ec85f29a1aeb814b557c7dd7c2f51fd2d6c Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 25 Nov 2021 16:37:02 +0000 Subject: [PATCH 04/12] Protect against Active Job's args changing --- lib/bugsnag/integrations/resque.rb | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/lib/bugsnag/integrations/resque.rb b/lib/bugsnag/integrations/resque.rb index 022d0662..8d997f80 100644 --- a/lib/bugsnag/integrations/resque.rb +++ b/lib/bugsnag/integrations/resque.rb @@ -45,13 +45,17 @@ def save } metadata = payload - class_name = payload['class'] + class_name = metadata['class'] # when using Active Job the payload "class" will always be the Resque - # "JobWrapper", not the actual job class so we need to fix this here - if class_name == 'ActiveJob::QueueAdapters::ResqueAdapter::JobWrapper' && metadata['args'] && metadata['args'][0] && metadata['args'][0]['job_class'] - class_name = metadata['args'][0]['job_class'] - metadata['wrapped'] ||= class_name + # "JobWrapper", so we need to unwrap the actual class name + if class_name == "ActiveJob::QueueAdapters::ResqueAdapter::JobWrapper" + unwrapped_class_name = metadata['args'][0]['job_class'] rescue nil + + if unwrapped_class_name + class_name = unwrapped_class_name + metadata['wrapped'] ||= unwrapped_class_name + end end context = "#{class_name}@#{queue}" From 6e1ddf08ab24da98a3f94fc8605bcf42d9e550ae Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 25 Nov 2021 16:37:19 +0000 Subject: [PATCH 05/12] Add spec to prevent regressions --- spec/integrations/resque_spec.rb | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/spec/integrations/resque_spec.rb b/spec/integrations/resque_spec.rb index c75ac1f7..4716b33a 100644 --- a/spec/integrations/resque_spec.rb +++ b/spec/integrations/resque_spec.rb @@ -118,6 +118,33 @@ def require(path) } end + it "handles when args[0] does not respond to '[]'" do + resque = Bugsnag::Resque.new + exception = RuntimeError.new("hello") + + allow(resque).to receive(:exception).and_return(exception) + allow(resque).to receive(:queue).and_return("queue") + allow(resque).to receive(:payload).and_return({ + "class" => "ResqueJob", + "args" => [1234] + }) + + report = Bugsnag::Report.new(exception, Bugsnag.configuration) + + resque.save + + expect(Bugsnag).to have_sent_notification { |payload, headers| + event = get_event_from_payload(payload) + + expect(event["context"]).to eq("ResqueJob@queue") + expect(event["metaData"]["context"]).to eq("ResqueJob@queue") + expect(event["metaData"]["payload"]).to eq({ + "args" => [1234], + "class" => "ResqueJob", + }) + } + end + after do Object.send(:remove_const, :Resque) if @mocked_resque module Kernel From a6e8f22f3e44520a5f46a4ab2247eaa08294baeb Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 25 Nov 2021 16:37:35 +0000 Subject: [PATCH 06/12] Allow passing arguments to ResqueWorker --- .../rails_integrations/app/app/workers/resque_worker.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/fixtures/rails_integrations/app/app/workers/resque_worker.rb b/features/fixtures/rails_integrations/app/app/workers/resque_worker.rb index 0fa9d718..638068d4 100644 --- a/features/fixtures/rails_integrations/app/app/workers/resque_worker.rb +++ b/features/fixtures/rails_integrations/app/app/workers/resque_worker.rb @@ -1,7 +1,7 @@ class ResqueWorker @queue = :crash - def self.perform + def self.perform(*arguments, **named_arguments) raise 'broken' end end From f0b085b776d3357567d8dcd9b624db10d81bccd6 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 25 Nov 2021 16:37:48 +0000 Subject: [PATCH 07/12] Run Resque with arguments --- features/rails_features/integrations.feature | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/features/rails_features/integrations.feature b/features/rails_features/integrations.feature index 6743a0bc..29972f6e 100644 --- a/features/rails_features/integrations.feature +++ b/features/rails_features/integrations.feature @@ -72,7 +72,7 @@ Scenario: Rake @rails_integrations Scenario: Resque (no on_exit hooks) When I run "bundle exec rake resque:work" in the rails app - And I run "Resque.enqueue(ResqueWorker)" with the rails runner + And I run "Resque.enqueue(ResqueWorker, 123, %(abc), x: true, y: false)" with the rails runner And I wait to receive a request Then the request is valid for the error reporting API version "4.0" for the "Ruby Bugsnag Notifier" And the event "unhandled" is true @@ -85,6 +85,10 @@ Scenario: Resque (no on_exit hooks) And the event "metaData.config.delivery_method" equals "synchronous" And the event "metaData.context" equals "ResqueWorker@crash" And the event "metaData.payload.class" equals "ResqueWorker" + And the event "metaData.payload.args.0" equals 123 + And the event "metaData.payload.args.1" equals "abc" + And the event "metaData.payload.args.2.x" is true + And the event "metaData.payload.args.2.y" is false And the event "metaData.rake_task.name" equals "resque:work" And the event "metaData.rake_task.description" equals "Start a Resque worker" And the event "metaData.rake_task.arguments" is null @@ -93,7 +97,7 @@ Scenario: Resque (no on_exit hooks) Scenario: Resque (with on_exit hooks) Given I set environment variable "RUN_AT_EXIT_HOOKS" to "1" When I run "bundle exec rake resque:work" in the rails app - And I run "Resque.enqueue(ResqueWorker)" with the rails runner + And I run "Resque.enqueue(ResqueWorker, %(xyz), [7, 8, 9], a: 4, b: 5)" with the rails runner And I wait to receive a request Then the request is valid for the error reporting API version "4.0" for the "Ruby Bugsnag Notifier" And the event "unhandled" is true @@ -106,6 +110,12 @@ Scenario: Resque (with on_exit hooks) And the event "metaData.config.delivery_method" equals "thread_queue" And the event "metaData.context" equals "ResqueWorker@crash" And the event "metaData.payload.class" equals "ResqueWorker" + And the event "metaData.payload.args.0" equals "xyz" + And the event "metaData.payload.args.1.0" equals 7 + And the event "metaData.payload.args.1.1" equals 8 + And the event "metaData.payload.args.1.2" equals 9 + And the event "metaData.payload.args.2.a" equals 4 + And the event "metaData.payload.args.2.b" equals 5 And the event "metaData.rake_task.name" equals "resque:work" And the event "metaData.rake_task.description" equals "Start a Resque worker" And the event "metaData.rake_task.arguments" is null From 8610dd4915989b33ec2895e68ddeabd0188a6e63 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 25 Nov 2021 16:38:18 +0000 Subject: [PATCH 08/12] Fix typo --- features/rails_features/integrations.feature | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/features/rails_features/integrations.feature b/features/rails_features/integrations.feature index 29972f6e..7dc942ba 100644 --- a/features/rails_features/integrations.feature +++ b/features/rails_features/integrations.feature @@ -160,7 +160,7 @@ Scenario: Using Sidekiq as the Active Job queue adapter for a job that raises And the event "metaData.sidekiq.queue" equals "default" @rails_integrations -Scenario: Using Rescue as the Active Job queue adapter for a job that raises +Scenario: Using Resque as the Active Job queue adapter for a job that raises When I set environment variable "ACTIVE_JOB_QUEUE_ADAPTER" to "resque" And I run "bundle exec rake resque:work" in the rails app And I run "UnhandledJob.perform_later(1, yes: true)" with the rails runner @@ -231,7 +231,7 @@ Scenario: Using Sidekiq as the Active Job queue adapter for a job that works Then I should receive no requests @rails_integrations -Scenario: Using Rescue as the Active Job queue adapter for a job that works +Scenario: Using Resque as the Active Job queue adapter for a job that works When I set environment variable "ACTIVE_JOB_QUEUE_ADAPTER" to "resque" And I run "bundle exec rake resque:work" in the rails app And I run "WorkingJob.perform_later" with the rails runner From 3f5f267aa2e0609443b65ca599b2ea28eff2f256 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 25 Nov 2021 17:53:57 +0000 Subject: [PATCH 09/12] Update changelog --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index aabe56e4..d675ad4e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,14 @@ Changelog ========= +## TBD + +### Fixes + +* Fix metadata not being recorded when using Resque outside of Active Job + | [#710](https://github.com/bugsnag/bugsnag-ruby/pull/710) + | [isnotajoke](https://github.com/isnotajoke) + ## v6.24.0 (6 October 2021) ### Enhancements From 8c32f640e30a450c523307f23bf9d549e65057a9 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Tue, 30 Nov 2021 10:07:15 +0000 Subject: [PATCH 10/12] Add version & date to changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d675ad4e..730b0a49 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ Changelog ========= -## TBD +## v6.24.1 (30 November 2021) ### Fixes From 239733b3e06861c18232e2f24a957422cdd6b384 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Tue, 30 Nov 2021 10:07:29 +0000 Subject: [PATCH 11/12] Bump version --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 2496b04b..7aa9429e 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -6.24.0 +6.24.1 From 156c93909332b7cb95948339e994e4f242a4dffa Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Tue, 30 Nov 2021 10:24:11 +0000 Subject: [PATCH 12/12] Avoid YARD pulling in WEBrick on Ruby < 2.3 --- Gemfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile b/Gemfile index 5ae1a09c..94d1122e 100644 --- a/Gemfile +++ b/Gemfile @@ -6,7 +6,7 @@ group :test, optional: true do gem 'rake', ruby_version <= Gem::Version.new('1.9.3') ? '~> 11.3.0' : '~> 12.3.0' gem 'rspec' gem 'rspec-mocks' - gem 'yard', '~> 0.9.25' + gem 'yard', ruby_version < Gem::Version.new('2.3.0') ? '< 0.9.27' : '~> 0.9.25' gem 'pry' gem 'addressable', '~> 2.3.8'