Wait out the rate limit instead of failing the file - #11
Merged
Conversation
create, process and bulk share one bucket of 10 requests per minute, and a
run spends two of them per file. Every project past five files therefore
meets a 429 partway through, and the CLI treated it as a failed upload and
carried on: the run ended with some files registered and some not, and
"HTTP 429" as the only clue in the log.
A 429 now asks for a retry rather than reporting failure. The wrapper waits
and calls again, up to 5 times, honouring Retry-After when the server sends
one and otherwise backing off 15s, 30s, 45s, 60s towards the one-minute
window the limit is measured over. PTC does not send Retry-After today, so
the backoff is what runs; the header path is there because the answer to
"who paces this" belongs to the server the moment it wants it back.
Response headers reach the retry through a single optional -D in ptc_curl,
so the five branching curl invocations stay untouched.
Failure descriptions also gained the {"error": "..."} shape. source_files
create and process answer that way, and reading only "message" turned them
into a bare "HTTP 422" in the CI log — the one place the reason was needed.
35 new tests. Verified end to end against a mock that 429s the first two
attempts of each call: the run waits, retries, and downloads the
translations, where before it lost the file.
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.
The API allows 10 requests per minute on a bucket shared by
create,processandbulk, and a run spends two of them per file. Any project past five files meets a 429 partway through.Before this, the CLI reported that 429 as a failed upload and moved to the next file. The run ended with some files registered and some not, and
HTTP 429as the only clue in the log — no reason text, because the rate-limit body uses a response shape the CLI did not read.What changes
A 429 now asks for a retry instead of reporting failure. The wrapper waits and calls again, up to 5 times:
Retry-Afterwhen the server sends one (capped at 300s so a bad header cannot hang a job);PTC does not send
Retry-Aftertoday, so the backoff is what actually runs. The header path exists because pacing is the server's call the moment it wants it back — and it is exercised by the end-to-end check below.Response headers reach the retry through one optional
-Dinsideptc_curl, so the five branching curl invocations are untouched.A non-429 failure is passed straight through and still fails fast — only the rate limit is retried.
Failure text
describe_api_failurenow also reads the{"error": "..."}shape.source_files#createand#processanswer that way, so reading onlymessageturned a real rejection into a bareHTTP 422in the CI log. When both keys are present, both are kept — they say different things (TRIAL_EXPIREDvsYour trial has ended).Verification
35 new tests in
tests/test-rate-limit.sh(215 across the suite, 0 failures): the backoff ladder,Retry-Afterincluding CRLF, HTTP-date and absurd values, retry exhaustion, non-429 pass-through, and that the internal retry signal never escapes to the shell as an exit code.End to end against a mock that 429s the first two attempts of both calls:
It waited 2s rather than the 15s default, which is the
Retry-Afterheader being read. The run finished and wrotede.json; before this change that file was lost.Not included
No
VERSIONbump — this repository does that in its own release commit.