Skip to content

Commit 254616d

Browse files
fix(client): per-request abort in StreamableHTTP is a clean shutdown — no onerror, no reconnect
McpSubscription.close() aborts the listen POST via requestSignal, but _handleSseStream only knew about the transport-level abort: every close() fired a misleading 'SSE stream disconnected' onerror, and (when the server had stamped SSE event ids for resumability) scheduled a GET+Last-Event-ID reconnect that resurrected the subscription the caller just tore down. Thread the per-request requestSignal into _handleSseStream via StartSSEOptions; when EITHER the transport signal or the per-request signal is aborted, skip onerror and skip reconnect (mirrors the existing transport-level reconnect guard).
1 parent 85eadc3 commit 254616d

2 files changed

Lines changed: 77 additions & 4 deletions

File tree

packages/client/src/client/streamableHttp.ts

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,15 @@ export interface StartSSEOptions {
5050
* so that the response can be associated with the new resumed request.
5151
*/
5252
replayMessageId?: string | number;
53+
54+
/**
55+
* The per-request abort signal supplied by the caller via
56+
* `TransportSendOptions.requestSignal`. When this signal is aborted the
57+
* originating POST and its SSE response stream are torn down
58+
* intentionally — `_handleSseStream` treats it exactly like the
59+
* transport-level abort: no `onerror`, no reconnect.
60+
*/
61+
requestSignal?: AbortSignal;
5362
}
5463

5564
/**
@@ -391,7 +400,13 @@ export class StreamableHTTPClientTransport implements Transport {
391400
if (!stream) {
392401
return;
393402
}
394-
const { onresumptiontoken, replayMessageId } = options;
403+
const { onresumptiontoken, replayMessageId, requestSignal } = options;
404+
// An intentional abort — transport-wide close OR a per-request abort
405+
// (McpSubscription.close() aborting its `requestSignal`) — must read as
406+
// a clean shutdown: no misleading "SSE stream disconnected" onerror,
407+
// and no GET+Last-Event-ID reconnect that would resurrect a stream the
408+
// caller just tore down.
409+
const isIntentionalAbort = (): boolean => this._abortController?.signal.aborted === true || requestSignal?.aborted === true;
395410

396411
let lastEventId: string | undefined;
397412
// Track whether we've received a priming event (event with ID)
@@ -460,7 +475,7 @@ export class StreamableHTTPClientTransport implements Transport {
460475
// BUT don't reconnect if we already received a response - the request is complete
461476
const canResume = isReconnectable || hasPrimingEvent;
462477
const needsReconnect = canResume && !receivedResponse;
463-
if (needsReconnect && this._abortController && !this._abortController.signal.aborted) {
478+
if (needsReconnect && this._abortController && !isIntentionalAbort()) {
464479
this._scheduleReconnection(
465480
{
466481
resumptionToken: lastEventId,
@@ -471,6 +486,11 @@ export class StreamableHTTPClientTransport implements Transport {
471486
);
472487
}
473488
} catch (error) {
489+
if (isIntentionalAbort()) {
490+
// The reader threw because we aborted it. Not an error; do
491+
// not surface onerror, do not reconnect.
492+
return;
493+
}
474494
// Handle stream errors - likely a network disconnect
475495
this.onerror?.(new Error(`SSE stream disconnected: ${error}`));
476496

@@ -479,7 +499,7 @@ export class StreamableHTTPClientTransport implements Transport {
479499
// BUT don't reconnect if we already received a response - the request is complete
480500
const canResume = isReconnectable || hasPrimingEvent;
481501
const needsReconnect = canResume && !receivedResponse;
482-
if (needsReconnect && this._abortController && !this._abortController.signal.aborted) {
502+
if (needsReconnect && this._abortController && !isIntentionalAbort()) {
483503
// Use the exponential backoff reconnection strategy
484504
try {
485505
this._scheduleReconnection(
@@ -699,7 +719,7 @@ export class StreamableHTTPClientTransport implements Transport {
699719
// Handle SSE stream responses for requests
700720
// We use the same handler as standalone streams, which now supports
701721
// reconnection with the last event ID
702-
this._handleSseStream(response.body, { onresumptiontoken }, false);
722+
this._handleSseStream(response.body, { onresumptiontoken, requestSignal: options?.requestSignal }, false);
703723
} else if (contentType?.includes('application/json')) {
704724
// For non-streaming servers, we might get direct JSON responses
705725
const data = await response.json();

packages/client/test/client/streamableHttp.test.ts

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1102,6 +1102,59 @@ describe('StreamableHTTPClientTransport', () => {
11021102
expect(fetchMock.mock.calls[0]![1]?.method).toBe('POST');
11031103
});
11041104

1105+
it('per-request requestSignal abort: no onerror, no reconnect (McpSubscription.close())', async () => {
1106+
// ARRANGE — a POST stream that has been primed with an SSE event id
1107+
// (server-side resumability), so without the per-request abort
1108+
// guard the transport WOULD schedule a GET+Last-Event-ID reconnect.
1109+
transport = new StreamableHTTPClientTransport(new URL('http://localhost:1234/mcp'), {
1110+
reconnectionOptions: {
1111+
initialReconnectionDelay: 10,
1112+
maxRetries: 1,
1113+
maxReconnectionDelay: 1000,
1114+
reconnectionDelayGrowFactor: 1
1115+
}
1116+
});
1117+
const errorSpy = vi.fn();
1118+
transport.onerror = errorSpy;
1119+
1120+
let streamController!: ReadableStreamDefaultController<Uint8Array>;
1121+
const primedStream = new ReadableStream<Uint8Array>({
1122+
start(controller) {
1123+
streamController = controller;
1124+
// Priming event with an id — would arm POST-stream resumability.
1125+
controller.enqueue(new TextEncoder().encode('id: ev-1\ndata: \n\n'));
1126+
}
1127+
});
1128+
const fetchMock = globalThis.fetch as Mock;
1129+
fetchMock.mockImplementationOnce((_url, init: RequestInit) => {
1130+
// Propagate abort to the stream the way fetch does.
1131+
init.signal?.addEventListener('abort', () => streamController.error(init.signal?.reason), { once: true });
1132+
return Promise.resolve({
1133+
ok: true,
1134+
status: 200,
1135+
headers: new Headers({ 'content-type': 'text/event-stream' }),
1136+
body: primedStream
1137+
});
1138+
});
1139+
1140+
const requestAbort = new AbortController();
1141+
await transport.start();
1142+
await transport.send(
1143+
{ jsonrpc: '2.0', method: 'subscriptions/listen', id: 'listen-1', params: {} },
1144+
{ requestSignal: requestAbort.signal }
1145+
);
1146+
await vi.advanceTimersByTimeAsync(5);
1147+
expect(fetchMock).toHaveBeenCalledTimes(1);
1148+
1149+
// ACT — McpSubscription.close() aborts the per-request signal.
1150+
requestAbort.abort();
1151+
await vi.advanceTimersByTimeAsync(50);
1152+
1153+
// ASSERT — intentional per-request abort: no onerror, no reconnect.
1154+
expect(errorSpy).not.toHaveBeenCalled();
1155+
expect(fetchMock).toHaveBeenCalledTimes(1);
1156+
});
1157+
11051158
it('should NOT reconnect a POST stream when error response was received', async () => {
11061159
// ARRANGE
11071160
transport = new StreamableHTTPClientTransport(new URL('http://localhost:1234/mcp'), {

0 commit comments

Comments
 (0)