From 19251ea28b13b9c314c888b10ff9f079049cc871 Mon Sep 17 00:00:00 2001 From: Natik Gadzhi Date: Thu, 23 Nov 2023 13:53:58 -0800 Subject: [PATCH] Fix sentry-delayed_job: respect custom max_attempts (#2177) * sentry-delayed_job: respect custom max_attempts * Update sentry-delayed_job/spec/sentry/delayed_job_spec.rb Co-authored-by: Stan Lo * Changelog for delayed_job max_attempts fix --------- Co-authored-by: Stan Lo --- CHANGELOG.md | 1 + .../lib/sentry/delayed_job/plugin.rb | 3 ++- .../spec/sentry/delayed_job_spec.rb | 16 ++++++++++++++++ .../spec/sentry/transport/http_transport_spec.rb | 1 - 4 files changed, 19 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ae8bc5ee2..c5beeb7b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ - Fixed a deprecation in `sidekiq-ruby` error handler [#2160](https://github.com/getsentry/sentry-ruby/pull/2160) - Avoid invoking ActiveSupport::BroadcastLogger if not defined [#2169](https://github.com/getsentry/sentry-ruby/pull/2169) +- Respect custom `Delayed::Job.max_attempts` if it's defined [#2176](https://github.com/getsentry/sentry-ruby/pull/2176) ## 5.13.0 diff --git a/sentry-delayed_job/lib/sentry/delayed_job/plugin.rb b/sentry-delayed_job/lib/sentry/delayed_job/plugin.rb index 004a391b7..87640211d 100644 --- a/sentry-delayed_job/lib/sentry/delayed_job/plugin.rb +++ b/sentry-delayed_job/lib/sentry/delayed_job/plugin.rb @@ -87,7 +87,8 @@ def self.report?(job) # We use the predecessor because the job's attempts haven't been increased to the new # count at this point. - job.attempts >= Delayed::Worker.max_attempts.pred + max_attempts = job&.max_attempts&.pred || Delayed::Worker.max_attempts.pred + job.attempts >= max_attempts end def self.finish_transaction(transaction, status) diff --git a/sentry-delayed_job/spec/sentry/delayed_job_spec.rb b/sentry-delayed_job/spec/sentry/delayed_job_spec.rb index a0cd2e738..535828e28 100644 --- a/sentry-delayed_job/spec/sentry/delayed_job_spec.rb +++ b/sentry-delayed_job/spec/sentry/delayed_job_spec.rb @@ -143,6 +143,22 @@ def self.class_do_nothing expect(transport.events.count).to eq(1) end + # Default max_attemps is defined on Delayed::Worker.max_attempts == 25. + # However, users can customize max_attempts on the job class, and DelayedJob + # will respect that. + # Sentry needs to report an exception if report_after_retries is true and + # custom job-level max_attempts is reached. + # See https://github.com/collectiveidea/delayed_job#custom-jobs + it "reports exception after the job's custom max_attempts" do + enqueued_job.update(attempts: 2) + allow(enqueued_job).to receive(:max_attempts).and_return(3) + + expect do + enqueued_job.invoke_job + end.to raise_error(ZeroDivisionError) + expect(transport.events.count).to eq(1) + end + it "skips report if not on the last retry" do enqueued_job.update(attempts: 0) diff --git a/sentry-ruby/spec/sentry/transport/http_transport_spec.rb b/sentry-ruby/spec/sentry/transport/http_transport_spec.rb index e7ddf1866..5d1c4813d 100644 --- a/sentry-ruby/spec/sentry/transport/http_transport_spec.rb +++ b/sentry-ruby/spec/sentry/transport/http_transport_spec.rb @@ -275,7 +275,6 @@ stub_request(error_response) expect { subject.send_data(data) }.to raise_error(Sentry::ExternalError, /error_in_header/) - end end end