fix(sdk): apply undici headersTimeout for long-running requests#33535
Open
bashrusakh wants to merge 5 commits into
Open
fix(sdk): apply undici headersTimeout for long-running requests#33535bashrusakh wants to merge 5 commits into
bashrusakh wants to merge 5 commits into
Conversation
Node's undici fetch defaults headersTimeout to 300s, which kills long-running sessions at the ~303s mark regardless of provider timeout config. The SDK client's req.timeout = false was a no-op for undici. Map provider.timeout to undici headersTimeout on the server->LLM path (revives anomalyco#26599), and use an undici Agent with headersTimeout: 0 on the SDK client->server path. Bun is unchanged; dynamic import keeps browser compat. Fixes anomalyco#30252
github-actions
Bot
added
needs:compliance
Contributor
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
Address review feedback on anomalyco#33535: - Make undici a regular dependency of @opencode-ai/sdk (was optional peer with silent .catch fallback that hid missing dep) - Map bodyTimeout alongside headersTimeout on server→provider path (fixes SSE streams with >300s gaps between chunks) - Extract createUndiciDispatcher into shared core/util/undici-dispatcher.ts - Extract nodeFetchWithDispatcher into shared sdk/js/src/node-fetch.ts - Add unit test for createUndiciDispatcher covering all branches
Address review feedback:
- Extract resolveTimeoutMs() as pure function so the mapping logic is
testable under Bun without hitting the Node-only Agent guard
- Test now verifies all mapping branches via resolveTimeoutMs + Agent
instance check (toBeInstanceOf) instead of poking private fields
- Restore browser-safety in SDK: lazy import('undici') inside guard
instead of top-level static import that would pull node:* into bundles
even when the runtime guard prevents execution
Concurrent first calls to nodeFetchWithDispatcher could each create an Agent before either cached the result. Cache the import promise itself so all callers share one resolution.
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.
Issue for this PR
Fixes #30252
Type of change
What does this PR do?
The issue report assumes a "Go server", but the server is Node/undici — there is no Go component. The ~303s wall is undici's default
headersTimeoutandbodyTimeout(both 300s).The change that actually fixes the repro is the SDK client→server dispatcher (
node-fetch.ts): the blockingPOST /session/:id/messageendpoint holds the connection open until the prompt completes, and undici kills it at 300s. The previousreq.timeout = falsewas a no-op — undici does not readRequest.timeout. The SDK lazily imports undici (now a hard dependency) inside a runtime guard and caches the import promise, so browser bundles never pull undici (and itsnode:*deps) and concurrent first calls share a singleAgentwith bothheadersTimeout: 0andbodyTimeout: 0.The
aisdk.ts/provider.tschanges mapprovider.timeoutto undici'sheadersTimeoutandbodyTimeouton the server→provider outbound path (revives #26599 by @osamahaltassan). These only take effect when core/server run under Node. When noprovider.timeoutis configured, the server→provider default stays at 300s — by design, to honor the documented config. The mapping logic is extracted as a pureresolveTimeoutMsfunction so it is testable under Bun without hitting the Node-only Agent constructor.Two shared modules avoid duplication:
packages/core/src/util/undici-dispatcher.ts—resolveTimeoutMs+createUndiciDispatcher, used by bothaisdk.tsandprovider.tspackages/sdk/js/src/node-fetch.ts—nodeFetchWithDispatcher, used by bothclient.tsandv2/client.tsBun is unchanged: both modules guard via
process.versions.bun. Custom providerfetchis unchanged: dispatcher is skipped when a custom fetch is set.bodyTimeoutis mapped alongsideheadersTimeout(same value), so SSE streams with gaps >300s between chunks are not killed by undici's defaultbodyTimeout. This does not conflict with the existingchunkTimeout/headerTimeoutabort signals — those useAbortSignal/AbortController, which are independent of the undici dispatcher's socket-level timeouts.How did you verify your code works?
bun typecheckpasses onpackages/coreandpackages/sdk/jspackages/opencodehas pre-existing errors inglobal.tsandproject.tsunrelated to this change (confirmed viagit stash— the errors exist on cleandev)bun test test/util/undici-dispatcher.test.ts— 10 pass, 0 fail:resolveTimeoutMs: false→0, positive number→value, undefined→undefined, 0→undefined, negative→undefined, null→undefined, string→undefinedcreateUndiciDispatcher: Bun guard returns undefined, undefined/0 returns undefined, valid timeout returnsAgentinstance (toBeInstanceOf)undefined/fall through to plainfetchwhenprocess.versions.bunis setfetchis unchanged: dispatcher is skipped when a custom fetch is providedScreenshots / recordings
N/A — no UI changes.
Checklist