Skip to content

Commit 02abb9c

Browse files
KKonstantinovKKonstantinov
authored
Merge branch 'main' into fweinberger/drop-zod-peer-dep
2 parents c1f68c7 + 89fb094 commit 02abb9c

12 files changed

Lines changed: 231 additions & 69 deletions

‎.changeset/fix-abort-listener-leak.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@modelcontextprotocol/core': patch
3+
---
4+
5+
Consolidate per-request cleanup in `_requestWithSchema` into a single `.finally()` block. This fixes an abort signal listener leak (listeners accumulated when a caller reused one `AbortSignal` across requests) and two cases where `_responseHandlers` entries leaked on send-failure paths.

‎.changeset/fix-session-status-codes.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@modelcontextprotocol/examples-server': patch
3+
---
4+
5+
Example servers now return HTTP 404 (not 400) when a request includes an unknown session ID, so clients can correctly detect they need to start a new session. Requests missing a session ID entirely still return 400.

‎examples/server/src/elicitationFormExample.ts‎

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -364,14 +364,17 @@ async function main() {
364364

365365
await transport.handleRequest(req, res, req.body);
366366
return;
367+
} else if (sessionId) {
368+
res.status(404).json({
369+
jsonrpc: '2.0',
370+
error: { code: -32_001, message: 'Session not found' },
371+
id: null
372+
});
373+
return;
367374
} else {
368-
// Invalid request - no session ID or not initialization request
369375
res.status(400).json({
370376
jsonrpc: '2.0',
371-
error: {
372-
code: -32_000,
373-
message: 'Bad Request: No valid session ID provided'
374-
},
377+
error: { code: -32_000, message: 'Bad Request: Session ID required' },
375378
id: null
376379
});
377380
return;
@@ -399,8 +402,12 @@ async function main() {
399402
// Handle GET requests for SSE streams
400403
const mcpGetHandler = async (req: Request, res: Response) => {
401404
const sessionId = req.headers['mcp-session-id'] as string | undefined;
402-
if (!sessionId || !transports[sessionId]) {
403-
res.status(400).send('Invalid or missing session ID');
405+
if (!sessionId) {
406+
res.status(400).send('Missing session ID');
407+
return;
408+
}
409+
if (!transports[sessionId]) {
410+
res.status(404).send('Session not found');
404411
return;
405412
}
406413

@@ -414,8 +421,12 @@ async function main() {
414421
// Handle DELETE requests for session termination
415422
const mcpDeleteHandler = async (req: Request, res: Response) => {
416423
const sessionId = req.headers['mcp-session-id'] as string | undefined;
417-
if (!sessionId || !transports[sessionId]) {
418-
res.status(400).send('Invalid or missing session ID');
424+
if (!sessionId) {
425+
res.status(400).send('Missing session ID');
426+
return;
427+
}
428+
if (!transports[sessionId]) {
429+
res.status(404).send('Session not found');
419430
return;
420431
}
421432

‎examples/server/src/elicitationUrlExample.ts‎

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -606,14 +606,17 @@ const mcpPostHandler = async (req: Request, res: Response) => {
606606

607607
await transport.handleRequest(req, res, req.body);
608608
return; // Already handled
609+
} else if (sessionId) {
610+
res.status(404).json({
611+
jsonrpc: '2.0',
612+
error: { code: -32_001, message: 'Session not found' },
613+
id: null
614+
});
615+
return;
609616
} else {
610-
// Invalid request - no session ID or not initialization request
611617
res.status(400).json({
612618
jsonrpc: '2.0',
613-
error: {
614-
code: -32_000,
615-
message: 'Bad Request: No valid session ID provided'
616-
},
619+
error: { code: -32_000, message: 'Bad Request: Session ID required' },
617620
id: null
618621
});
619622
return;
@@ -643,8 +646,12 @@ app.post('/mcp', authMiddleware, mcpPostHandler);
643646
// Handle GET requests for SSE streams (using built-in support from StreamableHTTP)
644647
const mcpGetHandler = async (req: Request, res: Response) => {
645648
const sessionId = req.headers['mcp-session-id'] as string | undefined;
646-
if (!sessionId || !transports[sessionId]) {
647-
res.status(400).send('Invalid or missing session ID');
649+
if (!sessionId) {
650+
res.status(400).send('Missing session ID');
651+
return;
652+
}
653+
if (!transports[sessionId]) {
654+
res.status(404).send('Session not found');
648655
return;
649656
}
650657

@@ -682,8 +689,12 @@ app.get('/mcp', authMiddleware, mcpGetHandler);
682689
// Handle DELETE requests for session termination (according to MCP spec)
683690
const mcpDeleteHandler = async (req: Request, res: Response) => {
684691
const sessionId = req.headers['mcp-session-id'] as string | undefined;
685-
if (!sessionId || !transports[sessionId]) {
686-
res.status(400).send('Invalid or missing session ID');
692+
if (!sessionId) {
693+
res.status(400).send('Missing session ID');
694+
return;
695+
}
696+
if (!transports[sessionId]) {
697+
res.status(404).send('Session not found');
687698
return;
688699
}
689700

‎examples/server/src/jsonResponseStreamableHttp.ts‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -110,14 +110,17 @@ app.post('/mcp', async (req: Request, res: Response) => {
110110
await server.connect(transport);
111111
await transport.handleRequest(req, res, req.body);
112112
return; // Already handled
113+
} else if (sessionId) {
114+
res.status(404).json({
115+
jsonrpc: '2.0',
116+
error: { code: -32_001, message: 'Session not found' },
117+
id: null
118+
});
119+
return;
113120
} else {
114-
// Invalid request - no session ID or not initialization request
115121
res.status(400).json({
116122
jsonrpc: '2.0',
117-
error: {
118-
code: -32_000,
119-
message: 'Bad Request: No valid session ID provided'
120-
},
123+
error: { code: -32_000, message: 'Bad Request: Session ID required' },
121124
id: null
122125
});
123126
return;

‎examples/server/src/simpleStreamableHttp.ts‎

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -689,14 +689,17 @@ const mcpPostHandler = async (req: Request, res: Response) => {
689689

690690
await transport.handleRequest(req, res, req.body);
691691
return; // Already handled
692+
} else if (sessionId) {
693+
res.status(404).json({
694+
jsonrpc: '2.0',
695+
error: { code: -32_001, message: 'Session not found' },
696+
id: null
697+
});
698+
return;
692699
} else {
693-
// Invalid request - no session ID or not initialization request
694700
res.status(400).json({
695701
jsonrpc: '2.0',
696-
error: {
697-
code: -32_000,
698-
message: 'Bad Request: No valid session ID provided'
699-
},
702+
error: { code: -32_000, message: 'Bad Request: Session ID required' },
700703
id: null
701704
});
702705
return;
@@ -730,8 +733,12 @@ if (useOAuth && authMiddleware) {
730733
// Handle GET requests for SSE streams (using built-in support from StreamableHTTP)
731734
const mcpGetHandler = async (req: Request, res: Response) => {
732735
const sessionId = req.headers['mcp-session-id'] as string | undefined;
733-
if (!sessionId || !transports[sessionId]) {
734-
res.status(400).send('Invalid or missing session ID');
736+
if (!sessionId) {
737+
res.status(400).send('Missing session ID');
738+
return;
739+
}
740+
if (!transports[sessionId]) {
741+
res.status(404).send('Session not found');
735742
return;
736743
}
737744

@@ -761,8 +768,12 @@ if (useOAuth && authMiddleware) {
761768
// Handle DELETE requests for session termination (according to MCP spec)
762769
const mcpDeleteHandler = async (req: Request, res: Response) => {
763770
const sessionId = req.headers['mcp-session-id'] as string | undefined;
764-
if (!sessionId || !transports[sessionId]) {
765-
res.status(400).send('Invalid or missing session ID');
771+
if (!sessionId) {
772+
res.status(400).send('Missing session ID');
773+
return;
774+
}
775+
if (!transports[sessionId]) {
776+
res.status(404).send('Session not found');
766777
return;
767778
}
768779

‎examples/server/src/simpleTaskInteractive.ts‎

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -670,10 +670,17 @@ app.post('/mcp', async (req: Request, res: Response) => {
670670
await server.connect(transport);
671671
await transport.handleRequest(req, res, req.body);
672672
return;
673+
} else if (sessionId) {
674+
res.status(404).json({
675+
jsonrpc: '2.0',
676+
error: { code: -32_001, message: 'Session not found' },
677+
id: null
678+
});
679+
return;
673680
} else {
674681
res.status(400).json({
675682
jsonrpc: '2.0',
676-
error: { code: -32_000, message: 'Bad Request: No valid session ID' },
683+
error: { code: -32_000, message: 'Bad Request: Session ID required' },
677684
id: null
678685
});
679686
return;
@@ -695,8 +702,12 @@ app.post('/mcp', async (req: Request, res: Response) => {
695702
// Handle GET requests for SSE streams
696703
app.get('/mcp', async (req: Request, res: Response) => {
697704
const sessionId = req.headers['mcp-session-id'] as string | undefined;
698-
if (!sessionId || !transports[sessionId]) {
699-
res.status(400).send('Invalid or missing session ID');
705+
if (!sessionId) {
706+
res.status(400).send('Missing session ID');
707+
return;
708+
}
709+
if (!transports[sessionId]) {
710+
res.status(404).send('Session not found');
700711
return;
701712
}
702713

@@ -707,8 +718,12 @@ app.get('/mcp', async (req: Request, res: Response) => {
707718
// Handle DELETE requests for session termination
708719
app.delete('/mcp', async (req: Request, res: Response) => {
709720
const sessionId = req.headers['mcp-session-id'] as string | undefined;
710-
if (!sessionId || !transports[sessionId]) {
711-
res.status(400).send('Invalid or missing session ID');
721+
if (!sessionId) {
722+
res.status(400).send('Missing session ID');
723+
return;
724+
}
725+
if (!transports[sessionId]) {
726+
res.status(404).send('Session not found');
712727
return;
713728
}
714729

‎examples/server/src/standaloneSseWithGetStreamableHttp.ts‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -86,14 +86,17 @@ app.post('/mcp', async (req: Request, res: Response) => {
8686
// Handle the request - the onsessioninitialized callback will store the transport
8787
await transport.handleRequest(req, res, req.body);
8888
return; // Already handled
89+
} else if (sessionId) {
90+
res.status(404).json({
91+
jsonrpc: '2.0',
92+
error: { code: -32_001, message: 'Session not found' },
93+
id: null
94+
});
95+
return;
8996
} else {
90-
// Invalid request - no session ID or not initialization request
9197
res.status(400).json({
9298
jsonrpc: '2.0',
93-
error: {
94-
code: -32_000,
95-
message: 'Bad Request: No valid session ID provided'
96-
},
99+
error: { code: -32_000, message: 'Bad Request: Session ID required' },
97100
id: null
98101
});
99102
return;
@@ -119,8 +122,12 @@ app.post('/mcp', async (req: Request, res: Response) => {
119122
// Handle GET requests for SSE streams (now using built-in support from StreamableHTTP)
120123
app.get('/mcp', async (req: Request, res: Response) => {
121124
const sessionId = req.headers['mcp-session-id'] as string | undefined;
122-
if (!sessionId || !transports[sessionId]) {
123-
res.status(400).send('Invalid or missing session ID');
125+
if (!sessionId) {
126+
res.status(400).send('Missing session ID');
127+
return;
128+
}
129+
if (!transports[sessionId]) {
130+
res.status(404).send('Session not found');
124131
return;
125132
}
126133

‎packages/core/src/shared/protocol.ts‎

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -800,6 +800,9 @@ export abstract class Protocol<ContextT extends BaseContext> {
800800
): Promise<SchemaOutput<T>> {
801801
const { relatedRequestId, resumptionToken, onresumptiontoken } = options ?? {};
802802

803+
let onAbort: (() => void) | undefined;
804+
let cleanupMessageId: number | undefined;
805+
803806
// Send the request
804807
return new Promise<SchemaOutput<T>>((resolve, reject) => {
805808
const earlyReject = (error: unknown) => {
@@ -823,6 +826,7 @@ export abstract class Protocol<ContextT extends BaseContext> {
823826
options?.signal?.throwIfAborted();
824827

825828
const messageId = this._requestMessageId++;
829+
cleanupMessageId = messageId;
826830
const jsonrpcRequest: JSONRPCRequest = {
827831
...request,
828832
jsonrpc: '2.0',
@@ -841,9 +845,7 @@ export abstract class Protocol<ContextT extends BaseContext> {
841845
}
842846

843847
const cancel = (reason: unknown) => {
844-
this._responseHandlers.delete(messageId);
845848
this._progressHandlers.delete(messageId);
846-
this._cleanupTimeout(messageId);
847849

848850
this._transport
849851
?.send(
@@ -885,9 +887,8 @@ export abstract class Protocol<ContextT extends BaseContext> {
885887
}
886888
});
887889

888-
options?.signal?.addEventListener('abort', () => {
889-
cancel(options?.signal?.reason);
890-
});
890+
onAbort = () => cancel(options?.signal?.reason);
891+
options?.signal?.addEventListener('abort', onAbort, { once: true });
891892

892893
const timeout = options?.timeout ?? DEFAULT_REQUEST_TIMEOUT_MSEC;
893894
const timeoutHandler = () => cancel(new SdkError(SdkErrorCode.RequestTimeout, 'Request timed out', { timeout }));
@@ -907,27 +908,38 @@ export abstract class Protocol<ContextT extends BaseContext> {
907908
let outboundQueued = false;
908909
try {
909910
const taskResult = this._taskManager.processOutboundRequest(jsonrpcRequest, options, messageId, responseHandler, error => {
910-
this._cleanupTimeout(messageId);
911+
this._progressHandlers.delete(messageId);
911912
reject(error);
912913
});
913914
if (taskResult.queued) {
914915
outboundQueued = true;
915916
}
916917
} catch (error) {
917-
this._responseHandlers.delete(messageId);
918918
this._progressHandlers.delete(messageId);
919-
this._cleanupTimeout(messageId);
920919
reject(error);
921920
return;
922921
}
923922

924923
if (!outboundQueued) {
925924
// No related task or no module - send through transport normally
926925
this._transport.send(jsonrpcRequest, { relatedRequestId, resumptionToken, onresumptiontoken }).catch(error => {
927-
this._cleanupTimeout(messageId);
926+
this._progressHandlers.delete(messageId);
928927
reject(error);
929928
});
930929
}
930+
}).finally(() => {
931+
// Per-request cleanup that must run on every exit path. Consolidated
932+
// here so new exit paths added to the promise body can't forget it.
933+
// _progressHandlers is NOT cleaned up here: _onresponse deletes it
934+
// conditionally (preserveProgress for task flows), and error paths
935+
// above delete it inline since no task exists in those cases.
936+
if (onAbort) {
937+
options?.signal?.removeEventListener('abort', onAbort);
938+
}
939+
if (cleanupMessageId !== undefined) {
940+
this._responseHandlers.delete(cleanupMessageId);
941+
this._cleanupTimeout(cleanupMessageId);
942+
}
931943
});
932944
}
933945

0 commit comments

Comments
 (0)