diff --git a/.changeset/closed-transport-not-parse-error.md b/.changeset/closed-transport-not-parse-error.md new file mode 100644 index 0000000000..b37150d36b --- /dev/null +++ b/.changeset/closed-transport-not-parse-error.md @@ -0,0 +1,21 @@ +--- +'@modelcontextprotocol/server': patch +--- + +Stop reporting server-internal POST failures as client parse errors in +`WebStandardStreamableHTTPServerTransport`. + +`handleRequest()` re-checks `_closed` and answers `404 Session not found`, but +`writePrimingEvent()` awaits the user-supplied event store _after_ those checks. A +`close()` landing during that write left the priming event enqueueing onto an +already-closed controller, and the resulting `Invalid state` error fell into the POST +handler's catch-all — which answered `400 Parse error (-32700)`. The client was told to +fix a request body that was never the problem. + +`writePrimingEvent()` now returns early if the transport closed during the store write, +the POST handler answers `404 Session not found` at that suspension point like it does at +the two before it, and the catch-all maps to `500 Internal error (-32603)`. Genuine parse +failures are unaffected: invalid JSON and invalid JSON-RPC messages are still answered +`400 -32700` by the dedicated guards that precede it. + +Also applies to `NodeStreamableHTTPServerTransport`, which wraps this transport. diff --git a/packages/middleware/node/test/streamableHttp.test.ts b/packages/middleware/node/test/streamableHttp.test.ts index 140717c6bb..16537b8548 100644 --- a/packages/middleware/node/test/streamableHttp.test.ts +++ b/packages/middleware/node/test/streamableHttp.test.ts @@ -2661,9 +2661,12 @@ describe('Zod v4', () => { const tempServer = result.server; const tempUrl = result.baseUrl; - // Initialize should fail when callback throws + // Initialize should fail when callback throws. The failure is + // server-internal — the client's body parsed fine — so it is + // reported as -32603, not as a parse error. const initResponse = await sendPostRequest(tempUrl, TEST_MESSAGES.initialize); - expect(initResponse.status).toBe(400); + expect(initResponse.status).toBe(500); + expectErrorResponse(await initResponse.json(), -32_603, /Internal error/); // Clean up consoleErrorSpy.mockRestore(); diff --git a/packages/server/src/server/streamableHttp.ts b/packages/server/src/server/streamableHttp.ts index c0f48560a2..53d6c11a2a 100644 --- a/packages/server/src/server/streamableHttp.ts +++ b/packages/server/src/server/streamableHttp.ts @@ -445,6 +445,13 @@ export class WebStandardStreamableHTTPServerTransport implements Transport { const primingEventId = await this._eventStore.storeEvent(streamId, {} as JSONRPCMessage); + // storeEvent() is user-supplied and suspends here: a close() landing + // during the write has already closed this controller, so enqueueing + // would throw `Invalid state` out of the POST handler. + if (this._closed) { + return; + } + let primingEvent = `id: ${primingEventId}\ndata: \n\n`; if (this._retryInterval !== undefined) { primingEvent = `id: ${primingEventId}\nretry: ${this._retryInterval}\ndata: \n\n`; @@ -937,6 +944,14 @@ export class WebStandardStreamableHTTPServerTransport implements Transport { // Write priming event if event store is configured (after mapping is set up) await this.writePrimingEvent(streamController!, encoder, streamId, clientProtocolVersion); + // writePrimingEvent() awaits the user-supplied event store, so it is + // a third suspension point after the `_closed` checks above. Answer + // a close() that landed here the same way they do, rather than + // letting the stream teardown surface as a client-fault error. + if (this._closed) { + return this.createJsonErrorResponse(404, -32_001, 'Session not found'); + } + // handle each message for (const message of messages) { // Build closeSSEStream callback for requests when eventStore is configured @@ -964,9 +979,13 @@ export class WebStandardStreamableHTTPServerTransport implements Transport { } return new Response(readable, { status: 200, headers }); } catch (error) { - // return JSON-RPC formatted error + // Both parse failures (invalid JSON, invalid JSON-RPC) are already + // answered with -32700 above, so anything reaching here is a + // server-internal failure — stream setup, the event store, a + // handler. Reporting those as a client parse error tells the client + // to fix a body that was fine, so map them to -32603 instead. this.onerror?.(error as Error); - return this.createJsonErrorResponse(400, -32_700, 'Parse error', { data: String(error) }); + return this.createJsonErrorResponse(500, -32_603, 'Internal error', { data: String(error) }); } } diff --git a/packages/server/test/server/streamableHttp.test.ts b/packages/server/test/server/streamableHttp.test.ts index 9ec6baf46c..2edaf0b39f 100644 --- a/packages/server/test/server/streamableHttp.test.ts +++ b/packages/server/test/server/streamableHttp.test.ts @@ -1406,6 +1406,44 @@ describe('Zod v4', () => { expect(cleanupCalls).toEqual(['stream-1']); }); }); + + describe('close() racing the priming event', () => { + it('should answer Session not found, not a client parse error, when close() lands during the priming write', async () => { + let release: () => void = () => {}; + const gate = new Promise(resolve => (release = resolve)); + + const eventStore: EventStore = { + async storeEvent(streamId: StreamId): Promise { + await gate; + return `${streamId}_0`; + }, + async replayEventsAfter(): Promise { + return ''; + } + }; + + const transport = new WebStandardStreamableHTTPServerTransport({ sessionIdGenerator: undefined, eventStore }); + const errors: Error[] = []; + transport.onerror = error => errors.push(error); + transport.onmessage = () => {}; + + // The POST parks inside writePrimingEvent's storeEvent() call. + const pending = transport.handleRequest( + createRequest('POST', { jsonrpc: '2.0', id: 1, method: 'ping' } as JSONRPCMessage, { + extraHeaders: { 'mcp-protocol-version': '2025-11-25' } + }) + ); + await new Promise(resolve => setTimeout(resolve, 10)); + + await transport.close(); + release(); + + const res = await pending; + expect(res.status).toBe(404); + expectErrorResponse(await res.json(), -32_001, /Session not found/); + expect(errors).toEqual([]); + }); + }); }); describe('WebStandardStreamableHTTPServerTransport SSE keep-alive', () => {