Skip to content

Commit 06e0b59

Browse files
test: improve maxRetries=0 test to verify the actual fix
The previous test didn't properly exercise _scheduleReconnection. This improved test: - Directly tests _scheduleReconnection with maxRetries=0 - Verifies the "Maximum reconnection attempts (0) exceeded" error - Confirms no setTimeout is scheduled (no retry attempt) - Adds a companion test for maxRetries>0 to verify normal behavior
1 parent ba9013c commit 06e0b59

File tree

1 file changed

+33
-28
lines changed

1 file changed

+33
-28
lines changed

src/client/streamableHttp.test.ts

Lines changed: 33 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1508,7 +1508,7 @@ describe('StreamableHTTPClientTransport', () => {
15081508
beforeEach(() => vi.useFakeTimers());
15091509
afterEach(() => vi.useRealTimers());
15101510

1511-
it('should not retry forever with maxRetries 0', async () => {
1511+
it('should not schedule any reconnection attempts when maxRetries is 0', async () => {
15121512
// ARRANGE
15131513
transport = new StreamableHTTPClientTransport(new URL('http://localhost:1234/mcp'), {
15141514
reconnectionOptions: {
@@ -1522,39 +1522,44 @@ describe('StreamableHTTPClientTransport', () => {
15221522
const errorSpy = vi.fn();
15231523
transport.onerror = errorSpy;
15241524

1525-
const failingStream = new ReadableStream({
1526-
start(controller) {
1527-
controller.error(new Error('Network failure'));
1528-
}
1529-
});
1525+
// ACT - directly call _scheduleReconnection which is the code path the fix affects
1526+
transport['_scheduleReconnection']({});
15301527

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
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+
}
15381549
});
15391550

1540-
// ACT
1541-
await transport.start();
1542-
await transport['_startOrAuthSse']({});
1551+
const errorSpy = vi.fn();
1552+
transport.onerror = errorSpy;
15431553

1544-
// Advance time to check if any retries were scheduled
1545-
await vi.advanceTimersByTimeAsync(100);
1554+
// ACT - call _scheduleReconnection with attemptCount 0
1555+
transport['_scheduleReconnection']({});
15461556

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');
1557+
// ASSERT - should schedule a reconnection, not report error yet
1558+
expect(errorSpy).not.toHaveBeenCalled();
1559+
expect(transport['_reconnectionTimeout']).toBeDefined();
15511560

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-
);
1561+
// Clean up the timeout to avoid test pollution
1562+
clearTimeout(transport['_reconnectionTimeout']);
15581563
});
15591564
});
15601565

0 commit comments

Comments
 (0)