Skip to content

Commit d67929f

Browse files
author
kai-agent-free
committed
fix: call onerror callback for all error responses in StreamableHTTPServerTransport
Previously, many error paths in handlePostRequest, handleGetRequest, and handleDeleteRequest would return JSON error responses without calling the onerror callback, making these errors invisible to users who set up error logging via transport.onerror. This fix moves the onerror call into createJsonErrorResponse itself, ensuring every error response consistently triggers the callback. Redundant manual onerror calls before createJsonErrorResponse are removed to avoid double-firing. Fixes #1395
1 parent ccb78f2 commit d67929f

2 files changed

Lines changed: 81 additions & 4 deletions

File tree

packages/server/src/server/streamableHttp.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,7 @@ export class WebStandardStreamableHTTPServerTransport implements Transport {
287287
options?: { headers?: Record<string, string>; data?: string }
288288
): Response {
289289
const error: { code: number; message: string; data?: string } = { code, message };
290+
this.onerror?.(new Error(message));
290291
if (options?.data !== undefined) {
291292
error.data = options.data;
292293
}
@@ -321,7 +322,6 @@ export class WebStandardStreamableHTTPServerTransport implements Transport {
321322
const hostHeader = req.headers.get('host');
322323
if (!hostHeader || !this._allowedHosts.includes(hostHeader)) {
323324
const error = `Invalid Host header: ${hostHeader}`;
324-
this.onerror?.(new Error(error));
325325
return this.createJsonErrorResponse(403, -32_000, error);
326326
}
327327
}
@@ -331,7 +331,6 @@ export class WebStandardStreamableHTTPServerTransport implements Transport {
331331
const originHeader = req.headers.get('origin');
332332
if (originHeader && !this._allowedOrigins.includes(originHeader)) {
333333
const error = `Invalid Origin header: ${originHeader}`;
334-
this.onerror?.(new Error(error));
335334
return this.createJsonErrorResponse(403, -32_000, error);
336335
}
337336
}
@@ -554,7 +553,6 @@ export class WebStandardStreamableHTTPServerTransport implements Transport {
554553

555554
return new Response(readable, { headers });
556555
} catch (error) {
557-
this.onerror?.(error as Error);
558556
return this.createJsonErrorResponse(500, -32_000, 'Error replaying events');
559557
}
560558
}
@@ -807,7 +805,6 @@ export class WebStandardStreamableHTTPServerTransport implements Transport {
807805
return new Response(readable, { status: 200, headers });
808806
} catch (error) {
809807
// return JSON-RPC formatted error
810-
this.onerror?.(error as Error);
811808
return this.createJsonErrorResponse(400, -32_700, 'Parse error', { data: String(error) });
812809
}
813810
}

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

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -765,4 +765,84 @@ 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 errors: Error[];
772+
773+
beforeEach(async () => {
774+
transport = new WebStandardStreamableHTTPServerTransport({
775+
sessionIdGenerator: () => randomUUID()
776+
});
777+
errors = [];
778+
transport.onerror = (error) => errors.push(error);
779+
await transport.start();
780+
});
781+
782+
it('should call onerror for invalid Accept header on POST', async () => {
783+
const request = createRequest('POST', TEST_MESSAGES.initialize, { accept: 'application/json' });
784+
const response = await transport.handleRequest(request);
785+
786+
expect(response.status).toBe(406);
787+
expect(errors.length).toBe(1);
788+
expect(errors[0].message).toMatch(/Not Acceptable/);
789+
});
790+
791+
it('should call onerror for invalid Content-Type on POST', async () => {
792+
const request = new Request('http://localhost/mcp', {
793+
method: 'POST',
794+
headers: {
795+
Accept: 'application/json, text/event-stream',
796+
'Content-Type': 'text/plain'
797+
},
798+
body: JSON.stringify(TEST_MESSAGES.initialize)
799+
});
800+
const response = await transport.handleRequest(request);
801+
802+
expect(response.status).toBe(415);
803+
expect(errors.length).toBe(1);
804+
expect(errors[0].message).toMatch(/Unsupported Media Type/);
805+
});
806+
807+
it('should call onerror for invalid JSON on POST', async () => {
808+
const request = new Request('http://localhost/mcp', {
809+
method: 'POST',
810+
headers: {
811+
Accept: 'application/json, text/event-stream',
812+
'Content-Type': 'application/json'
813+
},
814+
body: 'not valid json'
815+
});
816+
const response = await transport.handleRequest(request);
817+
818+
expect(response.status).toBe(400);
819+
expect(errors.length).toBe(1);
820+
expect(errors[0].message).toMatch(/Parse error/);
821+
});
822+
823+
it('should call onerror for invalid JSON-RPC message on POST', async () => {
824+
const request = new Request('http://localhost/mcp', {
825+
method: 'POST',
826+
headers: {
827+
Accept: 'application/json, text/event-stream',
828+
'Content-Type': 'application/json'
829+
},
830+
body: JSON.stringify({ not: 'a jsonrpc message' })
831+
});
832+
const response = await transport.handleRequest(request);
833+
834+
expect(response.status).toBe(400);
835+
expect(errors.length).toBe(1);
836+
expect(errors[0].message).toMatch(/Parse error.*Invalid JSON-RPC/);
837+
});
838+
839+
it('should call onerror for invalid Accept header on GET', async () => {
840+
const request = createRequest('GET', undefined, { accept: 'application/json' });
841+
const response = await transport.handleRequest(request);
842+
843+
expect(response.status).toBe(406);
844+
expect(errors.length).toBe(1);
845+
expect(errors[0].message).toMatch(/Not Acceptable/);
846+
});
847+
});
768848
});

0 commit comments

Comments
 (0)