fix: make retry backoff able to clear a rate-limit quota window - #72
Merged
Conversation
The 429 retry path could not recover from a per-minute quota limit by construction, which is the common case on Gemini's free tier (5 RPM) for any model calling an LLM per agent per tick. - Raise the per-sleep ceiling: the fixed 10s MaxDelayMs truncated Google's actual 18.5s ask, so every retry fired inside the still-closed window and was guaranteed to fail. A single delay is now capped by the policy's maxDelay (64s) and bounded overall by a total elapsed budget. - Parse the provider's requested delay from the response BODY, not just the Retry-After header. Gemini returns it only in error.details[].RetryInfo.retryDelay (and in error.message prose); reading the header alone discarded the one value that would make the retry succeed. Exposed as the parseRetryDelayMs hook for per-provider overrides. - Raise the total retry budget: 3 retries at 1s/2s/4s spent ~7s, far short of the 60s an RPM window needs. Default is now 6 retries / 65s, tunable via retry_max_retries and retry_max_elapsed_seconds. - Add jitter (25%, additive only) so agents rate-limited by the same window do not retry in lockstep - the deterministic backoff was worst-case for ABM. Jitter never shortens a provider-requested delay. - Waiting out a quota window no longer counts against timeout_seconds. That budget covers the request itself, so the await bound is now the request timeout plus the retry budget; lowering timeout_seconds no longer forfeits rate-limit recovery. - Surface long waits: waits >=2s print a stderr note (a silent 20-60s stall in `go` reads as a hang) and increment session counters. Budget exhaustion now returns actionable guidance that the quota may be too low for the simulation. Risk: default behavior now waits materially longer before failing a persistent 429 (up to ~65s vs ~7s). This is intended - failing fast on a rate limit meant failing always. Both bounds are configurable, and non-rate-limit errors still fail immediately. Refs #65
Retry-After: 0 parses successfully to Some(0), so short-circuiting on the header discarded a body-supplied delay entirely. A 429 carrying both Retry-After: 0 and Gemini's retryDelay: "18.5s" retried after the local 1s backoff — inside the quota window, guaranteed to fail. That is the exact defect this policy exists to fix, reachable through the fix itself. Neither source may mask the other, so both are parsed and the longer wins. The previous test asserted header-wins as intended behaviour and is replaced by one covering both directions, plus a regression test pinning the Retry-After: 0 case. Also corrects two documented claims that measurement contradicted: - maxDelay is a ceiling on the computed backoff, applied BEFORE jitter, so an actual sleep can exceed it by up to jitterFactor (25% by default) - a persistent 429 fails after roughly 31-39s of waiting, not the 65s budget: canRetry refuses the sixth sleep, and the old comment's 1+2+4+8+16+32 sums to 63s rather than exceeding 60s of additional wait - maxElapsed bounds scheduled SLEEP, not wall-clock; request time is not counted 75 tests pass.
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
1 participant
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.
Closes #65.
Why
Retry-with-backoff (#37) correctly detected 429s but could not satisfy any provider asking for more than 10 seconds. Three defects, all visible in one real Gemini free-tier response asking for 18.5s:
MaxDelayMscapped at 10s — every retry fired inside the window and was guaranteed to failRetry-Afterheader was parsed — Gemini returns the delay in the JSON body (error.details[].RetryInfo.retryDelay), so the one piece of information that would have made the retry succeed was discardedGemini's free tier is 5 requests/minute. Any model calling an LLM per agent per tick exceeds that on tick one, so this is the common case, not an edge case.
What
RetryPolicy— retry logic as a testable value type, separate from the HTTP loop. This is what allows testing 60s+ budget behaviour without sleeping.RetryInfo, prose inerror.message, and theRetry-Afterheader (which wins when both are present)MaxDelayMs/MaxRetriesreplaced with config-driven bounds; total budget defaults to 65s so it can actually clear a per-minute windowDesign calls
timeout_secondsinteraction. That key is the request budget; sitting out a provider's quota window is not the request being slow. The await bound is nowtimeout_seconds + retry_budget, so loweringtimeout_secondsdoes not silently forfeit rate-limit recovery.Bounded, not unlimited. Unbounded waiting turns a quota misconfiguration into an indistinguishable hang. Budget exhaustion produces actionable guidance — the real fix for 5 RPM against 200 agents is fewer requests, so the message leads with that.
Behaviour change worth noting
A persistent 429 now takes ~65s to fail instead of ~7s. That is the intended trade — failing fast on a rate limit meant failing always — but anyone relying on quick failure will notice. Both bounds are configurable, and non-rate-limit errors still fail immediately.
Tests
74 passing, 0 failed (baseline 54, +20). No live API calls; run three times consecutively with identical results.
sbt assemblyclean under-Xfatal-warnings.One pre-existing test was modified and this deserves review:
"fails after exhausting retries on persistent 429"hardcoded"after 4 attempts", which asserted the old 3-retry default — the exact thing being fixed. It now pinsretry_max_retries=3so it still asserts exhaustion behaviour, and a separate test asserts the new default is greater than 3.Left out deliberately
timeout_secondsis currently only applied as anAwaitbound inLLMExtension, never as a socket timeout on the request itself. Separate latent gap, left alone.