Skip to content

Conversation

@felixweinberger
Copy link
Contributor

@felixweinberger felixweinberger commented Nov 28, 2025

Adds proper transport cleanup on disconnect.

Motivation and Context

After disconnect, pending SSE reconnection timeouts would fire and attempt to reconnect to a session that no longer exists, causing "Failed to reconnect SSE stream: Bad Request" errors.

Change

Call close() after terminateSession() in the DELETE handler:

  • terminateSession() notifies the server (sends DELETE request)
  • close() cleans up client-side resources (abort controller, pending reconnection timeouts)
  • Both are needed for a complete disconnect

How Has This Been Tested?

  • Tested connect/disconnect cycles with everything server
  • Verified no reconnection errors on disconnect
CleanShot 2025-11-28 at 19 39 10

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
  • I have added or updated documentation as needed

felixweinberger and others added 2 commits November 28, 2025 19:39
1. createWebReadableStream: Add closed flag and cancel() callback to
   prevent double-close crashes when SSE streams end. This fixes the
   "Controller is already closed" error that occurs when the SDK calls
   response.body.cancel() on 202 responses.

2. DELETE handler: Call serverTransport.close() after terminateSession()
   to properly cleanup the transport and cancel pending reconnections.
   - terminateSession() notifies the server (sends DELETE request)
   - close() cleans up client-side resources (abort controller, timeouts)
   - Both are needed for a complete disconnect
@github-actions
Copy link

github-actions bot commented Nov 28, 2025

🎭 Playwright E2E Test Results

✅  24 passed

Details

24 tests across 3 suites
 38.8 seconds
 3058b56
ℹ️  Test Environment: Ubuntu Latest, Node.js v24.11.1
Browsers: Chromium, Firefox

📊 View Detailed HTML Report (download artifacts)

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.

2 participants