Skip to content

Conversation

@charlotte-zhuang
Copy link

Description

In src/everything/streamableHttp.ts

  • Move createServer() to inside the "New initialization request" block
  • Servers are now 1:1 with transports
  • Add serverCleanupHooks global variable so that servers are still cleaned up and closed like before

Server Details

  • Server: everything
  • Changes to: Streamable HTTP Express app

Motivation and Context

There's a bug with the streamable HTTP version of the "Everything" reference server where the server tries to notify the client before any transport is connected.

This change moves the createServer() call to when a new session is created so that a transport is immediately connected to the server like in the MCP TypeScript SDK readme.

How Has This Been Tested?

The bug can be reproduced by doing this

cd src/everything
npm install
npm run start:streamableHttp

After waiting a few seconds

> @modelcontextprotocol/server-everything@0.6.2 start:streamableHttp
> node dist/streamableHttp.js

Starting Streamable HTTP server...
MCP Streamable HTTP Server listening on port 3001
file:///Users/charlotte.zhuang/repos/mcp-servers/src/everything/node_modules/@modelcontextprotocol/sdk/dist/esm/shared/protocol.js:305
            throw new Error("Not connected");
                  ^

Error: Not connected
    at Server.notification (file:///Users/charlotte.zhuang/repos/mcp-servers/src/everything/node_modules/@modelcontextprotocol/sdk/dist/esm/shared/protocol.js:305:19)
    at Timeout._onTimeout (file:///Users/charlotte.zhuang/repos/mcp-servers/src/everything/dist/everything.js:118:20)
    at listOnTimeout (node:internal/timers:573:17)
    at process.processTimers (node:internal/timers:514:7)

Node.js v20.11.0
npm ERR! Lifecycle script `start:streamableHttp` failed with error: 
npm ERR! Error: command failed 
npm ERR!   in workspace: @modelcontextprotocol/server-everything@0.6.2 
npm ERR!   at location: /Users/charlotte.zhuang/repos/mcp-servers/src/everything 

With these changes, I verified that I could initialize multiple sessions and use tools as expected with the TypeScript Client.

> @modelcontextprotocol/server-everything@0.6.2 start:streamableHttp
> node dist/streamableHttp.js

Starting Streamable HTTP server...
MCP Streamable HTTP Server listening on port 8101
Received MCP POST request
Session initialized with ID: e81cfd8b-a26e-407a-9633-6090ebeed283
Received MCP POST request
Received MCP GET request
Establishing new SSE stream for session e81cfd8b-a26e-407a-9633-6090ebeed283
Received MCP POST request
Received MCP POST request
Received MCP POST request
Received MCP POST request
Session initialized with ID: fbbae356-19cd-461e-bcba-0aa0e033c98f
Received MCP POST request
Received MCP GET request
Establishing new SSE stream for session fbbae356-19cd-461e-bcba-0aa0e033c98f
Received MCP POST request
Received MCP POST request
Received MCP POST request
Received MCP POST request
Session initialized with ID: e044ae96-e616-4e81-8982-9a87f095edd8
Received MCP POST request
Received MCP GET request
Establishing new SSE stream for session e044ae96-e616-4e81-8982-9a87f095edd8
Received MCP POST request
Received MCP POST request
Received MCP POST request
Received MCP POST request
Received MCP POST request
Received MCP POST request
Received MCP POST request
Received MCP POST request
^CShutting down server...
Closing transport for session e81cfd8b-a26e-407a-9633-6090ebeed283
Closing transport for session fbbae356-19cd-461e-bcba-0aa0e033c98f
Closing transport for session e044ae96-e616-4e81-8982-9a87f095edd8
Server shutdown complete

Breaking Changes

None.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Protocol Documentation
  • My changes follows MCP security best practices
  • I have updated the server's README accordingly
  • I have tested this with an LLM client
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have documented all environment variables and configuration options

Additional context

It's a bit unexpected how this change cleans up servers via the array serverCleanupHooks while transports are cleaned up via the transports record, but I wasn't sure if there was a good way to map session ID to server + cleanup since the session ID gets created after the transport is is initialized.

Some alternatives I thought of

  • call cleanup and server.close from inside of the transport.onclose listener
  • change sessionIdGenerator to return a constant so that the server can be aware of the session ID it's handling

I didn't go with those alternatives since the only time transports are closed are on process interrupt and I'm not really sure how all these callbacks are supposed to interact with each other, but it would be nice to know if there's a "right" way to do this.

@olaservo
Copy link
Member

Thanks for the PR! I was just looking at @cliffhall 's everything server PR here: #1884

I think there might be some overlap with the types of issues we are dealing with here, especially if we want to use the cleanup approach here which I don't think is covered in the other PR. I tagged Cliff on this one to take a look at it.

@olaservo olaservo requested a review from cliffhall May 27, 2025 02:26
@cliffhall
Copy link
Member

cliffhall commented May 28, 2025

I think there might be some overlap with the types of issues we are dealing with here, especially if we want to use the cleanup approach here which I don't think is covered in the other PR. I tagged Cliff on this one to take a look at it.

Thanks for tagging this, @olaservo and for noticing the problem @charlotte-zhuang. All of this is covered by #1884

  • This timeout issue no longer happens in my PR, because the server creation is deferred until the endpoint is hit. At that point a transport is immediately connected, the server is no longer waiting for a connection, and thus doesn't timeout.

  • The cleanups all happen because the server is created in the same scope as its onclose handler (i.e., closure), so there is no need to push them individually onto a global array to be iterated later.

  • The servers are closed automatically when their transports are closed, which happens when a client disconnects.

@cliffhall
Copy link
Member

Just a note that we should close this one if/when #1884 is merged.

@charlotte-zhuang charlotte-zhuang deleted the fix-not-connected-error-in-everything-streamable-http-server branch June 13, 2025 17:55
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