-
Notifications
You must be signed in to change notification settings - Fork 510
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
fix(integrations): Check retries_left before capturing exception #3803
fix(integrations): Check retries_left before capturing exception #3803
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @malkovro!
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #3803 +/- ##
==========================================
+ Coverage 79.87% 79.90% +0.03%
==========================================
Files 137 137
Lines 15371 15373 +2
Branches 2608 2608
==========================================
+ Hits 12277 12284 +7
+ Misses 2223 2221 -2
+ Partials 871 868 -3
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like we need to make this compatible with older rq versions where the jobs don't have the retries_left
attribute, see here: https://github.com/getsentry/sentry-python/actions/runs/11915419148/job/33205698444?pr=3803
Should hopefully be a small change, maybe we can use getattr
and make sure if the property is not there, the behavior remains the same as before this change.
f4fdbb5
to
2754747
Compare
Bring back compatibility across rq versions Fixes GH-3707
2754747
to
30e8044
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, ty!
Since rq/rq#1964 the job status is set to Failed before the handler decides whether to capture or not the exception while
handle_job_failure
has not yet been called so the job is not yet re-scheduled leading to all exceptions getting captured in RQ version >= 2.0.Related to #1076
Fixes #3707