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.

This was referenced Aug 18, 2026
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