Skip to content

cloudsec: retry a code-scan push refused with 429, honouring Retry-After - #401

Merged
maximelb merged 2 commits into
masterfrom
ingest-retry-busy
Sep 26, 2026
Merged

maximelb merged 2 commits into
masterfrom
ingest-retry-busy

Conversation

@maximelb

Copy link
Copy Markdown
Contributor

What

A code-scan push (CloudSec.ingest_code_results, so both cloudsec code ingest and cloudsec code scan --ingest) now re-sends a push that the API refuses with HTTP 429.

The API answers 429 on this route in two cases:

  • the organization already has as many pushes in progress as it may (error_code: "ingest_busy", with Retry-After), which a CI job pushing many repositories at once can hit;
  • the per-identity request quota is spent.

Neither one records anything, so sending the same push again is safe. Until now the CLI failed the CI job on the first one.

How it backs off

  • It waits at least the response's Retry-After, clamped to 120 s so a single header cannot park a job for an hour.
  • With no Retry-After, it backs off exponentially: 5 s, 10 s, 20 s, 40 s, then 60 s.
  • It adds random jitter of 0 to 50% on top of each wait, never below it. A fan-out refused all at once therefore comes back spread out.
  • Limits: 5 re-sends (busy_retries=, and 0 restores the old behaviour) and 10 minutes of total waiting. After that the RateLimitError is raised.
  • The client's own --retry 429 loop is turned off for this call (Client.request(..., retry_quota_errors=False)). Left on, it would ignore Retry-After, skip the jitter and multiply the attempts.

Supporting changes:

  • RateLimitError.retry_after is now filled from the Retry-After header. Both the seconds form and the HTTP-date form are read. When the header is absent the value is None, not 0.
  • The request body is serialized once and re-used across attempts.

Tests

tests/unit/test_code_ingest_busy_retry.py drives the real Client with only urlopen and the sleep replaced, so the header takes the same path it does against the API. The tests cover:

  • a push refused as busy, then accepted;
  • giving up after the configured number of re-sends;
  • busy_retries=0;
  • the 10-minute bound when the server asks for an hour;
  • no stacking with --retry;
  • no retry of a 400;
  • the backoff shape and the jitter bounds.

I checked that the tests can fail. Each of these three changes breaks at least two of them:

  • removing the retry loop;
  • dropping Retry-After;
  • ignoring the per-call override.

Full suite: pytest tests/unit/ tests/microbenchmarks/ gives 4585 passed.

The 429 with Retry-After comes from an API gateway change that ships separately. Against an API without it, busy pushes still come back as 400 and are not retried, which is the same behaviour as today.

🤖 Generated with Claude Code

A push is refused with 429 when the organization already has as many pushes
in progress as it may (error_code ingest_busy) or the request quota is spent.
Nothing is recorded for a refused push, so ingest_code_results now re-sends
it: at least the response's Retry-After (clamped to 120s), exponential backoff
without one, random jitter on top, at most 5 re-sends and 10 minutes of
waiting. RateLimitError.retry_after is now filled from the header.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
lcbill
lcbill previously approved these changes Sep 26, 2026
…sh gives up

Non-ASCII digits and overlong values no longer escape as ValueError; the
exhausted-retry error says how many times the push was refused instead of
suggesting --retry, which does not apply to this path.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@maximelb

Copy link
Copy Markdown
Contributor Author

Self-review record (this repo does not use the automated review bot): I ran an adversarial review from a pristine git archive extraction of c85313a. It found no high or medium issues, and four low ones. Three are fixed in a4d4657 and the fourth was noted only:

  • parse_retry_after used str.isdigit(), which accepts non-ASCII digits, and very long digit strings exceed Python's int-parse limit. Either one could let a ValueError escape from Client.request. It now requires ASCII digits and clamps overlong values. Both cases are tested.
  • When retries ran out, the error still suggested --retry, which does not affect this path. It now says how many times the push was refused and over how long.
  • The CHANGELOG now says retry_after is filled only when the client raises on the first 429.
  • Not changed: there is no gateway-side test for an empty busy message. The matching handles that case correctly.

End-to-end check against a full local stack: the real SDK, installed from this branch, drove 12 concurrent pushes for one org against a service limited to one push at a time with a queue of one.

  • busy_retries=0: 11 of 12 were refused with RateLimitError(retry_after=30).
  • Default settings: all 12 landed in 45 s. Each refused push waited once, and the waits were spread between 30.9 s and 44.9 s, so the jitter honoured the 30 s floor.

Leaving this for review. It is not merged.

@maximelb
maximelb merged commit cfc6cf6 into master Sep 26, 2026
6 checks passed
@maximelb
maximelb deleted the ingest-retry-busy branch September 26, 2026 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants