Skip to content

Commit ea12d1d

Browse files
bcdonadioclaude
andcommitted
fix: resolve SSE connection issues and prevent stack overflow on cleanup
This commit addresses three critical issues: 1. Fixed infinite recursion in transport cleanup - Added cleanup guard flags to prevent circular cleanup calls - Issue: cleanup() → server.close() → transport.close() → onclose → cleanup() - Affected both SSE and Streamable HTTP transports 2. Fixed 406 errors for SSE clients with non-standard Accept headers - Changed GET /mcp routing to default to SSE for sessionless requests - Previously rejected clients sending Accept: */* instead of text/event-stream - Now properly supports legacy clients like Kilo Code 3. Improved logging and diagnostics - Changed MCP server stderr logging from warn to debug level - Added comprehensive request logging for /mcp endpoints - Enhanced debugging for SSE and Streamable HTTP handlers 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent f914694 commit ea12d1d

File tree

4 files changed

+159
-41
lines changed

4 files changed

+159
-41
lines changed

README.md

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ This dual-interface approach means you can manage servers through the Hub's UI w
1616
| Category | Feature | Support | Notes |
1717
|----------|---------|---------|-------|
1818
| **Transport** ||||
19-
| | streamable-http || Primary transport protocol for remote servers |
20-
| | SSE || Fallback transport for remote servers |
19+
| | streamable-http || Primary transport protocol (client ↔ hub, hub ↔ servers) |
20+
| | SSE || Legacy transport for backward compatibility |
2121
| | STDIO || For running local servers |
2222
| **Authentication** ||||
2323
| | OAuth 2.0 || With PKCE flow |
@@ -55,12 +55,16 @@ Configure all MCP clients with just one endpoint:
5555
{
5656
"mcpServers" : {
5757
"Hub": {
58-
"url" : "http://localhost:37373/mcp"
58+
"url" : "http://localhost:37373/mcp"
5959
}
6060
}
6161
}
6262
```
6363

64+
**Transport Auto-Detection**: The Hub automatically detects which transport protocol your client uses:
65+
- **Modern clients**: Streamable HTTP transport (MCP 2025-03-26 spec)
66+
- **Legacy clients**: SSE transport (automatic fallback for backward compatibility)
67+
6468
The Hub automatically:
6569
- Namespaces capabilities to prevent conflicts (e.g., `filesystem__search` vs `database__search`)
6670
- Routes requests to the appropriate server
@@ -76,10 +80,20 @@ The Hub automatically:
7680
- Real-time capability updates when servers change
7781
- Simplified client configuration - just one endpoint instead of many
7882

83+
- **Modern Transport Layer**:
84+
- **Streamable HTTP**: Primary transport using MCP 2025-03-26 specification
85+
- Single unified endpoint for all client requests
86+
- Cryptographically secure session management
87+
- Optional DNS rebinding protection (configurable)
88+
- Efficient bidirectional communication
89+
- **SSE Fallback**: Automatic backward compatibility for legacy clients
90+
- Seamless detection and fallback
91+
- No configuration required
92+
7993
- **Dynamic Server Management**:
8094
- Start, stop, enable/disable servers on demand
8195
- Real-time configuration updates with automatic server reconnection
82-
- Support for local (STDIO) and remote (streamable-http/SSE) MCP servers
96+
- Support for local (STDIO) and remote (streamable-http/SSE) MCP servers
8397
- Health monitoring and automatic recovery
8498
- OAuth authentication with PKCE flow
8599
- Header-based token authentication
@@ -126,12 +140,14 @@ The main management server that:
126140
#### MCP Servers
127141
Connected services that:
128142
- Provide tools, resources, templates, and prompts
129-
- Support two connectivity modes:
130-
- Script-based STDIO servers for local operations
131-
- Remote servers (streamable-http/SSE) with OAuth support
143+
- Support three connectivity modes:
144+
- **STDIO**: Script-based servers for local operations
145+
- **Streamable HTTP**: Modern remote servers (MCP 2025-03-26)
146+
- **SSE**: Legacy remote servers (backward compatibility)
132147
- Implement real-time capability updates
133148
- Support automatic status recovery
134-
- Maintain consistent interface across transport types
149+
- OAuth authentication support for remote servers
150+
- Maintain consistent interface across all transport types
135151

136152
## Installation
137153

src/MCPConnection.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -682,7 +682,7 @@ export class MCPConnection extends EventEmitter {
682682
if (stderrStream) {
683683
stderrStream.on("data", (data) => {
684684
const errorOutput = data.toString().trim();
685-
logger.warn(`${this.name} stderr: ${errorOutput}`)
685+
logger.debug(`${this.name} stderr: ${errorOutput}`)
686686
});
687687
}
688688
return transport

src/mcp/server.js

Lines changed: 65 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -475,42 +475,65 @@ export class MCPServerEndpoint {
475475
* Handle SSE transport creation (GET /mcp)
476476
*/
477477
async handleSSEConnection(req, res) {
478+
logger.debug('handleSSEConnection called', {
479+
url: req.url,
480+
headers: req.headers,
481+
method: req.method
482+
});
478483

479-
// Create SSE transport
480-
const transport = new SSEServerTransport('/messages', res);
481-
const sessionId = transport.sessionId;
484+
try {
485+
// Create SSE transport
486+
logger.debug('Creating SSE transport');
487+
const transport = new SSEServerTransport('/messages', res);
488+
const sessionId = transport.sessionId;
489+
logger.debug('SSE transport created', { sessionId });
482490

483-
// Create a new server instance for this connection
484-
const server = this.createServer();
491+
// Create a new server instance for this connection
492+
const server = this.createServer();
493+
logger.debug('Server instance created for SSE connection');
485494

486-
// Store transport and server together
487-
this.clients.set(sessionId, { transport, server });
495+
// Store transport and server together
496+
this.clients.set(sessionId, { transport, server });
488497

489-
let clientInfo
498+
let clientInfo
499+
let cleanupCalled = false;
490500

491501

492-
// Setup cleanup on close
493-
const cleanup = async () => {
494-
this.clients.delete(sessionId);
495-
try {
496-
await server.close();
497-
} catch (error) {
498-
logger.warn(`Error closing server connected to ${clientInfo?.name ?? "Unknown"}: ${error.message}`);
499-
} finally {
500-
logger.info(`'${clientInfo?.name ?? "Unknown"}' client disconnected from MCP HUB`);
501-
}
502-
};
502+
// Setup cleanup on close
503+
const cleanup = async () => {
504+
if (cleanupCalled) return;
505+
cleanupCalled = true;
503506

504-
res.on("close", cleanup);
505-
transport.onclose = cleanup;
507+
this.clients.delete(sessionId);
508+
try {
509+
await server.close();
510+
} catch (error) {
511+
logger.warn(`Error closing server connected to ${clientInfo?.name ?? "Unknown"}: ${error.message}`);
512+
} finally {
513+
logger.info(`'${clientInfo?.name ?? "Unknown"}' client disconnected from MCP HUB`);
514+
}
515+
};
516+
517+
res.on("close", cleanup);
518+
transport.onclose = cleanup;
506519

507-
// Connect MCP server to transport
508-
await server.connect(transport);
509-
server.oninitialized = () => {
510-
clientInfo = server.getClientVersion()
511-
if (clientInfo) {
512-
logger.info(`'${clientInfo.name}' client connected to MCP HUB`)
520+
// Connect MCP server to transport
521+
logger.debug('Connecting server to SSE transport');
522+
await server.connect(transport);
523+
logger.debug('Server connected to SSE transport successfully');
524+
525+
server.oninitialized = () => {
526+
clientInfo = server.getClientVersion()
527+
if (clientInfo) {
528+
logger.info(`'${clientInfo.name}' client connected to MCP HUB`)
529+
}
513530
}
531+
} catch (error) {
532+
logger.error('SSE_CONNECTION_ERROR', 'Error in handleSSEConnection', {
533+
error: error.message,
534+
stack: error.stack
535+
});
536+
throw error;
514537
}
515538
}
516539

@@ -550,19 +573,30 @@ export class MCPServerEndpoint {
550573
* Supports both POST and GET requests on a single endpoint
551574
*/
552575
async handleStreamableHTTP(req, res) {
576+
logger.debug('handleStreamableHTTP called', {
577+
method: req.method,
578+
url: req.url,
579+
headers: req.headers,
580+
sessionId: req.headers['mcp-session-id'] || 'none'
581+
});
582+
553583
try {
554584
// Check if this is for an existing session
555585
const sessionId = req.headers['mcp-session-id'];
556586

557587
if (sessionId) {
588+
logger.debug('Session ID found in request', { sessionId });
558589
// Reuse existing transport for this session
559590
const clientInfo = this.clients.get(sessionId);
560591
if (clientInfo) {
592+
logger.debug('Reusing existing session', { sessionId });
561593
await clientInfo.transport.handleRequest(req, res, req.body);
562594
return;
563595
}
564596
// Session not found - will create new one below
565597
logger.debug(`Session ${sessionId} not found, creating new session`);
598+
} else {
599+
logger.debug('No session ID in request, will create new session');
566600
}
567601

568602
// Create new transport and server for new session
@@ -579,9 +613,13 @@ export class MCPServerEndpoint {
579613
const server = this.createServer();
580614

581615
let clientInfo;
616+
let cleanupCalled = false;
582617

583618
// Setup cleanup for when transport closes
584619
const cleanup = async () => {
620+
if (cleanupCalled) return;
621+
cleanupCalled = true;
622+
585623
if (transport.sessionId) {
586624
this.clients.delete(transport.sessionId);
587625
}

src/server.js

Lines changed: 69 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,18 @@ const SERVER_ID = "mcp-hub";
2222
// Create Express app
2323
const app = express();
2424
app.use(express.json());
25+
26+
// Log all incoming requests to /api routes
27+
router.use((req, res, next) => {
28+
logger.debug('API request received', {
29+
method: req.method,
30+
path: req.path,
31+
url: req.url,
32+
headers: req.headers
33+
});
34+
next();
35+
});
36+
2537
app.use("/api", router);
2638

2739
// Helper to determine HTTP status code from error type
@@ -321,12 +333,21 @@ class ServiceManager {
321333

322334
// Register SSE endpoint
323335
registerRoute("GET", "/events", "Subscribe to server events", async (req, res) => {
336+
logger.debug('SSE connection attempt', {
337+
url: req.url,
338+
path: req.path,
339+
headers: req.headers,
340+
method: req.method
341+
});
342+
324343
try {
325344
if (!serviceManager?.sseManager) {
345+
logger.error('SSE_MANAGER_NOT_INITIALIZED', 'SSE manager not initialized when client attempted to connect');
326346
throw new ServerError("SSE manager not initialized");
327347
}
328348
// Add client connection
329349
const connection = await serviceManager.sseManager.addConnection(req, res);
350+
logger.debug('SSE connection established successfully');
330351
// Send initial state
331352
connection.send(EventTypes.HUB_STATE, serviceManager.getState());
332353
} catch (error) {
@@ -344,14 +365,26 @@ registerRoute("GET", "/events", "Subscribe to server events", async (req, res) =
344365

345366
// Register unified MCP endpoint for both Streamable HTTP (new) and SSE (legacy)
346367
app.post("/mcp", async (req, res) => {
368+
logger.debug('POST /mcp request received', {
369+
url: req.url,
370+
headers: req.headers,
371+
contentType: req.headers['content-type'],
372+
body: req.body
373+
});
374+
347375
try {
348376
if (!mcpServerEndpoint) {
377+
logger.error('MCP_ENDPOINT_NOT_INITIALIZED', 'MCP server endpoint not initialized');
349378
throw new ServerError("MCP server endpoint not initialized");
350379
}
351380
// POST requests use Streamable HTTP transport (new protocol)
381+
logger.debug('Routing POST to Streamable HTTP handler');
352382
await mcpServerEndpoint.handleStreamableHTTP(req, res);
353383
} catch (error) {
354-
logger.warn(`Failed to handle MCP POST request: ${error.message}`);
384+
logger.warn(`Failed to handle MCP POST request: ${error.message}`, {
385+
error: error.message,
386+
stack: error.stack
387+
});
355388
if (!res.headersSent) {
356389
res.status(500).json({
357390
jsonrpc: "2.0",
@@ -366,25 +399,47 @@ app.post("/mcp", async (req, res) => {
366399
});
367400

368401
app.get("/mcp", async (req, res) => {
402+
logger.debug('GET /mcp request received', {
403+
url: req.url,
404+
headers: req.headers,
405+
method: req.method,
406+
query: req.query
407+
});
408+
369409
try {
370410
if (!mcpServerEndpoint) {
411+
logger.error('MCP_ENDPOINT_NOT_INITIALIZED', 'MCP server endpoint not initialized');
371412
throw new ServerError("MCP server endpoint not initialized");
372413
}
373414

374415
// Check if this is a Streamable HTTP GET (has Mcp-Session-Id header)
375-
// or legacy SSE (Accept: text/event-stream header)
416+
// or legacy SSE (no session ID means initial connection attempt)
376417
const sessionId = req.headers['mcp-session-id'];
377418
const acceptsSSE = req.headers.accept?.includes('text/event-stream');
378419

379-
if (sessionId || !acceptsSSE) {
420+
logger.debug('MCP GET request type detection', {
421+
sessionId: sessionId || 'none',
422+
accept: req.headers.accept || 'none',
423+
acceptsSSE,
424+
willUseStreamableHTTP: !!sessionId,
425+
willUseSSE: !sessionId
426+
});
427+
428+
if (sessionId) {
380429
// Streamable HTTP GET request (for server-to-client messages in active session)
430+
logger.debug('Routing to Streamable HTTP handler (has session ID)');
381431
await mcpServerEndpoint.handleStreamableHTTP(req, res);
382432
} else {
383433
// Legacy SSE transport (backward compatibility)
434+
// Any GET request without a session ID is assumed to be an SSE connection attempt
435+
logger.debug('Routing to SSE handler (no session ID)');
384436
await mcpServerEndpoint.handleSSEConnection(req, res);
385437
}
386438
} catch (error) {
387-
logger.warn(`Failed to handle MCP GET request: ${error.message}`);
439+
logger.warn(`Failed to handle MCP GET request: ${error.message}`, {
440+
error: error.message,
441+
stack: error.stack
442+
});
388443
if (!res.headersSent) {
389444
res.status(500).send('Error establishing MCP connection');
390445
}
@@ -951,9 +1006,18 @@ router.use((err, req, res, next) => {
9511006
method: req.method,
9521007
});
9531008

1009+
const statusCode = getStatusCode(error);
1010+
logger.warn('Router error handler triggered', {
1011+
path: req.path,
1012+
method: req.method,
1013+
statusCode,
1014+
error: error.message,
1015+
code: error.code
1016+
});
1017+
9541018
// Only send error response if headers haven't been sent
9551019
if (!res.headersSent) {
956-
res.status(getStatusCode(error)).json({
1020+
res.status(statusCode).json({
9571021
error: error.message,
9581022
code: error.code,
9591023
data: error.data,

0 commit comments

Comments
 (0)