FIX: Ensure retry_count is set when checking if a job is being retried (#36034)

When checking how many times an email job has been retried, we need to check
that the `retry_count` attribute has been set.

In a related error, the `UserEmail` retry wait handler assumed that the
triggering exception would always have a `wrapped` attribute, which
isn't necessarily the case if the exception was raised outside of the
job execution code.
This commit is contained in:
Gary Pendergast
2025-11-14 14:18:50 +11:00
committed by GitHub
parent 4a951465cd
commit c7e105d15c
3 changed files with 12 additions and 1 deletions
+1
View File
@@ -8,6 +8,7 @@ module Jobs
sidekiq_options queue: "low"
sidekiq_retry_in do |count, exception|
next if exception.wrapped.nil?
# retry in an hour when SMTP server is busy
# or use default sidekiq retry formula. returning
# nil/0 will trigger the default sidekiq
+1 -1
View File
@@ -6,7 +6,7 @@ module Sidekiq
yield
rescue => e
# Only suppress email errors from Jobs::UserEmail, and only for the first 3 retries
if worker.class == Jobs::UserEmail && job["retry_count"] < 3
if worker.class == Jobs::UserEmail && !job["retry_count"].nil? && job["retry_count"] < 3
raise Jobs::HandledExceptionWrapper.new(e)
end
@@ -33,6 +33,16 @@ RSpec.describe Sidekiq::SuppressUserEmailErrors do
end
end
end
context "when retry_count is nil" do
it "does not wrap the exception, allowing normal error logging" do
job = { "class" => "Jobs::UserEmail", "retry_count" => nil }
expect do
middleware.call(worker, job, queue) { raise StandardError, "Email send failed" }
end.to raise_error(StandardError, "Email send failed")
end
end
end
context "with non-UserEmail jobs" do