diff --git a/.changeset/fix-silent-errors.md b/.changeset/fix-silent-errors.md new file mode 100644 index 000000000..7e4ef5bf3 --- /dev/null +++ b/.changeset/fix-silent-errors.md @@ -0,0 +1,5 @@ +--- +"@modelcontextprotocol/sdk": patch +--- + +Fix silent transport errors by ensuring onerror callback is invoked for all error responses in StreamableHTTPServerTransport diff --git a/packages/server/src/server/streamableHttp.ts b/packages/server/src/server/streamableHttp.ts index 74e689892..298f1296b 100644 --- a/packages/server/src/server/streamableHttp.ts +++ b/packages/server/src/server/streamableHttp.ts @@ -284,9 +284,10 @@ export class WebStandardStreamableHTTPServerTransport implements Transport { status: number, code: number, message: string, - options?: { headers?: Record; data?: string } + options?: { headers?: Record; data?: string; cause?: Error } ): Response { const error: { code: number; message: string; data?: string } = { code, message }; + this.onerror?.(options?.cause ?? new Error(message)); if (options?.data !== undefined) { error.data = options.data; } @@ -321,7 +322,6 @@ export class WebStandardStreamableHTTPServerTransport implements Transport { const hostHeader = req.headers.get('host'); if (!hostHeader || !this._allowedHosts.includes(hostHeader)) { const error = `Invalid Host header: ${hostHeader}`; - this.onerror?.(new Error(error)); return this.createJsonErrorResponse(403, -32_000, error); } } @@ -331,7 +331,6 @@ export class WebStandardStreamableHTTPServerTransport implements Transport { const originHeader = req.headers.get('origin'); if (originHeader && !this._allowedOrigins.includes(originHeader)) { const error = `Invalid Origin header: ${originHeader}`; - this.onerror?.(new Error(error)); return this.createJsonErrorResponse(403, -32_000, error); } } @@ -553,8 +552,7 @@ export class WebStandardStreamableHTTPServerTransport implements Transport { }); return new Response(readable, { headers }); - } catch (error) { - this.onerror?.(error as Error); + } catch { return this.createJsonErrorResponse(500, -32_000, 'Error replaying events'); } } @@ -586,23 +584,9 @@ export class WebStandardStreamableHTTPServerTransport implements Transport { * Handles unsupported requests (`PUT`, `PATCH`, etc.) */ private handleUnsupportedRequest(): Response { - return Response.json( - { - jsonrpc: '2.0', - error: { - code: -32_000, - message: 'Method not allowed.' - }, - id: null - }, - { - status: 405, - headers: { - Allow: 'GET, POST, DELETE', - 'Content-Type': 'application/json' - } - } - ); + return this.createJsonErrorResponse(405, -32_000, 'Method not allowed.', { + headers: { Allow: 'GET, POST, DELETE' } + }); } /** @@ -807,8 +791,10 @@ export class WebStandardStreamableHTTPServerTransport implements Transport { return new Response(readable, { status: 200, headers }); } catch (error) { // return JSON-RPC formatted error - this.onerror?.(error as Error); - return this.createJsonErrorResponse(400, -32_700, 'Parse error', { data: String(error) }); + return this.createJsonErrorResponse(400, -32_700, 'Parse error', { + data: String(error), + cause: error instanceof Error ? error : new Error(String(error)) + }); } } diff --git a/packages/server/test/server/streamableHttp.test.ts b/packages/server/test/server/streamableHttp.test.ts index ab6f22342..81c25acf2 100644 --- a/packages/server/test/server/streamableHttp.test.ts +++ b/packages/server/test/server/streamableHttp.test.ts @@ -765,4 +765,107 @@ describe('Zod v4', () => { await expect(transport.start()).rejects.toThrow('Transport already started'); }); }); + + describe('HTTPServerTransport - onerror callback', () => { + let transport: WebStandardStreamableHTTPServerTransport; + let errors: Error[]; + + beforeEach(async () => { + transport = new WebStandardStreamableHTTPServerTransport({ + sessionIdGenerator: () => randomUUID() + }); + errors = []; + transport.onerror = error => errors.push(error); + await transport.start(); + }); + + it('should call onerror for invalid Accept header on POST', async () => { + const request = createRequest('POST', TEST_MESSAGES.initialize, { accept: 'application/json' }); + const response = await transport.handleRequest(request); + + expect(response.status).toBe(406); + expect(errors.length).toBe(1); + expect(errors[0]!.message).toMatch(/Not Acceptable/); + }); + + it('should call onerror for invalid Content-Type on POST', async () => { + const request = new Request('http://localhost/mcp', { + method: 'POST', + headers: { + Accept: 'application/json, text/event-stream', + 'Content-Type': 'text/plain' + }, + body: JSON.stringify(TEST_MESSAGES.initialize) + }); + const response = await transport.handleRequest(request); + + expect(response.status).toBe(415); + expect(errors.length).toBe(1); + expect(errors[0]!.message).toMatch(/Unsupported Media Type/); + }); + + it('should call onerror for invalid JSON on POST', async () => { + const request = new Request('http://localhost/mcp', { + method: 'POST', + headers: { + Accept: 'application/json, text/event-stream', + 'Content-Type': 'application/json' + }, + body: 'not valid json' + }); + const response = await transport.handleRequest(request); + + expect(response.status).toBe(400); + expect(errors.length).toBe(1); + expect(errors[0]!.message).toMatch(/Parse error/); + }); + + it('should call onerror for invalid JSON-RPC message on POST', async () => { + const request = new Request('http://localhost/mcp', { + method: 'POST', + headers: { + Accept: 'application/json, text/event-stream', + 'Content-Type': 'application/json' + }, + body: JSON.stringify({ not: 'a jsonrpc message' }) + }); + const response = await transport.handleRequest(request); + + expect(response.status).toBe(400); + expect(errors.length).toBe(1); + expect(errors[0]!.message).toMatch(/Parse error.*Invalid JSON-RPC/); + }); + + it('should call onerror for invalid Accept header on GET', async () => { + const request = createRequest('GET', undefined, { accept: 'application/json' }); + const response = await transport.handleRequest(request); + + expect(response.status).toBe(406); + expect(errors.length).toBe(1); + expect(errors[0]!.message).toMatch(/Not Acceptable/); + }); + + it('should call onerror for unsupported HTTP methods', async () => { + const request = new Request('http://localhost/mcp', { method: 'PUT' }); + const response = await transport.handleRequest(request); + + expect(response.status).toBe(405); + expect(errors.length).toBe(1); + expect(errors[0]!.message).toBe('Method not allowed.'); + }); + + it('should preserve original error object when createJsonErrorResponse receives cause', async () => { + const thrown = new Error('boom-from-onmessage'); + transport.onmessage = () => { + throw thrown; + }; + + const request = createRequest('POST', TEST_MESSAGES.initialize); + const response = await transport.handleRequest(request); + + expect(response.status).toBe(400); + expect(errors.length).toBe(1); + expect(errors[0]).toBe(thrown); + }); + }); });