Provider error taxonomy + retry/backoff ([A6] / #13) - #61
Merged
Conversation
The agent retry loop needs to distinguish errors it should retry from
errors retrying will not fix. Two new variants:
- `ProviderError::RateLimit { message, retry_after }` — typically
HTTP 429. Carries the server's `Retry-After` hint when present so
the loop can honour it instead of clobbering it with its own
schedule.
- `ProviderError::Transient(message)` — typically HTTP 5xx and
request-level timeouts. Retried with exponential backoff.
Reclassification:
- `Transport` becomes retryable. TCP resets, DNS hiccups, and proxy
blips should not bubble up as hard failures on the first try.
- `Provider` (the catch-all bucket) stays non-retryable. Anything we
decide is worth retrying should land in `Transient` explicitly so
we don't hammer a provider with a request it rejected for a
non-transient reason.
Two helpers drive the retry decision: `is_retryable()` returns the
retry/no-retry table, and `retry_after()` exposes the rate-limit
hint to the loop. The provider impls (Gemini, future Anthropic via
[A4]) will map their wire formats to these variants in follow-ups.
…13) `retry_with_backoff` wraps an arbitrary `LlmProvider::complete`-shaped closure and retries on errors `ProviderError::is_retryable` flags. Behaviour: - Non-retryable errors return immediately. The reasoning loop never hammers a provider with a request it has already rejected for a non-transient reason (bad key, malformed request, safety filter). - Retryable errors back off exponentially: `initial × 2^attempt`, capped at `max_delay` so a high attempt count can't pin the loop on a multi-minute wait. - Jitter is applied to the computed delay so concurrent sessions don't synchronize a thundering herd on the same provider after a shared incident. - A `Retry-After` hint on `RateLimit` overrides the computed delay (still capped at `max_delay`) so a server's "wait 30s" is honoured exactly. `RetryConfig::interactive()` is the default — 3 retries, 250 ms initial, 8 s cap. `RetryConfig::aggressive()` is the patient batch profile. `RetryConfig::no_wait(n)` makes retry-loop tests deterministic without `tokio::time::pause()` boilerplate. The helper is pure async — no `tokio::spawn`, no task ownership — so the reasoning loop ([C2]) can call it inline around `provider.complete()` without restructuring the call shape.
The Gemini provider now maps HTTP status into the variants the retry helper branches on: - 429 → `ProviderError::RateLimit`, with `retry_after` filled from the response's `Retry-After` header (integer-seconds form only — the HTTP-date alternative falls back to exponential backoff). - 500-599 → `ProviderError::Transient`. The retry loop will back off and try again, preserving the status code in the message for the debug log. The existing buckets are unchanged: 400 stays `InvalidRequest`, 401/403 stay `Auth`, anything else 4xx stays in the unmodeled `Provider` catch-all (not retried). Tests cover each branch, plus the `parse_retry_after` shape — an integer header value round-trips, a date-form value yields `None`, a missing header yields `None`.
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
2 participants
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 #13.
Summary
Adds the typed-error classification + retry-loop the agent harness ([C2]) needs around
LlmProvider::complete:ProviderErrorgainsRateLimit { message, retry_after }andTransient(message)variants;Transportis reclassified as retryable.ProviderError::is_retryable()/retry_after()drive the retry decision tree.retry_with_backoff(cfg, op)wraps an async closure with exponential backoff, jitter, a max-delay cap, andRetry-Afterhonouring.classify_http_errormaps 429 →RateLimit(with header), 5xx →Transient; everything else stays where it was.Commits
Extend ProviderError with retry classification ([A6] / #13)— new variants + helpers + tests.Add retry_with_backoff helper for transient provider failures ([A6] / #13)— backoff helper module + tests.Classify Gemini HTTP errors into retry-aware variants ([A6] / #13)— Gemini wire mapping +Retry-Afterparsing + tests.Design
Retryable vs. not. Auth, InvalidRequest, Decode, and the unmodeled
Providerbucket are not retried — retrying does not change the answer, or worse, retrying a request the server rejected for safety / quota reasons hammers it. Transport, Transient, and RateLimit are retried.Backoff schedule.
RetryConfig::interactive()(default): 3 retries, 250 ms initial, 8 s cap, ±25% jitter.RetryConfig::aggressive(): 6 retries, 500 ms initial, 30 s cap.RetryConfig::no_wait(n): zero-delay, used by tests to exercise the loop withouttokio::time::pause()boilerplate.Retry-After. When set on aRateLimit, the helper uses it verbatim (still capped atmax_delayso a misbehaving provider can't pin the loop on a multi-minute wait).Jitter. Applied to the computed delay so concurrent sessions don't synchronize a thundering herd on the same provider after a shared incident.
The helper is pure async — no
tokio::spawn, no task ownership — so the reasoning loop ([C2]) can call it inline aroundprovider.complete()without restructuring the call shape.Test plan
cargo test -p core retry— 9 passed (delay table, jitter window,Retry-Afterprecedence, retry-budget exhaustion, success-stops-loop, non-retryable-not-retried).cargo test -p core provider::gemini— 11 passed, 1 ignored (existing live smoke). Covers 429 +Retry-After, 5xx, unmodeled 4xx,Retry-Afterparser.cargo test -p core provider::tests— 8 passed. Includesis_retryabletable andretry_afterexposure.cargo test -p core— 101 passed, 1 ignored, no regressions.Out of scope
Generated by Claude Code