Skip to content

Commit 3dcd989

Browse files
address review feedback
- guard reconnect() against late firing after close via aborted-signal check - wrap _cancelReconnection in try/finally so close() always aborts and fires onclose - wrap recursive _scheduleReconnection in try/catch to route scheduler errors to onerror instead of unhandled rejection - export ReconnectionScheduler type from package index - add tests for late-firing reconnect and throwing cancel function
1 parent 2d1622d commit 3dcd989

File tree

3 files changed

+57
-6
lines changed

3 files changed

+57
-6
lines changed

packages/client/src/client/streamableHttp.ts

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -340,9 +340,14 @@ export class StreamableHTTPClientTransport implements Transport {
340340

341341
const reconnect = (): void => {
342342
this._cancelReconnection = undefined;
343+
if (this._abortController?.signal.aborted) return;
343344
this._startOrAuthSse(options).catch(error => {
344345
this.onerror?.(new Error(`Failed to reconnect SSE stream: ${error instanceof Error ? error.message : String(error)}`));
345-
this._scheduleReconnection(options, attemptCount + 1);
346+
try {
347+
this._scheduleReconnection(options, attemptCount + 1);
348+
} catch (scheduleError) {
349+
this.onerror?.(scheduleError instanceof Error ? scheduleError : new Error(String(scheduleError)));
350+
}
346351
});
347352
};
348353

@@ -499,12 +504,13 @@ export class StreamableHTTPClientTransport implements Transport {
499504
}
500505

501506
async close(): Promise<void> {
502-
if (this._cancelReconnection) {
503-
this._cancelReconnection();
507+
try {
508+
this._cancelReconnection?.();
509+
} finally {
504510
this._cancelReconnection = undefined;
511+
this._abortController?.abort();
512+
this.onclose?.();
505513
}
506-
this._abortController?.abort();
507-
this.onclose?.();
508514
}
509515

510516
async send(

packages/client/src/index.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,12 @@ export type { SSEClientTransportOptions } from './client/sse.js';
6262
export { SSEClientTransport, SseError } from './client/sse.js';
6363
export type { StdioServerParameters } from './client/stdio.js';
6464
export { DEFAULT_INHERITED_ENV_VARS, getDefaultEnvironment, StdioClientTransport } from './client/stdio.js';
65-
export type { StartSSEOptions, StreamableHTTPClientTransportOptions, StreamableHTTPReconnectionOptions } from './client/streamableHttp.js';
65+
export type {
66+
ReconnectionScheduler,
67+
StartSSEOptions,
68+
StreamableHTTPClientTransportOptions,
69+
StreamableHTTPReconnectionOptions
70+
} from './client/streamableHttp.js';
6671
export { StreamableHTTPClientTransport } from './client/streamableHttp.js';
6772
export { WebSocketClientTransport } from './client/websocket.js';
6873

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1811,5 +1811,45 @@ describe('StreamableHTTPClientTransport', () => {
18111811

18121812
expect(clearTimeoutSpy).toHaveBeenCalledTimes(1);
18131813
});
1814+
1815+
it('ignores a late-firing reconnect after close()', async () => {
1816+
let capturedReconnect: (() => void) | undefined;
1817+
transport = new StreamableHTTPClientTransport(new URL('http://localhost:1234/mcp'), {
1818+
reconnectionOptions,
1819+
reconnectionScheduler: reconnect => {
1820+
capturedReconnect = reconnect;
1821+
}
1822+
});
1823+
const onerror = vi.fn();
1824+
transport.onerror = onerror;
1825+
1826+
await transport.start();
1827+
triggerReconnection(transport);
1828+
await transport.close();
1829+
1830+
capturedReconnect?.();
1831+
await vi.runAllTimersAsync();
1832+
1833+
expect(onerror).not.toHaveBeenCalled();
1834+
});
1835+
1836+
it('still aborts and fires onclose if the cancel function throws', async () => {
1837+
transport = new StreamableHTTPClientTransport(new URL('http://localhost:1234/mcp'), {
1838+
reconnectionOptions,
1839+
reconnectionScheduler: () => () => {
1840+
throw new Error('cancel failed');
1841+
}
1842+
});
1843+
const onclose = vi.fn();
1844+
transport.onclose = onclose;
1845+
1846+
await transport.start();
1847+
triggerReconnection(transport);
1848+
const abortController = transport['_abortController'];
1849+
1850+
await expect(transport.close()).rejects.toThrow('cancel failed');
1851+
expect(abortController?.signal.aborted).toBe(true);
1852+
expect(onclose).toHaveBeenCalledTimes(1);
1853+
});
18141854
});
18151855
});

0 commit comments

Comments
 (0)