Skip to content

Resolve #2860 - Do not ignore Job class sidekiq options while assessing retryable job #2861

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
aovertus wants to merge 1 commit into
base: master
Choose a base branch
from
+3 −0
Open

Resolve #2860 - Do not ignore Job class sidekiq options while assessing retryable job #2861

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions sentry-sidekiq/lib/sentry/sidekiq/error_handler.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ def call(ex, context, sidekiq_config = nil)

def retryable?(context)
retry_option = context.dig(:job, "retry")
retry_option ||= context.dig(:job, "class")&.safe_constantize
&.get_sidekiq_options
&.[]("retry")
Comment on lines 62 to +65
Copy link

@sentry sentry bot Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The use of ||= in retryable? causes a job-level retry: false setting to be incorrectly overridden by the class-level retry option, potentially suppressing Sentry error reports.
Severity: HIGH

Suggested Fix

Replace the ||= operator with an explicit if retry_option.nil? check. This ensures that a false value for the retry option from the job payload is respected and not overridden by the class-level configuration.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: sentry-sidekiq/lib/sentry/sidekiq/error_handler.rb#L62-L65

Potential issue: In the `retryable?` method, the `||=` operator is used to fall back to
class-level Sidekiq options. However, `||=` treats a boolean `false` as a falsy value.
If a specific job is enqueued with `retry: false`, which is a valid job-level setting,
the code will incorrectly ignore this `false` value and instead use the retry setting
from the worker class. When `report_after_job_retries` is enabled, this causes the error
handler to incorrectly assume the job will be retried. As a result, the initial error is
suppressed, and because the job will not actually retry, the error is never reported to
Sentry at all.

Copy link

@cursor cursor bot Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

||= overrides explicit retry:false from job hash

High Severity

Using ||= to fall back to class-level sidekiq options treats an explicit "retry" => false in the job hash the same as a missing/nil "retry" key. Since false is falsy in Ruby, ||= will overwrite it with the class-level option. A job explicitly marked as non-retryable in its payload could be treated as retryable if the class defines sidekiq_options retry: true. A nil check is needed instead of ||=.

Fix in Cursor Fix in Web

Copy link

@cursor cursor bot Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

retry_limit returns 0 for class-option-only retryable jobs

Medium Severity

When retryable? returns true based on class-level sidekiq_options (because the job hash has no "retry" key), retry_limit still only reads from context.dig(:job, "retry") which is nil. This causes retry_limit to return 0 via the else branch, making report_after_job_retries never defer reporting since retry_count < -1 is always false. The retry_limit method needs the same class-option fallback.

Additional Locations (1)

Fix in Cursor Fix in Web

# when `retry` is not specified, it's default is `true` and it means 25 retries.
retry_option == true || (retry_option.is_a?(Integer) && retry_option.positive?)
end
Expand Down