fix(providers): bound a non-streaming request with an explicit 10-minute deadline - #6304
fix(providers): bound a non-streaming request with an explicit 10-minute deadline#6304waleedlatif1 wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview
New tests in Reviewed by Cursor Bugbot for commit db40573. Configure here. |
Greptile SummaryThis PR gives non-streaming OpenAI Responses requests an explicit ten-minute deadline while preserving caller cancellation and leaving streaming requests governed by idle timeout behavior.
Confidence Score: 5/5The PR appears safe to merge with no actionable correctness or security issues identified. The deadline is applied to every reachable non-streaming Responses request, streaming payloads consistently bypass it, caller cancellation remains composed into the resulting signal, and the pinned server runtime supports the APIs used.
|
| Filename | Overview |
|---|---|
| apps/sim/providers/openai/core.ts | Applies the explicit deadline at the shared Responses fetch boundary and correctly distinguishes literal streaming payloads. |
| apps/sim/providers/timeouts.ts | Defines and documents the ten-minute non-streaming provider-request timeout. |
| apps/sim/providers/openai/core.deadline.test.ts | Covers the timeout constant, non-streaming signal creation, streaming exclusion, and composition with caller cancellation. |
Reviews (1): Last reviewed commit: "fix(providers): bound a non-streaming re..." | Re-trigger Greptile
Correction from the 25-provider survey (73 agents, adversarially verified)The number is right; my characterisation of it was not. Eight "600000ms" claims failed adversarial verification — in every case the value was exact and the scope was wrong. The SDK's 600s is per-attempt and time-to-headers: the timer is armed before This PR arms a total deadline ( What the survey found that this PR does not fix
Where I disagree with the survey's own recommendationIt proposes a 60s TTFB deadline, on the grounds that 60s is the tightest vendor consensus (Groq, Cerebras, Together, Fireworks, Mistral). For streaming that is sound — TTFB genuinely means time-to-first-token. For non-streaming it would be a serious regression: headers don't arrive until generation completes, so a 60s TTFB caps every non-streaming generation at 60 seconds. Both production failures we're fixing here ran 279s and 296s and were legitimate long generations ("Build Story Bible", "Build Slide Render Specification"). A 60s cap would fail them faster rather than let them finish. The survey flags this in its own risks section but still carries 60s into the headline recommendation. I'd keep 600s for non-streaming as shipped here, and treat 60s TTFB as a streaming-only control alongside the stream-idle deadline — which is the genuinely novel finding, since no vendor implements an inter-chunk timeout at all. Confirmed as the bigger gapRetry is near-unanimous at maxRetries=2 across 8 independent vendors, on connection errors / 408 / 429 / 5xx. Sim does zero on this path. That remains the larger day-to-day deviation and the natural follow-up. Two traps the survey correctly refused: a widely-repeated "Z.ai 30s/3 retries" traces to a third-party MCP server's config, not Z.ai; and Sakana's |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit db40573. Configure here.
| headers: config.headers, | ||
| body: JSON.stringify(payload), | ||
| signal: abortSignal, | ||
| signal: withRequestDeadline(abortSignal, payload.stream === true), |
There was a problem hiding this comment.
Deadline ignored by Bun idle timer
High Severity
On Bun 1.3.14, AbortSignal.timeout does not outrank the runtime's ~300s idle fetch limit, so silent non-streaming generations can still die around five minutes—the failure mode this change aims to fix. The new 600s deadline never becomes the effective bound unless that idle timer is disarmed on the request.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit db40573. Configure here.


Summary
Sim sets no request timeout on provider calls, so every one inherits whatever the runtime imposes. Under Bun that is an undocumented ~300s idle timer — half of OpenAI's own documented default, chosen by nobody, and not configurable.
This sets an explicit 600,000 ms deadline on non-streaming provider requests, matching the vendor default verbatim:
node_modules/openai/client.d.ts:179—[opts.timeout=10 minutes].Why 600s and not a number I made up
It is the OpenAI client's own default, in the SDK version this repo vendors. Both measured production failures — 295,823 ms and 278,920 ms, on gpt-5.4 and gpt-5.6-luna — were generations still in progress, not stalled connections. Both would have completed under the vendor default.
This is not hypothetical or one customer: four models across two providers (OpenAI and Gemini), and the Gemini case recurs on the same workflow roughly daily for at least two weeks.
Streaming is deliberately excluded
I measured Bun's default with a raw-TCP probe on 1.3.14, the production version:
The timer is idle-based — any received byte resets it. So a streaming response, which emits continuously, is already bounded correctly by the runtime, and a total deadline there would cut off a long answer still arriving normally. The carve-out keys on
payload.stream === true, i.e. the request's own semantics.Caveat recorded honestly: the probe is HTTP/1.1 plaintext to localhost, and production is h2 over TLS. It explains the mechanism but does not explain why the two production failures fired early (4.2s and 21.1s before 300s). Leading hypothesis is that Bun's idle timer is connection-anchored and not reset on reuse of a pooled h2 connection, which would predict exactly those margins. Unproven. It does not change this fix — the runtime timer is demonstrably not anchored to our request, which is the reason to arm our own.
No regressions
abortSignalis preserved viaAbortSignal.any, so a user pressing Stop still wins.AbortSignal.timeoutaborts with aTimeoutError, which the existing phase annotation already classifies — diagnostics keep working unchanged.providers/timeouts.tsrather than the@/providersbarrel. That barrel is replaced wholesale byvi.mockin 21 test files, so an export added there resolves toundefinedin all of them. I hit this: the first version broke 30 tests across 4 files.Type of Change
Testing
4 tests: the constant matches the vendor default, a deadline is armed on non-streaming, no deadline on streaming, and the caller signal still aborts. Each verified fail-detectable by breaking it and watching it go red.
Providers + agent-handler suites: 110 files / 1420 tests passing. Typecheck, lint, and
check:api-validationclean.Follow-up
A survey of default timeout and retry policy across all 25 providers is running. Sim also performs zero retries on the OpenAI path, where the vendor client does 2 with exponential backoff on 408/409/429/5xx and connection errors — including retrying timeouts. That gap is bigger day-to-day than this one and is the natural next PR.
Checklist