Skip to content

Commit a014363

Browse files
committed
fix(streamableHttp): fix infinite retries when maxRetries is set to 0
closes #1179
1 parent 356b7e6 commit a014363

File tree

2 files changed

+59
-1
lines changed

2 files changed

+59
-1
lines changed

src/client/streamableHttp.test.ts

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1501,6 +1501,64 @@ describe('StreamableHTTPClientTransport', () => {
15011501
});
15021502
});
15031503

1504+
describe('Reconnection Logic with maxRetries 0', () => {
1505+
let transport: StreamableHTTPClientTransport;
1506+
1507+
// Use fake timers to control setTimeout and make the test instant.
1508+
beforeEach(() => vi.useFakeTimers());
1509+
afterEach(() => vi.useRealTimers());
1510+
1511+
it('should not retry forever with maxRetries 0', async () => {
1512+
// ARRANGE
1513+
transport = new StreamableHTTPClientTransport(new URL('http://localhost:1234/mcp'), {
1514+
reconnectionOptions: {
1515+
initialReconnectionDelay: 10,
1516+
maxRetries: 0, // This should disable retries completely
1517+
maxReconnectionDelay: 1000,
1518+
reconnectionDelayGrowFactor: 1
1519+
}
1520+
});
1521+
1522+
const errorSpy = vi.fn();
1523+
transport.onerror = errorSpy;
1524+
1525+
const failingStream = new ReadableStream({
1526+
start(controller) {
1527+
controller.error(new Error('Network failure'));
1528+
}
1529+
});
1530+
1531+
const fetchMock = global.fetch as Mock;
1532+
// Mock the initial GET request, which will fail
1533+
fetchMock.mockResolvedValueOnce({
1534+
ok: true,
1535+
status: 200,
1536+
headers: new Headers({ 'content-type': 'text/event-stream' }),
1537+
body: failingStream
1538+
});
1539+
1540+
// ACT
1541+
await transport.start();
1542+
await transport['_startOrAuthSse']({});
1543+
1544+
// Advance time to check if any retries were scheduled
1545+
await vi.advanceTimersByTimeAsync(100);
1546+
1547+
// ASSERT
1548+
// THE KEY ASSERTION: Fetch was only called ONCE. No retries with maxRetries: 0
1549+
expect(fetchMock).toHaveBeenCalledTimes(1);
1550+
expect(fetchMock.mock.calls[0][1]?.method).toBe('GET');
1551+
1552+
// Should still report the error but not retry
1553+
expect(errorSpy).toHaveBeenCalledWith(
1554+
expect.objectContaining({
1555+
message: expect.stringContaining('SSE stream disconnected: Error: Network failure')
1556+
})
1557+
);
1558+
});
1559+
});
1560+
1561+
15041562
describe('prevent infinite recursion when server returns 401 after successful auth', () => {
15051563
it('should throw error when server returns 401 after successful auth', async () => {
15061564
const message: JSONRPCMessage = {

src/client/streamableHttp.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ export class StreamableHTTPClientTransport implements Transport {
279279
const maxRetries = this._reconnectionOptions.maxRetries;
280280

281281
// Check if we've exceeded maximum retry attempts
282-
if (maxRetries > 0 && attemptCount >= maxRetries) {
282+
if (attemptCount >= maxRetries) {
283283
this.onerror?.(new Error(`Maximum reconnection attempts (${maxRetries}) exceeded.`));
284284
return;
285285
}

0 commit comments

Comments
 (0)