mirror of
https://github.com/czlonkowski/n8n-mcp.git
synced 2026-04-04 00:23:08 +00:00
fix: resolve SSE reconnection loop with separate /sse + /messages endpoints (v2.46.1) (#699)
Fix SSE clients entering rapid reconnection loops because POST /mcp never routed messages to SSEServerTransport.handlePostMessage() (#617). Root cause: SSE sessions were stored in a separate `this.session` property invisible to the StreamableHTTP POST handler. The POST handler only checked `this.transports` (StreamableHTTP map), so SSE messages were never delivered, causing immediate reconnection and rate limiter exhaustion. Changes: - Add GET /sse + POST /messages endpoints following the official MCP SDK backward-compatible server pattern (separate endpoints per transport) - Store SSE transports in the shared this.transports map with instanceof guards for type discrimination - Remove legacy this.session singleton, resetSessionSSE(), and isExpired() - Extract duplicated auth logic into authenticateRequest() method - Add Bearer token auth and rate limiting to SSE endpoints - Add skipSuccessfulRequests to authLimiter to prevent 429 storms - Mark SSE transport as deprecated (removed in MCP SDK v2.x) The handleRequest() codepath used by the downstream SaaS backend (N8NMCPEngine.processRequest()) is unchanged. Session persistence (exportSessionState/restoreSessionState) is unchanged. Closes #617 Conceived by Romuald Członkowski - https://www.aiadvisors.pl/en Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
committed by
GitHub
parent
2d4115530c
commit
12d7d5bdb6
@@ -219,9 +219,23 @@ describe('HTTP Server n8n Mode', () => {
|
||||
mcp: {
|
||||
method: 'POST',
|
||||
path: '/mcp',
|
||||
description: 'Main MCP JSON-RPC endpoint',
|
||||
description: 'Main MCP JSON-RPC endpoint (StreamableHTTP)',
|
||||
authentication: 'Bearer token required'
|
||||
},
|
||||
sse: {
|
||||
method: 'GET',
|
||||
path: '/sse',
|
||||
description: 'DEPRECATED: SSE stream for legacy clients. Migrate to StreamableHTTP (POST /mcp).',
|
||||
authentication: 'Bearer token required',
|
||||
deprecated: true
|
||||
},
|
||||
messages: {
|
||||
method: 'POST',
|
||||
path: '/messages',
|
||||
description: 'DEPRECATED: Message delivery for SSE sessions. Migrate to StreamableHTTP (POST /mcp).',
|
||||
authentication: 'Bearer token required',
|
||||
deprecated: true
|
||||
},
|
||||
health: {
|
||||
method: 'GET',
|
||||
path: '/health',
|
||||
|
||||
@@ -59,11 +59,24 @@ vi.mock('@modelcontextprotocol/sdk/server/streamableHttp.js', () => ({
|
||||
})
|
||||
}));
|
||||
|
||||
vi.mock('@modelcontextprotocol/sdk/server/sse.js', () => ({
|
||||
SSEServerTransport: vi.fn().mockImplementation(() => ({
|
||||
close: vi.fn().mockResolvedValue(undefined)
|
||||
}))
|
||||
}));
|
||||
vi.mock('@modelcontextprotocol/sdk/server/sse.js', () => {
|
||||
class MockSSEServerTransport {
|
||||
sessionId: string;
|
||||
onclose: (() => void) | null = null;
|
||||
onerror: ((error: Error) => void) | null = null;
|
||||
close = vi.fn().mockResolvedValue(undefined);
|
||||
handlePostMessage = vi.fn().mockImplementation(async (_req: any, res: any) => {
|
||||
res.writeHead(202);
|
||||
res.end('Accepted');
|
||||
});
|
||||
start = vi.fn().mockResolvedValue(undefined);
|
||||
|
||||
constructor(_endpoint: string, _res: any) {
|
||||
this.sessionId = 'sse-' + Math.random().toString(36).substring(2, 11);
|
||||
}
|
||||
}
|
||||
return { SSEServerTransport: MockSSEServerTransport };
|
||||
});
|
||||
|
||||
vi.mock('../../src/mcp/server', () => ({
|
||||
N8NDocumentationMCPServer: vi.fn().mockImplementation(() => ({
|
||||
@@ -1100,24 +1113,16 @@ describe('HTTP Server Session Management', () => {
|
||||
'session-2': { lastAccess: new Date(), createdAt: new Date() }
|
||||
};
|
||||
|
||||
// Set up legacy session for SSE compatibility
|
||||
const mockLegacyTransport = { close: vi.fn().mockResolvedValue(undefined) };
|
||||
(server as any).session = {
|
||||
transport: mockLegacyTransport
|
||||
};
|
||||
|
||||
await server.shutdown();
|
||||
|
||||
// All transports should be closed
|
||||
expect(mockTransport1.close).toHaveBeenCalled();
|
||||
expect(mockTransport2.close).toHaveBeenCalled();
|
||||
expect(mockLegacyTransport.close).toHaveBeenCalled();
|
||||
|
||||
// All data structures should be cleared
|
||||
expect(Object.keys((server as any).transports)).toHaveLength(0);
|
||||
expect(Object.keys((server as any).servers)).toHaveLength(0);
|
||||
expect(Object.keys((server as any).sessionMetadata)).toHaveLength(0);
|
||||
expect((server as any).session).toBe(null);
|
||||
});
|
||||
|
||||
it('should handle transport close errors during shutdown', async () => {
|
||||
@@ -1169,22 +1174,21 @@ describe('HTTP Server Session Management', () => {
|
||||
expect(Array.isArray(sessionInfo.sessions!.sessionIds)).toBe(true);
|
||||
});
|
||||
|
||||
it('should show legacy SSE session when present', async () => {
|
||||
it('should show active when transports exist', async () => {
|
||||
server = new SingleSessionHTTPServer();
|
||||
|
||||
// Mock legacy session
|
||||
const mockSession = {
|
||||
sessionId: 'sse-session-123',
|
||||
// Add a transport to simulate an active session
|
||||
(server as any).transports['session-123'] = { close: vi.fn() };
|
||||
(server as any).sessionMetadata['session-123'] = {
|
||||
lastAccess: new Date(),
|
||||
isSSE: true
|
||||
createdAt: new Date()
|
||||
};
|
||||
(server as any).session = mockSession;
|
||||
|
||||
const sessionInfo = server.getSessionInfo();
|
||||
|
||||
expect(sessionInfo.active).toBe(true);
|
||||
expect(sessionInfo.sessionId).toBe('sse-session-123');
|
||||
expect(sessionInfo.age).toBeGreaterThanOrEqual(0);
|
||||
expect(sessionInfo.sessions!.total).toBe(1);
|
||||
expect(sessionInfo.sessions!.sessionIds).toContain('session-123');
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user