Skip to content

Unwraps Sidekiq::JobRetry::Handled errors#185

Merged
nikz merged 2 commits intoMindscapeHQ:masterfrom
nikz:unwrap-sidekiq-handled-error
Jul 28, 2024
Merged

Unwraps Sidekiq::JobRetry::Handled errors#185
nikz merged 2 commits intoMindscapeHQ:masterfrom
nikz:unwrap-sidekiq-handled-error

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, PanosNB, TheRealAgentK and miquelbeltran 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.

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
@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