Repository navigation
fix(provider): pin undici headersTimeout/bodyTimeout so long-running Node requests survive past 300s - #50656
Open
Yedigaryan wants to merge 3 commits into
Open
fix(provider): pin undici headersTimeout/bodyTimeout so long-running Node requests survive past 300s#50656Yedigaryan wants to merge 3 commits into
Yedigaryan wants to merge 3 commits into
Conversation
…Node requests survive past 300s Node's undici fetch defaults both headersTimeout and bodyTimeout to 300000ms. This silently kills any provider request — including SSE streams with a >300s gap between chunks — at the ~303s mark, regardless of a provider's own `timeout: false` / AbortSignal-based config, because those are cancellation mechanisms and don't touch undici's socket-level Agent dispatcher settings. Bun's fetch has no such default and is unaffected. Adds a shared `createUndiciDispatcher` util in packages/core that builds a custom undici Agent from the provider's `timeout` option (or disables both timeouts entirely when `timeout: false`), guarded to a no-op under Bun. Wires it into both provider-call paths (V2's aisdk.ts and V1's provider.ts) and into the SDK's own client→server fetch (node-fetch.ts), which needs the timeouts disabled outright since a blocking session request can run far longer than 5 minutes. Closes anomalyco#26602. Revives the approach from anomalyco#33535 (auto-closed by the repo's fewer-than-2-reactions PR-cleanup bot, not on technical grounds) against current dev, with one correctness fix: the SDK client's fetch wrapper also needs Bun's `timeout: false` request flag (oven-sh/bun#16682) preserved on the non-dispatcher path, which anomalyco#33535's node-fetch.ts had dropped. Verified: bun test in packages/core (1108 pass), packages/sdk/js (1 pass), and packages/opencode test/provider (714 pass); bun typecheck clean in all three touched packages (packages/opencode's pre-existing Buffer/Uint8Array errors in unrelated files confirmed present on unmodified dev via git stash); oxlint on touched files: 0 new errors. Co-Authored-By: Claude Sonnet 5 <[email protected]>
Contributor
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
Upstream moved the inline fetch wrapper into timeoutFetch(); the undici dispatcher is now built inside it so both callers get it. Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
This branch has not been deployed
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
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
Closes #26602
Type of change
What does this PR do?
Node's undici fetch defaults both
headersTimeoutandbodyTimeoutto 300000ms. That silently kills any provider request — including an SSE stream with a >300s gap between chunks — at the ~303s mark, regardless of a provider's owntimeout: false/AbortSignal-based config, because those are cancellation mechanisms and don't touch undici's socket-levelAgentdispatcher settings. Bun's fetch has no such default, which is why this only reproduces on Node (Desktop's sidecar process, or the SDK used from a Node host).This adds a shared
createUndiciDispatcherutil (packages/core/src/util/undici-dispatcher.ts) that builds a custom undiciAgentfrom the provider'stimeoutoption —falsedisables both timeouts outright, a positive number uses that value in ms, anything else leaves undici's defaults alone. It's guarded to a no-op under Bun viaprocess.versions.bun.Wired into three places that construct the outgoing fetch:
packages/core/src/aisdk.tspackages/opencode/src/provider/provider.tspackages/sdk/js/src/node-fetch.ts(new, used by bothclient.tsandv2/client.ts), which needs both timeouts disabled outright since a blocking session request can legitimately run past 5 minutesThis doesn't conflict with the existing
chunkTimeout/headerTimeoutabort-signal handling in either provider path — those areAbortSignal/AbortController-based and independent of undici's dispatcher-level socket timeouts.This revives the approach from #33535, which was auto-closed by this repo's engagement-based PR-cleanup bot (fewer than 2 reactions after a month) rather than for a technical reason — same fate as the earlier #26599. Rebased against current
dev. One correctness fix versus #33535: itsnode-fetch.tsdropped the pre-existingreq.timeout = falsemutation that disables Bun's own per-request idle timeout (oven-sh/bun#16682) on the non-dispatcher (Bun) path. This PR keeps it — it's a no-op under Node/undici and still required under Bun.How did you verify your code works?
bun testinpackages/core— 1108 pass, 0 fail (10 new tests forcreateUndiciDispatcher/resolveTimeoutMs, covering thefalse/number/undefined/Bun-guard cases)bun testinpackages/sdk/js— 1 pass, 0 failbun test test/providerinpackages/opencode— 714 pass, 0 failbun typecheckclean inpackages/core,packages/opencode,packages/sdk/js(packages/opencode has pre-existing Buffer/Uint8Array typecheck errors in unrelated files —test/image/image.test.ts,packages/tui, etc. — confirmed present on unmodifieddevviagit stash, not introduced by this change)oxlinton touched files: 0 new errorsScreenshots / recordings
N/A — not a UI change.
Checklist