Repository navigation
fix: add per-attempt request timeout so a hung provider call fails into the retry ladder - #52
Conversation
…to the retry ladder A stalled OpenRouter request previously hung the job until the runner's 6-hour kill: chat.send had no timeout, and the retry/fallback ladder only engages once an attempt fails. Observed live on vault-onboarding PR #7, where a deepseek-v4-flash call sat 17+ minutes with fallback_model unset. - New request_timeout_seconds input (default 600) threaded from action.yml through config into the OpenRouter client - chat.send now passes the SDK's RequestOptions.timeoutMs, so a timed-out attempt aborts the HTTP call and classifies as a status-less retryable api_error — same-model retry, then fallback model Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![umm-actually[bot]](https://avatars.githubusercontent.com/in/4233716?s=60&v=4){kind=small}
|
Bound the generation cost lookup with the same request timeout
Pre-existing: the generation cost lookup runs with no timeout, so a stalled generations endpoint hangs the job until the runner's 6-hour kill — the exact incident class this PR's timeout was added to eliminate — even though the review was already accepted. The PR's own per-attempt timeout on chat.send is the fix template for this sibling SDK call. Failure scenario: An accepted chat response carries no usage cost (the makeNoCostChatResult path), and OpenRouter's generations endpoint stalls; requestReview awaits getGeneration without any bound, so the job sits until the runner kill and the accepted review never posts — the accepted attempt is in hand but postInlineFindings is never reached. Suggested fixExtend OpenRouterLike.generations.getGeneration to accept an options argument (request, options?: { timeoutMs?: number }) and call it as sdk.generations.getGeneration({ id: generationId }, { timeoutMs: requestTimeoutMs }); a timed-out lookup falls through the existing catch → warn → null-cost path. |
|
umm-actually re-reviewed at No new findings (3 tracked finding(s) across all runs). umm-actually · deepseek/deepseek-v4-flash-0731 |
A positive-integer check alone accepts values whose milliseconds overflow the timer cap (or reach Infinity), which timer implementations clamp to ~1 ms — every request would time out instantly. Bound the input at 2,147,483 seconds so seconds x 1000 always stays a valid delay. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The cost lookup ran with no timeout, so a stalled generations endpoint could hang the job after the review itself was already accepted — the same incident class the per-attempt timeout eliminates. The lookup is best-effort, so a timeout degrades to a null cost with a warning. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Addressing the PR-level umm-actually finding (generation cost lookup timeout, comment 5300432976): Valid — fixed in ff502de. 🔍 ship-check · pr-monitor · claude-fable-5 |
UMM_REQUEST_TIMEOUT_SECONDS tunes the per-attempt timeout from repo settings, matching the OPENROUTER_MODEL pattern. Uses an explicit || '600' fallback because a bare unset var passes an empty string, which would override the action default and fail validation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Workflows wiring a bare unset repo variable pass an empty string, which would otherwise override the action.yml default and fail validation. Empty now means "not provided" — same semantics as max_findings and fallback_model — so bare-var wiring works without an || fallback. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 participant
Problem
A stalled OpenRouter request hangs the job until the runner's 6-hour kill.
chat.sendhad no timeout, and the retry/fallback ladder only engages once an attempt fails — so a hung primary call means the fallback model never gets a chance.Observed live on vault-onboarding PR #7: a
deepseek/deepseek-v4-flash-0731call sat 17+ minutes past its typical ~6-minute completion, withfallback_modelunset, freezing the check indefinitely (it eventually returned, but nothing bounded it).Fix
request_timeout_secondsinput (default600), validated as a positive integer in config and threaded into the OpenRouter client asrequestTimeoutMschat.sendnow passes the SDK'sRequestOptions.timeoutMs— on timeout the SDK aborts the HTTP call and throws a status-less error, which the existing classifier already treats as a retryableapi_error: same-model retry once, then the fallback modelaction.ymlinput + README inputs tableVerification
tscclean,eslintclean, Prettier cleanoptions.timeoutMs, and config tests cover the new input's parse/reject paths🤖 Generated with Claude Code