fix(ai-client): cancel abandoned fetch response streams - #1598
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Important Review skippedReview was skipped as selected files did not have any reviewable changes. ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: TanStack/ai/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (2)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughStream readers now request cancellation without waiting when iteration exits. They ignore cancellation failures and release reader locks. Tests and documentation cover parsing failures, early exits, SSE ChangesStream response cleanup
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to No actionable issue is identified that should prevent merging after normal checks. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to Reader ownership and original errors are preserved while cleanup stops waiting for custom cancellation hooks. No expanded authority or new security exposure was established, but completing client cleanup does not guarantee that a custom transport has finished closing. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 5 files. (3 skipped: 3 unsupported.) ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Thanks for the PR, @quinnj! 🙌 @AlemTuzlak will take a look. Automated pre-review checks
Automated triage — a human review follows. |
|
View your CI Pipeline Execution ↗ for commit 7a309da
☁️ Nx Cloud last updated this comment at |
@tanstack/ai
@tanstack/ai-acp
@tanstack/ai-angular
@tanstack/ai-anthropic
@tanstack/ai-bedrock
@tanstack/ai-byteplus
@tanstack/ai-claude-code
@tanstack/ai-client
@tanstack/ai-cloudflare
@tanstack/ai-code-mode
@tanstack/ai-code-mode-snippets
@tanstack/ai-codex
@tanstack/ai-cohere
@tanstack/ai-compaction
@tanstack/ai-devtools-core
@tanstack/ai-durable-stream
@tanstack/ai-elevenlabs
@tanstack/ai-event-client
@tanstack/ai-fal
@tanstack/ai-gemini
@tanstack/ai-grok
@tanstack/ai-grok-build
@tanstack/ai-groq
@tanstack/ai-isolate-cloudflare
@tanstack/ai-isolate-daytona
@tanstack/ai-isolate-node
@tanstack/ai-isolate-quickjs
@tanstack/ai-isolate-quickjs-bun
@tanstack/ai-llmgateway
@tanstack/ai-lovable
@tanstack/ai-mcp
@tanstack/ai-memory
@tanstack/ai-mistral
@tanstack/ai-octane
@tanstack/ai-ollama
@tanstack/ai-ollaya
@tanstack/ai-openai
@tanstack/ai-opencode
@tanstack/ai-openrouter
@tanstack/ai-perplexity
@tanstack/ai-persistence
@tanstack/ai-preact
@tanstack/ai-react
@tanstack/ai-react-ui
@tanstack/ai-reactor
@tanstack/ai-remix
@tanstack/ai-sandbox
@tanstack/ai-sandbox-blaxel
@tanstack/ai-sandbox-boxd
@tanstack/ai-sandbox-cloudflare
@tanstack/ai-sandbox-daytona
@tanstack/ai-sandbox-docker
@tanstack/ai-sandbox-e2b
@tanstack/ai-sandbox-local-process
@tanstack/ai-sandbox-sprites
@tanstack/ai-sandbox-upstash-box
@tanstack/ai-sandbox-vercel
@tanstack/ai-skills
@tanstack/ai-solid
@tanstack/ai-solid-ui
@tanstack/ai-svelte
@tanstack/ai-typesafe
@tanstack/ai-utils
@tanstack/ai-vercel-gateway
@tanstack/ai-vertex
@tanstack/ai-vue
@tanstack/ai-vue-ui
@tanstack/ai-worldlabs
@tanstack/openai-base
@tanstack/preact-ai-devtools
@tanstack/react-ai-devtools
@tanstack/solid-ai-devtools
@tanstack/svelte-ai-devtools
commit: |
tombeckenham
left a comment
There was a problem hiding this comment.
Reproduced the open fetch-body leak on current main. This head closes the response on a chat parse error, an early iterator return, and a generation RUN_ERROR, and it still delivers a normal end.
Approving.
🦋 Changeset detectedLatest commit: 7a309da The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 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 |
Malformed chat data and early exits can leave a fetch response open after the client stops reading. This fix cancels unfinished chat and generation responses without delaying errors or iterator completion.
🎯 Changes
[DONE]marker.@tanstack/ai-client.✅ Checklist
pnpm run test:pr, or these tests do not apply to this pull request.docs/for this change, or this change is not user-facing.pnpm changeset), or this PR does not change a published package.🚀 Release Impact
Root cause
Issue. A chat parse error can leave the HTTP response open. The same leak occurs on early iterator exit or a generation
RUN_ERROR.Cause. Both
readStreamLinesimplementations only calledreader.releaseLock()infinally. Releasing the lock does not cancel the body or tell the server that the reader stopped.Fix. Each reader starts
reader.cancel(), observes any rejection, and releases its lock. Cleanup does not wait for a custom cancellation promise or replace the original error.Possible alternatives
finallycovers all exits.Testing
Commands run (Node 24.11.1, pnpm 11.9.0):
corepack pnpm --filter @tanstack/ai-client test:lib --run tests/sse-parser-cleanup.test.ts tests/connection-adapters-cleanup.test.ts tests/generation-client.test.ts tests/connection-adapters-resumable.test.ts: 103 passed.NX_DAEMON=false NX_PARALLEL=2 NX_BASE=origin/main corepack pnpm test:pr: all 149 tasks passed. The declaration scan passed for 1,102 files.corepack pnpm --filter @tanstack/ai-e2e test:e2e: 433 passed, three expected skips, no retries. Both new HTTP regressions passed.corepack pnpm exec oxfmt --check .: passed for 3,999 files. Coverage remains a CI check, as required by the contributing guide.Earlier E2E runs had intermittent failures outside the reader path. A separate clean-main run also failed in an existing test. One unchanged fal documentation snippet failed once in Kiira, then passed unchanged on main and this branch. The exact trigger remains unconfirmed.
Independent reproduction. Agent-written probes ran outside the PR in separate detached worktrees with the same Node version. Each HTTP probe kept the response open. The same probes failed on clean main and passed on
981c49a. Normal EOF, ignored generation input, and original reader errors remained correct.The commands used each worktree as the working directory:
The transcripts below retain result and assertion lines. Stack traces are omitted.
Clean main: chat (1f380ac)
PR head: chat (981c49a)
Clean main: generation (1f380ac)
PR head: generation (981c49a)
Cancellation liveness. A separate probe held each cancellation promise pending. Awaiting cancellation blocked all five operations. This head completed all five and released each reader lock before the cancellation promise settled.
Awaited-cancellation control (005e3b0)
PR head: pending cancellation (981c49a)
Manual test
data: {invalid json}\n\nfrom an HTTP response that stays open.fetchServerSentEvents. The iterator throws, but the server does not receive a response close event.RUN_ERRORfrom aGenerationClientfetcher. The client reports the error and closes the response.Easy test path. The branch adds 22 cleanup unit tests and two regressions in
testing/e2e/tests/error-handling.spec.ts. The browser regression uses native fetch against an open HTTP response. The generation regression exercises the publicGenerationClientwith native fetch.Risk / rollback
A custom stream can now observe cancellation when its consumer stops early. Closed streams keep their existing behavior. Failed or pending cancellation cannot hide the original error or delay iterator completion. Revert this PR to restore the previous cleanup behavior.
This change was researched and prepared through AI-driven maintenance, with source review and local checks.
Co-authored by Codex
Summary by CodeRabbit
Bug Fixes
[DONE], or generation reports an error. Cancellation failures do not replace the original error or delay error and loading-state updates.Documentation