Skip to content

Commit bfad917

Browse files
fix(streamableHttp): fix infinite retries when maxRetries is set to 0 (#1216)
Co-authored-by: Felix Weinberger <[email protected]>
1 parent 356b7e6 commit bfad917

File tree

2 files changed

+63
-1
lines changed

2 files changed

+63
-1
lines changed

src/client/streamableHttp.test.ts

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1501,6 +1501,68 @@ 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 schedule any reconnection attempts when maxRetries is 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+
// ACT - directly call _scheduleReconnection which is the code path the fix affects
1526+
transport['_scheduleReconnection']({});
1527+
1528+
// ASSERT - should immediately report max retries exceeded, not schedule a retry
1529+
expect(errorSpy).toHaveBeenCalledTimes(1);
1530+
expect(errorSpy).toHaveBeenCalledWith(
1531+
expect.objectContaining({
1532+
message: 'Maximum reconnection attempts (0) exceeded.'
1533+
})
1534+
);
1535+
1536+
// Verify no timeout was scheduled (no reconnection attempt)
1537+
expect(transport['_reconnectionTimeout']).toBeUndefined();
1538+
});
1539+
1540+
it('should schedule reconnection when maxRetries is greater than 0', async () => {
1541+
// ARRANGE
1542+
transport = new StreamableHTTPClientTransport(new URL('http://localhost:1234/mcp'), {
1543+
reconnectionOptions: {
1544+
initialReconnectionDelay: 10,
1545+
maxRetries: 1, // Allow 1 retry
1546+
maxReconnectionDelay: 1000,
1547+
reconnectionDelayGrowFactor: 1
1548+
}
1549+
});
1550+
1551+
const errorSpy = vi.fn();
1552+
transport.onerror = errorSpy;
1553+
1554+
// ACT - call _scheduleReconnection with attemptCount 0
1555+
transport['_scheduleReconnection']({});
1556+
1557+
// ASSERT - should schedule a reconnection, not report error yet
1558+
expect(errorSpy).not.toHaveBeenCalled();
1559+
expect(transport['_reconnectionTimeout']).toBeDefined();
1560+
1561+
// Clean up the timeout to avoid test pollution
1562+
clearTimeout(transport['_reconnectionTimeout']);
1563+
});
1564+
});
1565+
15041566
describe('prevent infinite recursion when server returns 401 after successful auth', () => {
15051567
it('should throw error when server returns 401 after successful auth', async () => {
15061568
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)