diff --git a/README.md b/README.md index 12587aa..fcf2821 100644 --- a/README.md +++ b/README.md @@ -1114,6 +1114,13 @@ Current database coverage (n8n v1.117.2): ## 🔄 Recent Updates +### v2.22.19 - Critical Bug Fix +**Fixed:** Stack overflow in session removal (Issue #427) +- Eliminated infinite recursion in HTTP server session cleanup +- Transport resources now deleted before closing to prevent circular event handler chain +- Production logs no longer show "RangeError: Maximum call stack size exceeded" +- All session cleanup operations now complete successfully without crashes + See [CHANGELOG.md](./docs/CHANGELOG.md) for full version history and recent changes. ## ⚠️ Known Issues diff --git a/package.json b/package.json index a06c669..144f54f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "n8n-mcp", - "version": "2.22.18", + "version": "2.22.19", "description": "Integration between n8n workflow automation and Model Context Protocol (MCP)", "main": "dist/index.js", "types": "dist/index.d.ts", diff --git a/src/http-server-single-session.ts b/src/http-server-single-session.ts index 17716d1..afd67b5 100644 --- a/src/http-server-single-session.ts +++ b/src/http-server-single-session.ts @@ -155,17 +155,22 @@ export class SingleSessionHTTPServer { */ private async removeSession(sessionId: string, reason: string): Promise { try { - // Close transport if exists - if (this.transports[sessionId]) { - await this.transports[sessionId].close(); - delete this.transports[sessionId]; - } - - // Remove server, metadata, and context + // Store reference to transport before deletion + const transport = this.transports[sessionId]; + + // Delete transport FIRST to prevent onclose handler from triggering recursion + // This breaks the circular reference: removeSession -> close -> onclose -> removeSession + delete this.transports[sessionId]; delete this.servers[sessionId]; delete this.sessionMetadata[sessionId]; delete this.sessionContexts[sessionId]; - + + // Close transport AFTER deletion + // When onclose handler fires, it won't find the transport anymore + if (transport) { + await transport.close(); + } + logger.info('Session removed', { sessionId, reason }); } catch (error) { logger.warn('Error removing session', { sessionId, reason, error }); diff --git a/tests/unit/http-server-session-management.test.ts b/tests/unit/http-server-session-management.test.ts index d1e4387..1deb67a 100644 --- a/tests/unit/http-server-session-management.test.ts +++ b/tests/unit/http-server-session-management.test.ts @@ -411,17 +411,17 @@ describe('HTTP Server Session Management', () => { it('should handle removeSession with transport close error gracefully', async () => { server = new SingleSessionHTTPServer(); - - const mockTransport = { + + const mockTransport = { close: vi.fn().mockRejectedValue(new Error('Transport close failed')) }; (server as any).transports = { 'test-session': mockTransport }; (server as any).servers = { 'test-session': {} }; - (server as any).sessionMetadata = { - 'test-session': { + (server as any).sessionMetadata = { + 'test-session': { lastAccess: new Date(), createdAt: new Date() - } + } }; // Should not throw even if transport close fails @@ -429,11 +429,67 @@ describe('HTTP Server Session Management', () => { // Verify transport close was attempted expect(mockTransport.close).toHaveBeenCalled(); - + // Session should still be cleaned up despite transport error // Note: The actual implementation may handle errors differently, so let's verify what we can expect(mockTransport.close).toHaveBeenCalledWith(); }); + + it('should not cause infinite recursion when transport.close triggers onclose handler', async () => { + server = new SingleSessionHTTPServer(); + + const sessionId = 'test-recursion-session'; + let closeCallCount = 0; + let oncloseCallCount = 0; + + // Create a mock transport that simulates the actual behavior + const mockTransport = { + close: vi.fn().mockImplementation(async () => { + closeCallCount++; + // Simulate the actual SDK behavior: close() triggers onclose handler + if (mockTransport.onclose) { + oncloseCallCount++; + await mockTransport.onclose(); + } + }), + onclose: null as (() => Promise) | null, + sessionId + }; + + // Set up the transport and session data + (server as any).transports = { [sessionId]: mockTransport }; + (server as any).servers = { [sessionId]: {} }; + (server as any).sessionMetadata = { + [sessionId]: { + lastAccess: new Date(), + createdAt: new Date() + } + }; + + // Set up onclose handler like the real implementation does + // This handler calls removeSession, which could cause infinite recursion + mockTransport.onclose = async () => { + await (server as any).removeSession(sessionId, 'transport_closed'); + }; + + // Call removeSession - this should NOT cause infinite recursion + await (server as any).removeSession(sessionId, 'manual_removal'); + + // Verify the fix works: + // 1. close() should be called exactly once + expect(closeCallCount).toBe(1); + + // 2. onclose handler should be triggered + expect(oncloseCallCount).toBe(1); + + // 3. Transport should be deleted and not cause second close attempt + expect((server as any).transports[sessionId]).toBeUndefined(); + expect((server as any).servers[sessionId]).toBeUndefined(); + expect((server as any).sessionMetadata[sessionId]).toBeUndefined(); + + // 4. If there was a recursion bug, closeCallCount would be > 1 + // or the test would timeout/crash with "Maximum call stack size exceeded" + }); }); describe('Session Metadata Tracking', () => {