Skip to content

🐛 fix: stop sentry app tasks from emitting NoRetriesRemainingErrors#98292

Merged
iamrajjoshi merged 4 commits into
masterfrom
raj/sentry-apps/actually-stop-emitting-rety-errors
Aug 27, 2025
Merged

🐛 fix: stop sentry app tasks from emitting NoRetriesRemainingErrors#98292
iamrajjoshi merged 4 commits into
masterfrom
raj/sentry-apps/actually-stop-emitting-rety-errors

Conversation

@iamrajjoshi

Copy link
Copy Markdown
Collaborator

these sentry app tasks are "fire and forget"/ best effort tasks to send a payload to a sentry app. if the service on the sentry app side is down, we can't do much and emitting the exception causes extra noise we don't need.

lets ignore them.

@iamrajjoshi iamrajjoshi self-assigned this Aug 26, 2025
@iamrajjoshi iamrajjoshi requested review from a team as code owners August 26, 2025 21:11
@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Aug 26, 2025
cursor[bot]

This comment was marked as outdated.

Comment on lines +106 to 107
raise_on_no_retries=False,
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Potential bug: Adding raise_on_no_retries=False causes tasks to fail with RetryError instead of being silently ignored, contradicting the intended 'fire and forget' behavior.
  • Description: Adding raise_on_no_retries=False to the retry_decorator changes the exception raised upon exhausting retries from NoRetriesRemainingError to RetryError. The task's exception handling is configured to silently ignore NoRetriesRemainingError but re-raises RetryError. This change unintentionally alters the behavior of these 'fire and forget' tasks, causing them to fail loudly with a RetryError when all retries are used up, instead of failing silently as intended. This leads to unexpected task failures.

  • Suggested fix: To restore the intended 'fire and forget' behavior, either add RetryError to the ignore tuple in the retry_decorator or remove the raise_on_no_retries=False parameter to revert to the previous exception-raising behavior.
    severity: 0.55, confidence: 0.95

Did we get this right? 👍 / 👎 to inform future reviews.

@Christinarlong Christinarlong left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

cursor[bot]

This comment was marked as outdated.

@iamrajjoshi iamrajjoshi merged commit 5f9a489 into master Aug 27, 2025
63 checks passed
@iamrajjoshi iamrajjoshi deleted the raj/sentry-apps/actually-stop-emitting-rety-errors branch August 27, 2025 16:37
@codecov

codecov Bot commented Aug 27, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 57.14286% with 3 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/sentry/taskworker/retry.py 25.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #98292      +/-   ##
==========================================
- Coverage   81.01%   80.98%   -0.03%     
==========================================
  Files        8562     8567       +5     
  Lines      376457   376806     +349     
  Branches    24123    24123              
==========================================
+ Hits       304982   305157     +175     
- Misses      71094    71268     +174     
  Partials      381      381              

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants