Skip to content
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.

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

Unwraps Sidekiq::JobRetry::Handled errors #185

Merged
merged 2 commits into from
Jul 28, 2024

Conversation

nikz
Copy link
Contributor

@nikz nikz commented Jul 23, 2024

This means the inner exception is reported to Raygun, with a tag sidekiq_retry of "true".

Also adds a config setting to disable this behaviour and reporting of retried Sidekiq jobs (config.track_retried_sidekiq_jobs defaults to true)

This means the inner exception is reported to Raygun, with a tag
`sidekiq_retry` of "true".

Also adds a config setting to disable this behaviour and reporting of
retried Sidekiq jobs (`config.track_retried_sidekiq_jobs` defaults to
true)
@nikz nikz requested a review from sumitramanga July 23, 2024 07:32
@nikz nikz self-assigned this Jul 23, 2024
@sumitramanga sumitramanga requested review from a team, miquelbeltran, PanosNB and TheRealAgentK and removed request for a team July 24, 2024 02:23
TheRealAgentK
TheRealAgentK previously approved these changes Jul 24, 2024
Copy link

@TheRealAgentK TheRealAgentK left a comment

Choose a reason for hiding this comment

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

I think the change in itself looks good to me.

Copy link

@miquelbeltran miquelbeltran left a comment

Choose a reason for hiding this comment

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

Change looks good as well. Left a question regarding the default setting.

@@ -143,7 +146,8 @@ def initialize
breadcrumb_level: :info,
record_raw_data: false,
send_in_background: false,
error_report_send_timeout: 10
error_report_send_timeout: 10,
track_retried_sidekiq_jobs: true

Choose a reason for hiding this comment

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

Is true a good default value for this? I am not sure how annoying it is to have retries reported, but it is good that it can be disabled if required.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think so - rationale being it's better to log more errors initially and then turn off any that are noisy rather than silently swallow them

@nikz nikz merged commit 0f418b0 into MindscapeHQ:master Jul 28, 2024
12 checks passed
@nikz nikz deleted the unwrap-sidekiq-handled-error branch July 28, 2024 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants