Skip to content

Commit 68fb385

Browse files
review: reject stateless reuse before delegating to getRequestListener so the caller keeps control of the response
1 parent 6a3a4c4 commit 68fb385

3 files changed

Lines changed: 35 additions & 9 deletions

File tree

packages/middleware/node/src/streamableHttp.ts

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,13 @@ export class NodeStreamableHTTPServerTransport implements Transport {
196196
* @param parsedBody - Optional pre-parsed body from body-parser middleware
197197
*/
198198
async handleRequest(req: IncomingMessage & { auth?: AuthInfo }, res: ServerResponse, parsedBody?: unknown): Promise<void> {
199+
// Fail fast before delegating: once getRequestListener is invoked, a
200+
// rejected dispatch is committed to `res` as an empty 500 before the
201+
// rejection can surface, leaving the caller unable to shape its own
202+
// error response. Checking up front rejects with nothing written to
203+
// `res`. The check lives on the wrapped transport (single source).
204+
this._webStandardTransport.assertNotReused();
205+
199206
// Store context for this request to pass through auth and parsedBody
200207
// We need to intercept the request creation to attach this context
201208
const authInfo = req.auth;
@@ -223,14 +230,15 @@ export class NodeStreamableHTTPServerTransport implements Transport {
223230
// including proper SSE streaming support
224231
await handler(req, res);
225232

226-
// getRequestListener absorbs a rejected dispatch into a generic 500
227-
// response, which would swallow the wrapped transport's single-use
228-
// throw. Re-raise it so the documented behavior (reusing a stateless
229-
// transport across requests throws at the call site) holds for the
230-
// Node adapter too. Other dispatch errors keep the existing
231-
// 500-response behavior. Matched by code, not `instanceof`, so the
232-
// check also holds if bundling ever yields two copies of the error
233-
// class.
233+
// Backstop for concurrent calls racing the up-front check (the
234+
// single-use flag is set inside the wrapped handleRequest, after the
235+
// request conversion): getRequestListener absorbs a rejected dispatch
236+
// into a generic 500, which would swallow the single-use throw.
237+
// Re-raise it so the documented behavior (reusing a stateless
238+
// transport across requests throws at the call site) holds on this
239+
// path too. Other dispatch errors keep the existing 500-response
240+
// behavior. Matched by code, not `instanceof`, so the check also
241+
// holds if bundling ever yields two copies of the error class.
234242
if (dispatchError instanceof Error && (dispatchError as { code?: unknown }).code === SdkErrorCode.StatelessTransportReuse) {
235243
throw dispatchError;
236244
}

packages/middleware/node/test/streamableHttp.test.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1667,11 +1667,13 @@ describe('Zod v4', () => {
16671667
perRequestServers.push(mcpServer);
16681668

16691669
const callSiteErrors: Error[] = [];
1670+
const headersSentAtRejection: boolean[] = [];
16701671
const sharedServer = createServer(async (req, res) => {
16711672
try {
16721673
await transport.handleRequest(req, res);
16731674
} catch (error) {
16741675
callSiteErrors.push(error as Error);
1676+
headersSentAtRejection.push(res.headersSent);
16751677
if (!res.headersSent) res.writeHead(500);
16761678
res.end();
16771679
}
@@ -1686,6 +1688,10 @@ describe('Zod v4', () => {
16861688
expect(callSiteErrors).toHaveLength(1);
16871689
expect(callSiteErrors[0]!.message).toContain('Stateless transport cannot be reused across requests');
16881690
expect((callSiteErrors[0] as { code?: unknown }).code).toBe('STATELESS_TRANSPORT_REUSE');
1691+
// The rejection reached the caller before anything was
1692+
// committed to `res` — the caller keeps control of the
1693+
// response it sends for the rejected request.
1694+
expect(headersSentAtRejection).toEqual([false]);
16891695
} finally {
16901696
sharedServer.close();
16911697
}

packages/server/src/server/streamableHttp.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -374,7 +374,15 @@ export class WebStandardStreamableHTTPServerTransport implements Transport {
374374
* Handles an incoming HTTP request, whether `GET`, `POST`, or `DELETE`
375375
* Returns a `Response` object (Web Standard)
376376
*/
377-
async handleRequest(req: Request, options?: HandleRequestOptions): Promise<Response> {
377+
/**
378+
* Throws when a stateless transport has already handled its single
379+
* request exchange. This is the same check `handleRequest()` performs on
380+
* entry; exposed so wrapping adapters (e.g. the Node middleware) can fail
381+
* fast before committing anything to their own response stream.
382+
*
383+
* @internal
384+
*/
385+
assertNotReused(): void {
378386
// In stateless mode (no sessionIdGenerator), each request must use a fresh transport.
379387
// Reusing a stateless transport causes message ID collisions between clients.
380388
if (!this.sessionIdGenerator && this._hasHandledRequest) {
@@ -383,6 +391,10 @@ export class WebStandardStreamableHTTPServerTransport implements Transport {
383391
'Stateless transport cannot be reused across requests. Create a new transport per request.'
384392
);
385393
}
394+
}
395+
396+
async handleRequest(req: Request, options?: HandleRequestOptions): Promise<Response> {
397+
this.assertNotReused();
386398
this._hasHandledRequest = true;
387399

388400
// Validate request headers for DNS rebinding protection

0 commit comments

Comments
 (0)