fix(kosong): fail stalled provider streams instead of hanging forever - #2797
fix(kosong): fail stalled provider streams instead of hanging forever#2797matthiasgoergens wants to merge 1 commit into
Conversation
🦋 Changeset detectedLatest commit: 12963a6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd3e642448
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }); | ||
|
|
||
| try { | ||
| const outcome = await Promise.race([iterator.next(), watchdog]); |
There was a problem hiding this comment.
When a provider or its transport has already subscribed to the linked signal and the pending iterator.next() rejects before the watchdog promise wins, this race rejects directly and bypasses the 'aborted' branch. A custom ChatProvider can therefore surface its provider-specific cancellation error instead of the documented AbortError, and its explicit teardown hooks are skipped. Catch failures from this race and, when the caller signal is aborted, cancel the stream and throw the standard abort error; the mirrored v2 implementation has the same race.
Useful? React with 👍 / 👎.
| // Abort first: providers forward the signal to their HTTP client, so | ||
| // this kills the dead connection and settles the in-flight iteration. | ||
| stallAbort.abort(); | ||
| await cancelStream(stream); |
There was a problem hiding this comment.
If a StreamedMessage is also its own async iterator, or exposes a cancel()/return() that waits for the currently pending next(), this await never settles when that iterator ignores the abort signal. The watchdog then reproduces the original indefinite hang instead of returning 'stalled' and throwing APITimeoutError; async-generator return() calls are queued behind an outstanding next(), so this is a valid provider shape. Make cancellation on the stall path fire-and-forget or otherwise bounded, as teardownIterator already intends; the mirrored v2 implementation has the same issue.
Useful? React with 👍 / 👎.
bd3e642 to
12963a6
Compare
A response that goes silent — before headers or mid-stream, connection open, no bytes (e.g. an overloaded or half-dead gateway) — blocked generate() forever: no timeout fired, cancel only took effect when the next part arrived, and the turn never produced a terminal event. Hosts wedged with it — VSCode sessions then rejected every follow-up message for the rest of the window's life. generate() now races both the header wait and each iterator.next() against a resettable inactivity timer (streamStallTimeoutMs, default 300s, 0 disables) and an internal AbortController linked into the request signal via AbortSignal.any. On stall the linked abort tears down the provider HTTP connection and an APITimeoutError is thrown; isRetryableGenerateError already classifies it as retryable, so chatWithRetry recovers transient stalls and persistent ones end the turn with a real error. All watchdog teardown is fire-and-forget, so a faulty provider whose cancel()/return() never settles cannot hang the stall path. The same race makes cancel-during-stall abort promptly. Mirrored in agent-core-v2's kosong contract copy. agent-core hosts can tune the budget via KIMI_STREAM_STALL_TIMEOUT_MS; the v2 copy takes the option programmatically only (no raw process.env read in v2 domains). Fixes MoonshotAI#1050
12963a6 to
eaa464d
Compare
1 participant
Related Issue
Resolve #1050
Problem
See linked issue. In short: a response that goes silent — before headers or mid-stream, connection open, no bytes (e.g. an overloaded or half-dead gateway) — blocks
generate()forever. No timeout fires, cancel only takes effect when the next part arrives, and the turn never produces a terminal event. Hosts wedge with it: in the VSCode extension the session then rejects every follow-up message for the rest of the window's life (surfaced as "Internal error occurred.", see #2796).Verified from a real session log: the step's
llm requestis logged, the wire log ends mid-thinking-token, and nollm responseever follows.What changed
generate()(kosong, and the mirroredagent-core-v2kosong contract) races both the response-headers wait and everyiterator.next()against:GenerateOptions.streamStallTimeoutMs(defaultDEFAULT_STREAM_STALL_TIMEOUT_MS = 300_000,0disables), andthrowIfAbortedonly ran when a part arrived).AbortControllerlinked into the request signal viaAbortSignal.anytears down the provider's HTTP connection (providers forward the signal to their HTTP clients), andAPITimeoutErroris thrown.isRetryableGenerateErroralready classifies that as retryable, sochatWithRetryrecovers transient stalls and persistent ones end the turn with a real error instead of a wedge.cancel()/return(), iteratorreturn(), late-resolvingprovider.generate()) is fire-and-forget — a faulty provider whose teardown never settles cannot hang the stall path; a stream acquired just as the watchdog fires is still cancelled.agent-corehosts can tune the budget viaKIMI_STREAM_STALL_TIMEOUT_MS(wired inKosongLLM, following the existingKIMI_*env-override pattern). The v2 copy takes the option programmatically only — no rawprocess.envread in v2 domains per the v2 config rule.for awaitsemantics on exceptional exit: a throwing callback/merge still closes the iterator and cancels the stream (best-effort, never blocking the original error).serverDecodeMs/clientConsumeMs) semantics preserved;Promise.racekeeps every abandoned promise observed, so no unhandled rejections.Tests: unit tests for stall mid-stream / stall-before-first-part / stall-before-headers / never-settling teardown / budget reset per part / abort-during-stall / disable-via-0 (both copies), plus a live-HTTP e2e driving the real
KimiChatProvideragainst a local server that sends one SSE chunk and then holds the socket open forever.Reproduced end-to-end with a mock OpenAI-compatible server that stalls mid-stream: pre-fix the CLI hangs indefinitely; post-fix the stall is detected after the budget and retried (
llm request failed ... errorName=APITimeoutError).Checklist
gen-changesetsskill, or this PR needs no changeset. (changeset included:@moonshot-ai/kimi-code+@moonshot-ai/kimi-code-sdkpatch)gen-docsskill, or this PR needs no doc update.