diff --git a/src/mcp-server.ts b/src/mcp-server.ts index de5df37..0ba69e3 100644 --- a/src/mcp-server.ts +++ b/src/mcp-server.ts @@ -443,8 +443,19 @@ export class MCPHttpServer { // idle by design (this server pushes no server-initiated notifications). // The global 120s socket timeout set in start() would otherwise reap it // every ~2 min, surfacing as a "stream terminated" reconnect churn in - // bridges and noisy logs (#221). Exempt only this socket from the idle - // timeout; POST request sockets keep the server-wide default. + // bridges and noisy logs (#221). Disable the idle timeout on this GET + // socket; POST request sockets keep the server-wide default. (A non-SSE + // GET is short-lived — its response is sent immediately — so the + // exemption is a harmless no-op for those.) + // + // Trade-off: with no socket-layer idle reap, a truly-dead SSE socket + // (client vanished without a FIN) is no longer dropped at ~2 min. The + // backstop is the SessionManager's 1h idle eviction, which calls + // transport.close() and thereby closes the socket — so abandoned streams + // are bounded by that timeout and the 32-session cap, not leaked + // unbounded. A long finite timeout was weighed against 0; the choice is + // deferred to the live-instance validation, since a finite value would + // reintroduce some (less frequent) reconnect churn. if (req.method === 'GET') { req.socket?.setTimeout(0); } diff --git a/tests/sse-socket-timeout.test.ts b/tests/sse-socket-timeout.test.ts index 9fd1cec..0882b43 100644 --- a/tests/sse-socket-timeout.test.ts +++ b/tests/sse-socket-timeout.test.ts @@ -68,6 +68,14 @@ describe('SSE notification-stream socket timeout exemption (#221)', () => { server = new MCPHttpServer(mockApp, 3001); }); + afterEach(async () => { + // The constructor arms the SessionManager + connection-pool intervals; + // server.stop() short-circuits when never started, so dispose them directly + // to avoid leaking timers across tests. + (server as any).sessionManager?.stop?.(); + await (server as any).connectionPool?.shutdown?.(); + }); + test('GET /mcp (SSE stream) disables its socket idle timeout', async () => { const { req, socket } = makeReq('GET', undefined, 'some-session'); await (server as any).handleMCPRequest(req, makeRes()); @@ -80,9 +88,12 @@ describe('SSE notification-stream socket timeout exemption (#221)', () => { expect(socket.setTimeout).not.toHaveBeenCalled(); }); - test('exemption tolerates a missing socket (no throw)', async () => { + test('exemption tolerates a missing socket and still routes the request', async () => { const req = { method: 'GET', body: undefined, headers: {} as Record }; const res = makeRes(); - await expect((server as any).handleMCPRequest(req, res)).resolves.toBeUndefined(); + await (server as any).handleMCPRequest(req, res); + // No socket to exempt → optional chaining no-ops; the GET (no session, not + // an initialize) still routes to the spec §2 "session required" 400. + expect(res.statusCode).toBe(400); }); });