Skip to content

Conversation

@felixweinberger
Copy link
Contributor

@felixweinberger felixweinberger commented Nov 27, 2025

Summary

  • Fix crash when the ReadableStream controller is closed twice (I'm not sure yet why this is happening twice - uncertain if it's because SDK or something in the inspector?)

Motivation and Context

When the server disconnects (as allowed by SEP-1699 for SSE polling), the fetch response body stream gets cancelled. This closes the ReadableStream controller, but the underlying Node PassThrough stream still emits its "end" event, which then tries to close the already-closed controller.

The fix:

  • Tracks controller state with a closed flag
  • Guards all controller operations
  • Adds a cancel() callback to properly clean up the Node stream

With this fix + JSON fix in modelcontextprotocol/typescript-sdk#1184 the inspector works again:

CleanShot 2025-11-27 at 21 54 11

How Has This Been Tested?

Tested locally with the everything-server via Streamable HTTP transport.

Breaking Changes

None

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling

Guard against calling controller.close() when the stream has already
been closed by an external cancellation. This can happen when the SDK
cancels the stream before the underlying Node stream emits its 'end'
event.

- Add `closed` flag to track controller state
- Guard all controller operations with closed check
- Add `cancel()` callback to handle external cancellation
- Properly destroy the Node stream on cancellation
@felixweinberger felixweinberger marked this pull request as draft November 27, 2025 21:58
Base automatically changed from fix/zod4-compat to main November 27, 2025 22:21
@github-actions
Copy link

🎭 Playwright E2E Test Results

✅  24 passed

Details

24 tests across 3 suites
 34.6 seconds
 573bda8
ℹ️  Test Environment: Ubuntu Latest, Node.js v24.11.1
Browsers: Chromium, Firefox

📊 View Detailed HTML Report (download artifacts)

Copy link
Member

@cliffhall cliffhall left a comment

Choose a reason for hiding this comment

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

Seems like robust stream handling.

@felixweinberger felixweinberger marked this pull request as ready for review November 28, 2025 18:35
@felixweinberger felixweinberger merged commit 6e4a52b into main Nov 28, 2025
8 checks passed
@felixweinberger felixweinberger deleted the fweinberger/fix-stream-double-close branch November 28, 2025 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants