dsh-llm-pi-ai drops Retry-After, so 503/429 retries fall back to a 500 ms local backoff #7926
songlairui
started this conversation in
Ideas
Replies: 0 comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
Summary
The harness already models a provider-requested retry delay (
LlmFailure.providerRetryAfterMs) and@deepseek-ai/dsh-llm-retryhonors it — but only@deepseek-ai/dsh-llm-deepseekever fills it in. Every route served by@deepseek-ai/dsh-llm-pi-ai(openai-responses,openai-completions) loses the response headers before the adapter sees them, so503/429+Retry-After: Ndegrades to the default exponential backoff: 5 attempts, 500 ms → 10 s, ±10% jitter.Environment: dsh
0.1.7-rc.1,dsh headless --jsonover a scratch profile (dsh-base+dsh-headless) whose default model points at a local mock.Measured
The mock answers every request with
503+Retry-After: 5. Time between successive POSTs:dsh-llm-deepseek(POST /v1/messages)+5024 / +5012 / +5015 / +5017 / +5015 ms— header honoreddsh-llm-pi-ai(POST /v1/responses)+1 / +540 / +1100 / +2085 / +4214 / +8615 ms— local backoffThe durable events show the same split:
429behaves the same way: pi-ai classifies it asRATE_LIMITand retries from 500 ms while ignoringRetry-After: 4.Repro
Why it matters
503 + Retry-After: 30is retried 0.5 s later, five times, then the turn fails.429is risk control on OpenAI-family accounts, not just capacity. Retrying it eagerly is exactly the shape of abusive traffic. Codex CLI defaults toretry_429: false/retry_5xx: true(HTTP 429 is surfaced as "exceeded retry limit" without retrying; expose retry_429 or clarify error openai/codex#30471 is about the misleading "exceeded retry limit" wording, not about the policy), and Anthropic documents 429s that carry noretry-afterat all. The harness default is more eager than either:RATE_LIMITis in the defaultretryableCodes,maxRetries: 5, backoff starting at 500 ms.Where the header is lost
dsh-llm-pi-aistreams through pi-ai with a fixed option set (profileOptions()plus temperature / maxTokens / sessionId / signal / headers) and never passes pi-ai'sonResponsecallback — the one call site that receives{ status, headers }right after the HTTP response (@earendil-works/pi-ai/dist/api/openai-responses.js). The failure path then keeps onlyformatProviderError(...), a string, soclassifyPiAiError()has nothing but message text to match/\b5\d\d\b/and/\b429\b/against. By the timeLlmFailureis constructed, the header no longer exists anywhere in the process.The parsing convention is already available in the dependency:
@earendil-works/pi-ai/dist/utils/provider-retry.js→getRetryDelayMs()readsretry-after-ms, thenretry-after(delta-seconds or HTTP-date) with a cap — the same shapedsh-llm-deepseek'sproviderError()implements.Suggested change
Forward the header into the field that already exists, inside
dsh-llm-pi-ai:onResponsethrough tomodels.streamSimple(...)(or otherwise capture the response headers for the attempt), andproviderRetryAfterMsof the terminal failure, matchingdsh-llm-deepseek'sproviderError(); keep5xx → SERVER,429 → RATE_LIMIT.That is enough for
dsh-llm-retryto wait exactly as long as the provider asked, with no new executor, no new policy surface, and no behavioral change for routes that send no header. Happy to open a PR if that is welcome —dsh-llm-mock-serveris already a devDependency of the adapter, so a mocked503 + Retry-Aftertest looks straightforward. A documented stance on429(retry vs. fail fast) would be a useful companion, since the current default retries it five times from 500 ms.Local stopgap (context, not a proposal)
Until this lands we bridge it in an out-of-tree plugin: a process-wide
fetchobserver recordsRetry-Afterfor retryable statuses, and a prependedagent/request-errorlistener writesproviderRetryAfterMs/statusback onto the failure — leaving the waiting, thellm/retryevents, the budget and cancellation todsh-llm-retry. The 429 timings quoted above come from that bridge, and it also offers a per-code cool-down floor for header-less risk-control 429s. It works, but it is a workaround for a header that could be read where it is produced.All reactions