Skip to content

Commit 78bae74

Browse files
fix(server): call onerror callback for all transport errors (#1433)
Co-authored-by: Felix Weinberger <fweinberger@anthropic.com> Co-authored-by: Felix Weinberger <3823880+felixweinberger@users.noreply.github.com>
1 parent 0ed7237 commit 78bae74

File tree

3 files changed

+218
-9
lines changed

3 files changed

+218
-9
lines changed
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@modelcontextprotocol/server': patch
3+
---
4+
5+
Fix transport errors being silently swallowed by adding missing `onerror` callback invocations before all `createJsonErrorResponse` calls in `WebStandardStreamableHTTPServerTransport`. This ensures errors like parse failures, invalid headers, and session validation errors are properly reported via the `onerror` callback.

packages/server/src/server/streamableHttp.ts

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,7 @@ export class WebStandardStreamableHTTPServerTransport implements Transport {
404404
// The client MUST include an Accept header, listing text/event-stream as a supported content type.
405405
const acceptHeader = req.headers.get('accept');
406406
if (!acceptHeader?.includes('text/event-stream')) {
407+
this.onerror?.(new Error('Not Acceptable: Client must accept text/event-stream'));
407408
return this.createJsonErrorResponse(406, -32_000, 'Not Acceptable: Client must accept text/event-stream');
408409
}
409410

@@ -430,6 +431,7 @@ export class WebStandardStreamableHTTPServerTransport implements Transport {
430431
// Check if there's already an active standalone SSE stream for this session
431432
if (this._streamMapping.get(this._standaloneSseStreamId) !== undefined) {
432433
// Only one GET SSE stream is allowed per session
434+
this.onerror?.(new Error('Conflict: Only one SSE stream is allowed per session'));
433435
return this.createJsonErrorResponse(409, -32_000, 'Conflict: Only one SSE stream is allowed per session');
434436
}
435437

@@ -481,6 +483,7 @@ export class WebStandardStreamableHTTPServerTransport implements Transport {
481483
*/
482484
private async replayEvents(lastEventId: string): Promise<Response> {
483485
if (!this._eventStore) {
486+
this.onerror?.(new Error('Event store not configured'));
484487
return this.createJsonErrorResponse(400, -32_000, 'Event store not configured');
485488
}
486489

@@ -491,11 +494,13 @@ export class WebStandardStreamableHTTPServerTransport implements Transport {
491494
streamId = await this._eventStore.getStreamIdForEventId(lastEventId);
492495

493496
if (!streamId) {
497+
this.onerror?.(new Error('Invalid event ID format'));
494498
return this.createJsonErrorResponse(400, -32_000, 'Invalid event ID format');
495499
}
496500

497501
// Check conflict with the SAME streamId we'll use for mapping
498502
if (this._streamMapping.get(streamId) !== undefined) {
503+
this.onerror?.(new Error('Conflict: Stream already has an active connection'));
499504
return this.createJsonErrorResponse(409, -32_000, 'Conflict: Stream already has an active connection');
500505
}
501506
}
@@ -529,7 +534,6 @@ export class WebStandardStreamableHTTPServerTransport implements Transport {
529534
send: async (eventId: string, message: JSONRPCMessage) => {
530535
const success = this.writeSSEEvent(streamController!, encoder, message, eventId);
531536
if (!success) {
532-
this.onerror?.(new Error('Failed replay events'));
533537
try {
534538
streamController!.close();
535539
} catch {
@@ -577,7 +581,8 @@ export class WebStandardStreamableHTTPServerTransport implements Transport {
577581
eventData += `data: ${JSON.stringify(message)}\n\n`;
578582
controller.enqueue(encoder.encode(eventData));
579583
return true;
580-
} catch {
584+
} catch (error) {
585+
this.onerror?.(error as Error);
581586
return false;
582587
}
583588
}
@@ -586,6 +591,7 @@ export class WebStandardStreamableHTTPServerTransport implements Transport {
586591
* Handles unsupported requests (`PUT`, `PATCH`, etc.)
587592
*/
588593
private handleUnsupportedRequest(): Response {
594+
this.onerror?.(new Error('Method not allowed.'));
589595
return Response.json(
590596
{
591597
jsonrpc: '2.0',
@@ -614,6 +620,7 @@ export class WebStandardStreamableHTTPServerTransport implements Transport {
614620
const acceptHeader = req.headers.get('accept');
615621
// The client MUST include an Accept header, listing both application/json and text/event-stream as supported content types.
616622
if (!acceptHeader?.includes('application/json') || !acceptHeader.includes('text/event-stream')) {
623+
this.onerror?.(new Error('Not Acceptable: Client must accept both application/json and text/event-stream'));
617624
return this.createJsonErrorResponse(
618625
406,
619626
-32_000,
@@ -623,6 +630,7 @@ export class WebStandardStreamableHTTPServerTransport implements Transport {
623630

624631
const ct = req.headers.get('content-type');
625632
if (!ct || !ct.includes('application/json')) {
633+
this.onerror?.(new Error('Unsupported Media Type: Content-Type must be application/json'));
626634
return this.createJsonErrorResponse(415, -32_000, 'Unsupported Media Type: Content-Type must be application/json');
627635
}
628636

@@ -635,7 +643,8 @@ export class WebStandardStreamableHTTPServerTransport implements Transport {
635643
if (options?.parsedBody === undefined) {
636644
try {
637645
rawMessage = await req.json();
638-
} catch {
646+
} catch (error) {
647+
this.onerror?.(error as Error);
639648
return this.createJsonErrorResponse(400, -32_700, 'Parse error: Invalid JSON');
640649
}
641650
} else {
@@ -649,7 +658,8 @@ export class WebStandardStreamableHTTPServerTransport implements Transport {
649658
messages = Array.isArray(rawMessage)
650659
? rawMessage.map(msg => JSONRPCMessageSchema.parse(msg))
651660
: [JSONRPCMessageSchema.parse(rawMessage)];
652-
} catch {
661+
} catch (error) {
662+
this.onerror?.(error as Error);
653663
return this.createJsonErrorResponse(400, -32_700, 'Parse error: Invalid JSON-RPC message');
654664
}
655665

@@ -660,9 +670,11 @@ export class WebStandardStreamableHTTPServerTransport implements Transport {
660670
// If it's a server with session management and the session ID is already set we should reject the request
661671
// to avoid re-initialization.
662672
if (this._initialized && this.sessionId !== undefined) {
673+
this.onerror?.(new Error('Invalid Request: Server already initialized'));
663674
return this.createJsonErrorResponse(400, -32_600, 'Invalid Request: Server already initialized');
664675
}
665676
if (messages.length > 1) {
677+
this.onerror?.(new Error('Invalid Request: Only one initialization request is allowed'));
666678
return this.createJsonErrorResponse(400, -32_600, 'Invalid Request: Only one initialization request is allowed');
667679
}
668680
this.sessionId = this.sessionIdGenerator?.();
@@ -842,18 +854,21 @@ export class WebStandardStreamableHTTPServerTransport implements Transport {
842854
}
843855
if (!this._initialized) {
844856
// If the server has not been initialized yet, reject all requests
857+
this.onerror?.(new Error('Bad Request: Server not initialized'));
845858
return this.createJsonErrorResponse(400, -32_000, 'Bad Request: Server not initialized');
846859
}
847860

848861
const sessionId = req.headers.get('mcp-session-id');
849862

850863
if (!sessionId) {
851864
// Non-initialization requests without a session ID should return 400 Bad Request
865+
this.onerror?.(new Error('Bad Request: Mcp-Session-Id header is required'));
852866
return this.createJsonErrorResponse(400, -32_000, 'Bad Request: Mcp-Session-Id header is required');
853867
}
854868

855869
if (sessionId !== this.sessionId) {
856870
// Reject requests with invalid session ID with 404 Not Found
871+
this.onerror?.(new Error('Session not found'));
857872
return this.createJsonErrorResponse(404, -32_001, 'Session not found');
858873
}
859874

@@ -877,11 +892,9 @@ export class WebStandardStreamableHTTPServerTransport implements Transport {
877892
const protocolVersion = req.headers.get('mcp-protocol-version');
878893

879894
if (protocolVersion !== null && !this._supportedProtocolVersions.includes(protocolVersion)) {
880-
return this.createJsonErrorResponse(
881-
400,
882-
-32_000,
883-
`Bad Request: Unsupported protocol version: ${protocolVersion} (supported versions: ${this._supportedProtocolVersions.join(', ')})`
884-
);
895+
const error = `Bad Request: Unsupported protocol version: ${protocolVersion} (supported versions: ${this._supportedProtocolVersions.join(', ')})`;
896+
this.onerror?.(new Error(error));
897+
return this.createJsonErrorResponse(400, -32_000, error);
885898
}
886899
return undefined;
887900
}

packages/server/test/server/streamableHttp.test.ts

Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -765,4 +765,195 @@ describe('Zod v4', () => {
765765
await expect(transport.start()).rejects.toThrow('Transport already started');
766766
});
767767
});
768+
769+
describe('HTTPServerTransport - onerror callback', () => {
770+
let transport: WebStandardStreamableHTTPServerTransport;
771+
let mcpServer: McpServer;
772+
let errors: Error[];
773+
774+
beforeEach(async () => {
775+
errors = [];
776+
mcpServer = new McpServer({ name: 'test-server', version: '1.0.0' }, { capabilities: {} });
777+
778+
transport = new WebStandardStreamableHTTPServerTransport({
779+
sessionIdGenerator: () => randomUUID()
780+
});
781+
782+
transport.onerror = err => errors.push(err);
783+
784+
await mcpServer.connect(transport);
785+
});
786+
787+
afterEach(async () => {
788+
await transport.close();
789+
});
790+
791+
async function initializeServer(): Promise<string> {
792+
const request = createRequest('POST', TEST_MESSAGES.initialize);
793+
const response = await transport.handleRequest(request);
794+
return response.headers.get('mcp-session-id') as string;
795+
}
796+
797+
it('should call onerror for invalid JSON', async () => {
798+
const request = new Request('http://localhost/mcp', {
799+
method: 'POST',
800+
headers: {
801+
Accept: 'application/json, text/event-stream',
802+
'Content-Type': 'application/json'
803+
},
804+
body: 'not valid json'
805+
});
806+
807+
const response = await transport.handleRequest(request);
808+
809+
expect(response.status).toBe(400);
810+
expect(errors.length).toBeGreaterThan(0);
811+
const error = errors[0];
812+
expect(error).toBeDefined();
813+
expect(error).toBeInstanceOf(SyntaxError);
814+
});
815+
816+
it('should call onerror for invalid JSON-RPC message', async () => {
817+
const request = new Request('http://localhost/mcp', {
818+
method: 'POST',
819+
headers: {
820+
Accept: 'application/json, text/event-stream',
821+
'Content-Type': 'application/json'
822+
},
823+
body: JSON.stringify({ not: 'valid jsonrpc' })
824+
});
825+
826+
const response = await transport.handleRequest(request);
827+
828+
expect(response.status).toBe(400);
829+
expect(errors.length).toBeGreaterThan(0);
830+
const error = errors[0];
831+
expect(error).toBeDefined();
832+
expect(error?.name).toBe('ZodError');
833+
});
834+
835+
it('should call onerror for missing Accept header on POST', async () => {
836+
const request = createRequest('POST', TEST_MESSAGES.initialize, { accept: 'application/json' });
837+
838+
const response = await transport.handleRequest(request);
839+
840+
expect(response.status).toBe(406);
841+
expect(errors.length).toBeGreaterThan(0);
842+
const error = errors[0];
843+
expect(error).toBeDefined();
844+
expect(error?.message).toContain('Not Acceptable');
845+
});
846+
847+
it('should call onerror for unsupported Content-Type', async () => {
848+
const request = new Request('http://localhost/mcp', {
849+
method: 'POST',
850+
headers: {
851+
Accept: 'application/json, text/event-stream',
852+
'Content-Type': 'text/plain'
853+
},
854+
body: JSON.stringify(TEST_MESSAGES.initialize)
855+
});
856+
857+
const response = await transport.handleRequest(request);
858+
859+
expect(response.status).toBe(415);
860+
expect(errors.length).toBeGreaterThan(0);
861+
const error = errors[0];
862+
expect(error).toBeDefined();
863+
expect(error?.message).toContain('Unsupported Media Type');
864+
});
865+
866+
it('should call onerror for server not initialized', async () => {
867+
const request = createRequest('POST', TEST_MESSAGES.toolsList);
868+
869+
const response = await transport.handleRequest(request);
870+
871+
expect(response.status).toBe(400);
872+
expect(errors.length).toBeGreaterThan(0);
873+
const error = errors[0];
874+
expect(error).toBeDefined();
875+
expect(error?.message).toContain('Server not initialized');
876+
});
877+
878+
it('should call onerror for invalid session ID', async () => {
879+
await initializeServer();
880+
881+
const request = createRequest('POST', TEST_MESSAGES.toolsList, { sessionId: 'invalid-session-id' });
882+
883+
const response = await transport.handleRequest(request);
884+
885+
expect(response.status).toBe(404);
886+
expect(errors.length).toBeGreaterThan(0);
887+
const error = errors[0];
888+
expect(error).toBeDefined();
889+
expect(error?.message).toContain('Session not found');
890+
});
891+
892+
it('should call onerror for re-initialization attempt', async () => {
893+
await initializeServer();
894+
895+
const request = createRequest('POST', TEST_MESSAGES.initialize);
896+
897+
const response = await transport.handleRequest(request);
898+
899+
expect(response.status).toBe(400);
900+
expect(errors.length).toBeGreaterThan(0);
901+
const error = errors[0];
902+
expect(error).toBeDefined();
903+
expect(error?.message).toContain('Server already initialized');
904+
});
905+
906+
it('should call onerror for GET without Accept header', async () => {
907+
const sessionId = await initializeServer();
908+
909+
const request = createRequest('GET', undefined, { sessionId, accept: 'application/json' });
910+
911+
const response = await transport.handleRequest(request);
912+
913+
expect(response.status).toBe(406);
914+
expect(errors.length).toBeGreaterThan(0);
915+
const error = errors[0];
916+
expect(error).toBeDefined();
917+
expect(error?.message).toContain('Not Acceptable');
918+
});
919+
920+
it('should call onerror for concurrent SSE streams', async () => {
921+
const sessionId = await initializeServer();
922+
923+
const request1 = createRequest('GET', undefined, { sessionId });
924+
await transport.handleRequest(request1);
925+
926+
const request2 = createRequest('GET', undefined, { sessionId });
927+
const response2 = await transport.handleRequest(request2);
928+
929+
expect(response2.status).toBe(409);
930+
expect(errors.length).toBeGreaterThan(0);
931+
const error = errors[0];
932+
expect(error).toBeDefined();
933+
expect(error?.message).toContain('Conflict');
934+
});
935+
936+
it('should call onerror for unsupported protocol version', async () => {
937+
const sessionId = await initializeServer();
938+
939+
const request = new Request('http://localhost/mcp', {
940+
method: 'POST',
941+
headers: {
942+
'Content-Type': 'application/json',
943+
Accept: 'application/json, text/event-stream',
944+
'mcp-session-id': sessionId,
945+
'mcp-protocol-version': 'unsupported-version'
946+
},
947+
body: JSON.stringify(TEST_MESSAGES.toolsList)
948+
});
949+
950+
const response = await transport.handleRequest(request);
951+
952+
expect(response.status).toBe(400);
953+
expect(errors.length).toBeGreaterThan(0);
954+
const error = errors[0];
955+
expect(error).toBeDefined();
956+
expect(error?.message).toContain('Unsupported protocol version');
957+
});
958+
});
768959
});

0 commit comments

Comments
 (0)