Skip to content

fix: rate limit on one data category no longer blocks all others - #5482

Merged
jamescrosswell merged 3 commits into
mainfrom
fix/3947-rate-limit-blocks-errors
Aug 13, 2026
Merged

fix: rate limit on one data category no longer blocks all others#5482
jamescrosswell merged 3 commits into
mainfrom
fix/3947-rate-limit-blocks-errors

Conversation

@jamescrosswell

Copy link
Copy Markdown
Collaborator

Fixes #3947

Summary

An org over its transaction quota also stopped sending errors, for the duration of every rate-limit window.

Two independent rate-limit mechanisms exist in the SDK:

  1. Per-categoryHttpTransportBase.ExtractRateLimits parses X-Sentry-Rate-Limits into CategoryLimitResets and drops only the matching envelope items. This is correct.
  2. GlobalRetryAfterHandler sits at the top of the handler pipeline and short-circuits every subsequent request on any 429, without looking at X-Sentry-Rate-Limits at all.

(2) runs above (1), so the correct per-category logic was bypassed entirely: a 60:transaction;profile;span:... limit gated errors, sessions, check-ins and everything else for 60s.

RetryAfterHandler now applies its blanket back off only to a 429 that carries no X-Sentry-Rate-Limits header — i.e. the proxy / older-Sentry global-limit case it was written for. This matches the spec (X-Sentry-Rate-Limits first; Retry-After on 429 only "without the above headers") and sentry-python, whose Retry-After branch is a literal elif on the header being absent.

Notes for review

  • This was data loss, not deferred delivery. The short-circuit returns a synthetic 429 response rather than throwing, so HandleFailure just logs and returns, and CachingTransport then deletes the cache file. Blocked envelopes were gone for good. The empty Server response: in the reporter's log is the tell-tale of that synthetic response.
  • Why no test caught this: every existing HttpTransportTests case builds new HttpClient(handler) directly, which leaves RetryAfterHandler out of the pipeline. The new transport-level test goes through DefaultSentryHttpClientFactory so the handler is present, as in production. It fails on main with Expected requestCount to be 2 ... but found 1.
  • A fully-limited envelope still costs no request. A global limit expressed as X-Sentry-Rate-Limits with empty categories is handled by RateLimitCategory.IsMatchAll, and HttpTransport.SendEnvelopeAsync skips the HTTP call when ProcessEnvelope leaves zero items — so delegating to the transport doesn't trade the bug for extra traffic.
  • The existing RetryAfterHandlerTests all use bare 429s, so they're unaffected and still assert the fallback behaviour.

🤖 Generated with Claude Code

When Sentry rejects an envelope with a 429 carrying X-Sentry-Rate-Limits, the
RetryAfterHandler was short-circuiting *every* subsequent request for the
retry-after window, regardless of which categories were actually limited. An org
over its transaction quota therefore stopped sending errors too, and because a
short-circuited request produces a response (not an exception), the affected
envelopes were discarded rather than retried.

The per-category limits in that header are already applied per envelope item by
HttpTransportBase, so the handler now only applies its blanket back off to a 429
that carries no such header, matching the SDK spec and the other SDKs.

Fixes #3947

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Comment thread src/Sentry/Internal/Http/RetryAfterHandler.cs Outdated
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Comment thread src/Sentry/Internal/Http/RetryAfterHandler.cs Outdated
@codecov

codecov Bot commented Aug 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.72%. Comparing base (3fe027d) to head (948ebeb).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5482      +/-   ##
==========================================
- Coverage   74.73%   74.72%   -0.02%     
==========================================
  Files         513      513              
  Lines       18744    18744              
  Branches     3666     3666              
==========================================
- Hits        14009    14007       -2     
- Misses       3863     3864       +1     
- Partials      872      873       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jamescrosswell
jamescrosswell marked this pull request as ready for review August 13, 2026 03:31
@github-actions github-actions Bot added the risk: medium PR risk score: medium label Aug 13, 2026
@jamescrosswell
jamescrosswell merged commit 1890242 into main Aug 13, 2026
50 checks passed
@jamescrosswell
jamescrosswell deleted the fix/3947-rate-limit-blocks-errors branch August 13, 2026 04:24
@longzheng

Copy link
Copy Markdown

Any ideas when this will get released?

@jamescrosswell

Copy link
Copy Markdown
Collaborator Author

Any ideas when this will get released?

I've just requested a 6.9.0 release, so hopefully within the next 24-48 hours.

@jamescrosswell

Copy link
Copy Markdown
Collaborator Author

@longzheng 6.9.0 is out now.

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

Labels

risk: medium PR risk score: medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Exceptions not showing in Issues when Transactions are rate limited

3 participants