Skip to content

mcp: handle HTTP stream cancellation during disposal - #336921

Merged
Dmitriy Vasyura (dmitrivMS) merged 1 commit into
mainfrom
fix/mcp-http-abort-cleanup
Sep 20, 2026
Merged

Dmitriy Vasyura (dmitrivMS) merged 1 commit into
mainfrom
fix/mcp-http-abort-cleanup

Conversation

@dmitrivMS

@dmitrivMS Dmitriy Vasyura (dmitrivMS) commented Sep 20, 2026

Copy link
Copy Markdown
Collaborator

Stopping or disposing an HTTP MCP server can abort a pending SSE read. The reader is already errored, but the cleanup path called reader.cancel() and discarded the rejected promise, surfacing an unhandled AbortError during otherwise normal teardown.

  • Skip redundant reader cancellation once disposal has already aborted the fetch.
  • Await cancellation for active read failures, keeping cleanup in the handled promise chain and genuine failures visible.
  • Add a disposal regression and an active-read-error control in the existing MCP host test suite.

Disposing an MCP HTTP transport already aborts its fetch response. Cancelling the errored reader again creates a rejected promise that was previously discarded, surfacing an unhandled AbortError during normal stop/disposal.

Skip that redundant cancellation after disposal and await cancellation for active read failures. Add a failing-first disposal regression and a control that keeps genuine stream failures visible.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings September 20, 2026 15:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The focused cleanup fix is consistent with stream semantics and is covered by targeted regression tests.

Review effort: Balanced
Findings: None

What changed in this PR

Prevents unhandled HTTP MCP stream cancellation rejections during normal teardown.

Changes:

  • Skips redundant reader cancellation after disposal.
  • Awaits cancellation for active stream failures.
  • Adds regression and control tests for both paths.
File Description
src/​vs/​workbench/​api/​common/​extHostMcp.ts Corrects SSE reader cleanup behavior.
src/​vs/​workbench/​api/​test/​common/​extHostMcp.test.ts Tests disposal and active-read failures.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@vs-code-engineering

Copy link
Copy Markdown
Contributor

📬 CODENOTIFY

The following users are being notified based on files changed in this PR:

TylerLeonhardt

Matched files:

  • src/vs/workbench/api/common/extHostMcp.ts

@dmitrivMS
Dmitriy Vasyura (dmitrivMS) merged commit 82a6aac into main Sep 20, 2026
54 of 56 checks passed
@dmitrivMS
Dmitriy Vasyura (dmitrivMS) deleted the fix/mcp-http-abort-cleanup branch September 20, 2026 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants