From 570c642fc456984de4bebec76c44e67442abdbf8 Mon Sep 17 00:00:00 2001 From: Felix Weinberger Date: Tue, 31 Mar 2026 11:51:54 +0000 Subject: [PATCH 1/2] fix(client): remove redundant onerror in _startOrAuthSse catch _startOrAuthSse's catch block fired onerror then threw. All three callers (streamableHttp.ts:344, :531, :642) already .catch(error => this.onerror?.(error)), so every failure double-fired. Removed the internal call; callers handle it. Fixes #868 Co-authored-by: Anil Kumar --- .../fix-double-onerror-startOrAuthSse.md | 5 +++ packages/client/src/client/streamableHttp.ts | 3 +- .../client/test/client/streamableHttp.test.ts | 40 +++++++++++++++++++ 3 files changed, 46 insertions(+), 2 deletions(-) create mode 100644 .changeset/fix-double-onerror-startOrAuthSse.md diff --git a/.changeset/fix-double-onerror-startOrAuthSse.md b/.changeset/fix-double-onerror-startOrAuthSse.md new file mode 100644 index 000000000..69b9c9f9f --- /dev/null +++ b/.changeset/fix-double-onerror-startOrAuthSse.md @@ -0,0 +1,5 @@ +--- +'@modelcontextprotocol/client': patch +--- + +Fix double `onerror` invocation when `_startOrAuthSse` fails. The internal catch block fired `onerror` then threw, and all callers already `.catch(onerror)`, causing every failure to fire twice. Removed the redundant internal call. diff --git a/packages/client/src/client/streamableHttp.ts b/packages/client/src/client/streamableHttp.ts index 56cbb4d98..e5982199e 100644 --- a/packages/client/src/client/streamableHttp.ts +++ b/packages/client/src/client/streamableHttp.ts @@ -294,8 +294,7 @@ export class StreamableHTTPClientTransport implements Transport { this._handleSseStream(response.body, options, true); } catch (error) { - this.onerror?.(error as Error); - throw error; + throw error as Error; } } diff --git a/packages/client/test/client/streamableHttp.test.ts b/packages/client/test/client/streamableHttp.test.ts index 4a23e6db4..96c1602b0 100644 --- a/packages/client/test/client/streamableHttp.test.ts +++ b/packages/client/test/client/streamableHttp.test.ts @@ -1131,6 +1131,46 @@ describe('StreamableHTTPClientTransport', () => { expect(fetchMock.mock.calls[0]![1]?.method).toBe('POST'); }); + it('should fire onerror exactly once when _startOrAuthSse fails (not double-fire from catch + caller)', async () => { + transport = new StreamableHTTPClientTransport(new URL('http://localhost:1234/mcp')); + + const errorSpy = vi.fn(); + transport.onerror = errorSpy; + + const fetchMock = globalThis.fetch as Mock; + + // POST returns 202, which triggers _startOrAuthSse when the outbound + // message is an initialized notification (streamableHttp.ts:642) + fetchMock.mockResolvedValueOnce({ + ok: true, + status: 202, + headers: new Headers(), + text: async () => '' + }); + + // The subsequent GET (_startOrAuthSse) fails with a non-ok status + fetchMock.mockResolvedValueOnce({ + ok: false, + status: 500, + statusText: 'Internal Server Error', + headers: new Headers(), + text: async () => 'server error' + }); + + await transport.start(); + // Sending an initialized notification triggers the _startOrAuthSse path + await transport.send({ jsonrpc: '2.0', method: 'notifications/initialized' }); + + // Let the fire-and-forget _startOrAuthSse().catch() settle + await vi.runAllTimersAsync(); + + // Before the fix: 2 calls (internal catch at :296 + caller's .catch at :642) + // After the fix: 1 call (caller's .catch only) + expect(errorSpy).toHaveBeenCalledTimes(1); + expect(errorSpy.mock.calls[0]![0].message).toContain('Failed to open SSE stream'); + }); + + it('should not throw JSON parse error on priming events with empty data', async () => { transport = new StreamableHTTPClientTransport(new URL('http://localhost:1234/mcp')); From 40413401b8fa0da956bc76bbb5b2867dd6a566e8 Mon Sep 17 00:00:00 2001 From: Felix Weinberger Date: Tue, 31 Mar 2026 11:52:53 +0000 Subject: [PATCH 2/2] fix(client): remove redundant onerror in _startOrAuthSse catch _startOrAuthSse's catch block fired onerror then threw. All three callers (streamableHttp.ts:344, :531, :642) already .catch(error => this.onerror?.(error)), so every failure double-fired. Removed the internal call; callers handle it. Fixes #868 Co-authored-by: Anil Kumar --- packages/client/test/client/streamableHttp.test.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/client/test/client/streamableHttp.test.ts b/packages/client/test/client/streamableHttp.test.ts index 96c1602b0..971ca37fc 100644 --- a/packages/client/test/client/streamableHttp.test.ts +++ b/packages/client/test/client/streamableHttp.test.ts @@ -1164,13 +1164,10 @@ describe('StreamableHTTPClientTransport', () => { // Let the fire-and-forget _startOrAuthSse().catch() settle await vi.runAllTimersAsync(); - // Before the fix: 2 calls (internal catch at :296 + caller's .catch at :642) - // After the fix: 1 call (caller's .catch only) expect(errorSpy).toHaveBeenCalledTimes(1); expect(errorSpy.mock.calls[0]![0].message).toContain('Failed to open SSE stream'); }); - it('should not throw JSON parse error on priming events with empty data', async () => { transport = new StreamableHTTPClientTransport(new URL('http://localhost:1234/mcp'));