From 6b5ed22b1615f72f9aa6648bec96914edc47b49a Mon Sep 17 00:00:00 2001 From: Sergei Frangulov Date: Thu, 7 May 2026 12:03:26 +0400 Subject: [PATCH] fix(client): stop propagating transport AbortController to POST/DELETE requests The SSE and StreamableHTTP client transports passed the transport-wide `_abortController.signal` to every outgoing POST (and DELETE in `terminateSession`). Calling `close()` then aborted that controller even when a request had already returned successfully, which made Undici-based instrumentation (e.g. OpenTelemetry) report the completed request as `UND_ERR_ABORTED` and mark its span as failed. The transport's signal is now used only where it is actually needed: the SSE GET stream and the reconnection-state gate. Per-request cancellation can still be supplied by the caller via `requestInit.signal`. Fixes #1231. --- .changeset/transport-graceful-close.md | 5 ++ packages/client/src/client/sse.ts | 3 +- packages/client/src/client/streamableHttp.ts | 6 +- packages/client/test/client/sse.test.ts | 18 ++++++ .../client/test/client/streamableHttp.test.ts | 56 +++++++++++++++++++ 5 files changed, 82 insertions(+), 6 deletions(-) create mode 100644 .changeset/transport-graceful-close.md diff --git a/.changeset/transport-graceful-close.md b/.changeset/transport-graceful-close.md new file mode 100644 index 0000000000..a71dad34ed --- /dev/null +++ b/.changeset/transport-graceful-close.md @@ -0,0 +1,5 @@ +--- +'@modelcontextprotocol/client': patch +--- + +Stop propagating the transport-wide `AbortController` signal to outgoing POST and DELETE requests in `SSEClientTransport` and `StreamableHTTPClientTransport`. Previously, calling `close()` on a transport that had just completed a successful POST would still abort the underlying `fetch` controller, causing Undici-based instrumentation (e.g. OpenTelemetry) to report the successful request as aborted (`UND_ERR_ABORTED`). The transport's own signal is now used only for the SSE GET stream and reconnection-state gating; per-request cancellation can still be supplied by the user via `requestInit.signal`. diff --git a/packages/client/src/client/sse.ts b/packages/client/src/client/sse.ts index f441e9cdb8..16c3b23995 100644 --- a/packages/client/src/client/sse.ts +++ b/packages/client/src/client/sse.ts @@ -261,8 +261,7 @@ export class SSEClientTransport implements Transport { ...this._requestInit, method: 'POST', headers, - body: JSON.stringify(message), - signal: this._abortController?.signal + body: JSON.stringify(message) }; const response = await (this._fetch ?? fetch)(this._endpoint, init); diff --git a/packages/client/src/client/streamableHttp.ts b/packages/client/src/client/streamableHttp.ts index cd643c96dc..53fc50319b 100644 --- a/packages/client/src/client/streamableHttp.ts +++ b/packages/client/src/client/streamableHttp.ts @@ -548,8 +548,7 @@ export class StreamableHTTPClientTransport implements Transport { ...this._requestInit, method: 'POST', headers, - body: JSON.stringify(message), - signal: this._abortController?.signal + body: JSON.stringify(message) }; const response = await (this._fetch ?? fetch)(this._url, init); @@ -715,8 +714,7 @@ export class StreamableHTTPClientTransport implements Transport { const init = { ...this._requestInit, method: 'DELETE', - headers, - signal: this._abortController?.signal + headers }; const response = await (this._fetch ?? fetch)(this._url, init); diff --git a/packages/client/test/client/sse.test.ts b/packages/client/test/client/sse.test.ts index b0b9588f02..3083b5ae74 100644 --- a/packages/client/test/client/sse.test.ts +++ b/packages/client/test/client/sse.test.ts @@ -211,6 +211,24 @@ describe('SSEClientTransport', () => { expect(JSON.parse((lastServerRequest as IncomingMessage & { body: string }).body)).toEqual(testMessage); }); + it('does not pass the transport-wide AbortController signal to POST requests (issue #1231)', async () => { + // Sharing the SSE-stream AbortController with POST requests means a successful POST + // followed by transport.close() leaves Undici-based instrumentation reporting the + // request as aborted. The transport must not propagate its own signal to POSTs. + let capturedSignal: AbortSignal | undefined | null = null; + const captureFetch: typeof fetch = (url, init) => { + capturedSignal = init?.signal ?? undefined; + return fetch(url as string | URL, init); + }; + + transport = new SSEClientTransport(resourceBaseUrl, { fetch: captureFetch }); + await transport.start(); + + await transport.send({ jsonrpc: '2.0', id: 'test-1', method: 'test', params: {} }); + + expect(capturedSignal).toBeUndefined(); + }); + it('handles POST request failures', async () => { // Create a server that returns 500 for POST await resourceServer.close(); diff --git a/packages/client/test/client/streamableHttp.test.ts b/packages/client/test/client/streamableHttp.test.ts index b2138b3fa8..4591385466 100644 --- a/packages/client/test/client/streamableHttp.test.ts +++ b/packages/client/test/client/streamableHttp.test.ts @@ -62,6 +62,62 @@ describe('StreamableHTTPClientTransport', () => { ); }); + it('does not cancel in-flight POST requests when close() is called (issue #1231)', async () => { + // POST requests must not share the transport-wide AbortController, otherwise calling + // close() while a request is being processed (or just after a successful 200) makes + // Undici-based instrumentation (e.g. OpenTelemetry) report the request as aborted + // even though the server responded successfully. + let capturedSignal: AbortSignal | undefined; + let resolveResponse!: (value: { ok: boolean; status: number; headers: Headers }) => void; + const responsePromise = new Promise<{ ok: boolean; status: number; headers: Headers }>(resolve => { + resolveResponse = resolve; + }); + + (globalThis.fetch as Mock).mockImplementationOnce((_url: unknown, init?: RequestInit) => { + capturedSignal = init?.signal ?? undefined; + return responsePromise; + }); + + const message: JSONRPCMessage = { jsonrpc: '2.0', method: 'test', params: {}, id: 'inflight-id' }; + const sendPromise = transport.send(message); + + // While the POST is in-flight, close the transport. + const closePromise = transport.close(); + + // The POST should not have been issued with the transport's abort signal. + expect(capturedSignal).toBeUndefined(); + + // Server completes the request after close(). + resolveResponse({ ok: true, status: 202, headers: new Headers() }); + + await sendPromise; + await closePromise; + }); + + it('does not cancel in-flight DELETE (terminateSession) when close() is called', async () => { + // First, get a session ID so terminateSession actually issues a DELETE. + (globalThis.fetch as Mock).mockResolvedValueOnce({ + ok: true, + status: 200, + headers: new Headers({ 'content-type': 'text/event-stream', 'mcp-session-id': 'session-x' }) + }); + await transport.send({ + jsonrpc: '2.0', + method: 'initialize', + params: { clientInfo: { name: 'c', version: '1.0' }, protocolVersion: '2025-03-26' }, + id: 'init-id' + }); + + let capturedSignal: AbortSignal | undefined; + (globalThis.fetch as Mock).mockImplementationOnce((_url: unknown, init?: RequestInit) => { + capturedSignal = init?.signal ?? undefined; + return Promise.resolve({ ok: true, status: 200, headers: new Headers() }); + }); + + await transport.terminateSession(); + expect(capturedSignal).toBeUndefined(); + }); + it('should send batch messages', async () => { const messages: JSONRPCMessage[] = [ { jsonrpc: '2.0', method: 'test1', params: {}, id: 'id1' },