fix: eliminate stack overflow in session removal (#427) (#428)

Critical bug fix for production crashes during session cleanup.

**Root Cause:**
Infinite recursion caused by circular event handler chain:
- removeSession() called transport.close()
- transport.close() triggered onclose event handler
- onclose handler called removeSession() again
- Loop continued until stack overflow

**Solution:**
Delete transport from registry BEFORE closing to break circular reference:
1. Store transport reference
2. Delete from this.transports first
3. Close transport after deletion
4. When onclose fires, transport no longer found, no recursion

**Impact:**
- Eliminates "RangeError: Maximum call stack size exceeded" errors
- Fixes session cleanup crashes every 5 minutes in production
- Prevents potential memory leaks from failed cleanup

**Testing:**
- Added regression test for infinite recursion prevention
- All 39 session management tests pass
- Build and typecheck succeed

Conceived by Romuald Członkowski - https://www.aiadvisors.pl/en

Closes #427

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

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",

src/http-server-single-session.ts

@@ -155,17 +155,22 @@ export class SingleSessionHTTPServer {
    */
   private async removeSession(sessionId: string, reason: string): Promise<void> {
     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 });

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<void>) | 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', () => {