Repository navigation
fix(cli): Don't auto-retry non-idempotent requests; honor rate-limit reset on 429 - #1677
Merged
Merged
Conversation
…reset on 429 The authenticated fetch retried every method on 408/429/5xx, network errors, and timeouts, so a POST that reached the server could run up to three times (for example, three Seer autofix runs). On 429 it ignored X-Sentry-Rate-Limit-Reset and Retry-After and backed off blindly. Automatic retries now apply only to GET/HEAD/OPTIONS, or to requests that opt in with `idempotent: true`. Chunk uploads (stored by SHA-1) and artifact-bundle/DIF assemble calls (state keyed by checksum) opt in. The 401 token-refresh retry still applies to every method. On 429 the client waits for the server-requested delay (rate-limit reset measured against the response Date header, or Retry-After), capped at 10s. Concurrency-limit 429s and responses without a hint use jittered exponential backoff. When the wait exceeds the cap, retries run out, or the method can't be retried, it throws RateLimitError with the time to wait. The policy lives in a pure retry-policy.ts so it can move to toolkit-core unchanged. Co-Authored-By: Burak Yigit Kaya <126780+BYK@users.noreply.github.com>
Sentry's rate-limit middleware rejects a request in process_view, before the endpoint runs, so a 429 never applied the write. Retry 429 for every method. 5xx, timeouts, and network errors stay limited to GET/HEAD/OPTIONS and the `idempotent` opt-in. Retried statuses stay 408/429/500/502/503/504; 501, 505 and other deterministic 5xx are not retried. RateLimitError now tells callers (including agents) to retry in N seconds, or a few seconds without a hint, not to repeat the request before then, and to make fewer requests in parallel.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
The assemble loop already re-sends the identical body to poll, and the server reuses the in-progress PreprodArtifact row via get_or_create, so retrying a transient 5xx/timeout is no riskier than the next poll.
BYK
requested changes
Oct 27, 2026
…try methods - RateLimitError now exits with EXIT.RATE_LIMITED (32) so scripts and agents can back off without parsing stderr. Documented in exit-codes.md. - Rename IDEMPOTENT_METHODS to SAFE_METHODS and reference RFC 9110 9.2.1/9.2.2; note the retryable status list mirrors common client defaults since HTTP defines none.
BYK
approved these changes
Oct 27, 2026
BYK
added a commit
to getsentry/sentry
that referenced
this pull request
Oct 27, 2026
…128930) Lets endpoints declare which non-safe methods are idempotent, and publishes that as an OpenAPI operation extension. Generated clients such as `@sentry/api`, and through it the CLI and MCP, can then retry those operations on timeouts and 5xx responses instead of hard-coding per-call opt-ins. Refs #128927. Context: getsentry/toolkit#1677. **What changes** - `Endpoint.idempotent_methods: frozenset[HTTP_METHOD_NAME]` (default empty). Listing a method there means resending the identical request after a timeout or 5xx cannot change the outcome. - `custom_preprocessing_hook` collects declarations for every endpoint, like silos, and resets them on every build. `custom_postprocessing_hook` stamps `"x-sentry-idempotent": true` on matching operations. Operations without a declaration get no marker. - The build fails if an endpoint lists a safe method (`GET`/`HEAD`/`OPTIONS`/`TRACE`, RFC 9110 §9.2.1) or a method it does not declare in `publish_status`. - `PUT`/`DELETE` are not marked automatically. Not every handler lives up to RFC 9110 §9.2.2, so the absence of the marker means "not known to be idempotent". - Documented in `src/sentry/apidocs/README.md`. **Declared on** - `ChunkUploadEndpoint` `POST`: blobs are unique per checksum. - `DifAssembleEndpoint` `POST` and `OrganizationArtifactBundleAssembleEndpoint` `POST`: assemble state is cached per checksum, so a resend reports it instead of starting a second job. `ProjectPreprodArtifactAssembleEndpoint` is deliberately **not** marked. `create_preprod_artifact` only reuses a row while it is still `UPLOADING`, so a resend after that creates a second artifact and enqueues another assemble task. **Open questions for review** - All three declared endpoints are `PRIVATE`, so the public schema doesn't change yet. The declarations are inert until those endpoints are published, or until `@sentry/api` builds from a schema that includes them. Which route do we want? - Name and shape: a boolean `x-sentry-idempotent`, or a richer `x-sentry-retry` object? **Verification** - `ruff check` and `ruff format --check` are clean on the changed files. - New `IdempotentOperationsTest` cases in `tests/apidocs/test_hooks.py` cover: marking only declared methods, collecting unpublished methods, resetting between builds, and both validation errors. - I couldn't run the Sentry pytest/mypy environment in my sandbox, so I ran the same scenarios against the real `hooks.py` with stubbed imports, and they pass. CI is the authoritative run. <!-- junior-request-attribution:start --> via **BYK**. <!-- junior-request-attribution:end --> <!-- junior-session-footer:start --> <!-- junior-conversation-id:slack%3AC08J1NSPU6S%3A1792772794.175999 --> -- [View Junior Session](https://junior-prod.sentry.dev/conversations/slack%3AC08J1NSPU6S%3A1792772794.175999) [[Sentry]](https://sentry.sentry.io/explore/conversations/slack%3AC08J1NSPU6S%3A1792772794.175999/?project=4510944073809921) <!-- junior-session-footer:end --> --------- Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com> Co-authored-by: BYK <burak.kaya@sentry.io>
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
packages/cli/src/lib/sentry-client.tsretried every HTTP method, including POST/PUT/DELETE, on 408/429/500/502/503/504 and on network errors and timeouts. That can replay writes. A 429 also only got the generic 1s/2s backoff: the client never read Sentry'sX-Sentry-Rate-Limit-Resetheader, and Sentry doesn't sendRetry-Afteron API rate limits.Behavior changes
idempotent: trueopt-in ongetSdkConfig/ApiRequestOptions.process_view, before the endpoint runs, so nothing was applied.X-Sentry-Rate-Limit-Reset(read only when Remaining is 0) andRetry-After(seconds or HTTP date), both measured against the responseDateto guard against clock skew. Concurrency-limit 429s, or 429s with no hint, use jittered backoff: the upper half of min(1s·2^n, 10s).MAX_RETRIESis 2 and each wait is at most 10s.RateLimitError(extendsApiError, status 429). It's thrown when the wait would exceed the cap or retries run out. The message is agent-friendly: server detail, then "Retry in N seconds; do not repeat the request before then. Make fewer requests in parallel."packages/cli/src/lib/retry-policy.ts. That way the CLI can switch to the shared@sentry/toolkit-core/api-retryfrom fix(mcp): Retry rate-limited GETs using Sentry rate-limit reset headers #1673 without a rewrite.POST callers given the
idempotentopt-inEach was checked against its getsentry/sentry handler:
chunk-upload.ts): blobs are stored by SHA-1 (FileBlob.from_files).pollAssembly(sourcemaps.ts).debug-files.ts,proguard.tsanddart-symbols.ts: the server checks the cached assemble status by checksum.Not opted in: preprod assemble (each call creates a new
PreprodArtifact), Seer/autofix/, snapshot create and archive trigger, and SDK andsentry apiwrites.Tests
Retry-Afterwait, clock skew, and the cap leading toRateLimitErrorapi-client.coverageused 429 as a sample error and now uses 400.Checks
tsc --noEmitandpnpm run lintpass.bf3b710: 11,482 passed and 6 failed. The same 6 fail onmain(37c7193) in this sandbox. They're chmod-based permission tests (completions ×2, code scanner, init file-changes, telemetry readonly, upgrade lock), and the sandbox runs with full capabilities, so they can't fail the way they expect.7099e2b), all other CLI test files that mention 429 pass (1,195), apart from the 2 known sandbox permission failures.Relation to #1669
This matches the retry policy in #1669 (task 5.0 and the
sentry-api-transportspec). Task 5.1 will move this behavior into toolkit-core's shared transport unchanged.via BYK.
--
View Junior Session [Sentry]