diff --git a/src/__tests__/__snapshots__/server.http.test.ts.snap b/src/__tests__/__snapshots__/server.http.test.ts.snap index 40071dbd..cb1984ae 100644 --- a/src/__tests__/__snapshots__/server.http.test.ts.snap +++ b/src/__tests__/__snapshots__/server.http.test.ts.snap @@ -298,16 +298,30 @@ exports[`startHttpTransport should start HTTP server, with port and host: server [Function], ], ], - "setupHandlers": [ - [ - "connection", - [Function], + "setupHandlers": { + "off": [ + [ + "error", + [Function], + ], ], - [ - "error", - [Function], + "on": [ + [ + "connection", + [Function], + ], + [ + "error", + [Function], + ], ], - ], + "once": [ + [ + "error", + [Function], + ], + ], + }, "setupServer": [ [ { @@ -315,11 +329,6 @@ exports[`startHttpTransport should start HTTP server, with port and host: server "sessionId": "test-session-123", }, ], - [ - 3000, - "localhost", - [Function], - ], ], "setupTransport": [ [ diff --git a/src/__tests__/server.http.test.ts b/src/__tests__/server.http.test.ts index 68b12138..0cd4e6d2 100644 --- a/src/__tests__/server.http.test.ts +++ b/src/__tests__/server.http.test.ts @@ -28,7 +28,10 @@ describe('getProcessOnPort', () => { describe('startHttpTransport', () => { const mockFunction = jest.fn(); - const mockEventHandler = jest.fn(); + const mockOnHandler = jest.fn(); + const mockOnceHandler = jest.fn(); + const mockOffHandler = jest.fn(); + const mockListen = jest.fn(); const mockServerClose = jest.fn(); let mockRequestHandler: ((req: any, res: any) => void) | undefined; let mockServer: any; @@ -41,8 +44,10 @@ describe('startHttpTransport', () => { registerTool: mockFunction }; mockHttpServer = { - on: mockEventHandler, - listen: mockFunction.mockImplementation((_port: any, _host: any, callback: any) => { + on: mockOnHandler, + once: mockOnceHandler, + off: mockOffHandler, + listen: mockListen.mockImplementation((_port: any, _host: any, callback: any) => { if (callback) { callback(); } @@ -78,11 +83,43 @@ describe('startHttpTransport', () => { expect({ setupServer: mockFunction.mock.calls, setupTransport: MockStreamableHTTPServerTransport.mock.calls, - setupHandlers: mockEventHandler.mock.calls, + setupHandlers: { + on: mockOnHandler.mock.calls, + once: mockOnceHandler.mock.calls, + off: mockOffHandler.mock.calls + }, serverClose: mockServerClose.mock.calls }).toMatchSnapshot('server setup'); }); + it('should reject startup on EADDRINUSE error', async () => { + mockHttpServer.listen.mockImplementation(() => { + const errorHandler = mockHttpServer.once.mock.calls.find((call: any) => call[0] === 'error')?.[1]; + + if (errorHandler) { + const err: any = new Error('Address in use'); + + err.code = 'EADDRINUSE'; + errorHandler(err); + } + }); + + await expect( + startHttpTransport(mockServer, { http: { port: 5000, host: 'localhost' } } as any) + ).rejects.toThrow('Port 5000 is already in use'); + }); + + it('should log runtime error emitted after startup without throwing unhandled exception', async () => { + const server = await startHttpTransport(mockServer, { http: { port: 3000, host: 'localhost' } } as any); + + const runtimeErrorHandler = mockHttpServer.on.mock.calls.find((call: any) => call[0] === 'error')?.[1]; + + expect(runtimeErrorHandler).toBeDefined(); + await expect(runtimeErrorHandler(new Error('Connection reset'))).resolves.toBeUndefined(); + + await server.close(); + }); + it.each([ { description: 'with invalid port', diff --git a/src/server.http.ts b/src/server.http.ts index dbd34f4c..98ea39e0 100644 --- a/src/server.http.ts +++ b/src/server.http.ts @@ -214,17 +214,9 @@ const startHttpTransport = async (mcpServer: McpServer, options = getOptions()): // Start the server. Port conflicts will be handled in the error handler below await new Promise((resolve, reject) => { - server.listen(http.port, http.host, () => { - log.info(`${name} server running on http://${http.host}:${getPort()}`); - resolve(); - }); + const onStartupError = async (error: NodeJS.ErrnoException) => { + server.off('error', onStartupError); - server.on('connection', socket => { - connections.add(socket); - socket.on('close', () => connections.delete(socket)); - }); - - server.on('error', async (error: NodeJS.ErrnoException) => { if (error.code === 'EADDRINUSE') { const processInfo = await getProcessOnPort(http.port); const errorMessage = `Port ${http.port} is already in use${processInfo ? ` by PID ${processInfo.pid}` : ''}.`; @@ -235,9 +227,26 @@ const startHttpTransport = async (mcpServer: McpServer, options = getOptions()): log.error(`HTTP server error: ${error}`); reject(error); } + }; + + server.once('error', onStartupError); + + server.listen(http.port, http.host, () => { + server.off('error', onStartupError); + log.info(`${name} server running on http://${http.host}:${getPort()}`); + resolve(); }); }); + server.on('connection', socket => { + connections.add(socket); + socket.on('close', () => connections.delete(socket)); + }); + + server.on('error', async (error: NodeJS.ErrnoException) => { + log.error(`HTTP server runtime error: ${error.message || error}`); + }); + return { port: getPort(), close: async () => {