Skip to content

Commit e031a56

Browse files
PatrickSysPatrickSys
committed
fix: guard against unhandled rejections and resource leaks in HTTP transport
Three issues flagged in code review: 1. Wrap entire request handler in try/catch — any await that throws (malformed init, SDK I/O error) previously caused an unhandled promise rejection that could crash the long-lived server process. 2. DELETE handler: use try/finally so sessions.delete() always runs even if transport.close() rejects, preventing session map leaks. 3. Init path: if transport.sessionId is null after handleRequest (malformed request), close the orphaned Server+Transport pair immediately rather than silently abandoning it.
1 parent d0b036f commit e031a56

File tree

1 file changed

+97
-81
lines changed

1 file changed

+97
-81
lines changed

‎src/server/http.ts‎

Lines changed: 97 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -95,112 +95,128 @@ export async function startHttpServer(options: HttpServerOptions): Promise<HttpS
9595
timeoutCheck.unref();
9696

9797
const httpServer = createHttpServer(async (req: IncomingMessage, res: ServerResponse) => {
98-
const url = new URL(req.url ?? '/', `http://${host}:${port}`);
98+
try {
99+
const url = new URL(req.url ?? '/', `http://${host}:${port}`);
99100

100-
// Health check on root
101-
if (url.pathname === '/' && req.method === 'GET') {
102-
res.writeHead(200, { 'Content-Type': 'application/json' });
103-
res.end(JSON.stringify({ status: 'ok', sessions: sessions.size }));
104-
return;
105-
}
106-
107-
// Only handle /mcp
108-
if (url.pathname !== '/mcp') {
109-
res.writeHead(404, { 'Content-Type': 'application/json' });
110-
res.end(JSON.stringify({ error: 'Not found' }));
111-
return;
112-
}
113-
114-
const method = req.method?.toUpperCase();
115-
116-
if (method === 'POST') {
117-
const sessionId = req.headers['mcp-session-id'] as string | undefined;
118-
119-
if (sessionId && sessions.has(sessionId)) {
120-
// Existing session — delegate to its transport
121-
touchSession(sessionId);
122-
const session = sessions.get(sessionId)!;
123-
await session.transport.handleRequest(req, res);
101+
// Health check on root
102+
if (url.pathname === '/' && req.method === 'GET') {
103+
res.writeHead(200, { 'Content-Type': 'application/json' });
104+
res.end(JSON.stringify({ status: 'ok', sessions: sessions.size }));
124105
return;
125106
}
126107

127-
if (sessionId && !sessions.has(sessionId)) {
128-
// Unknown session ID — 404
108+
// Only handle /mcp
109+
if (url.pathname !== '/mcp') {
129110
res.writeHead(404, { 'Content-Type': 'application/json' });
130-
res.end(JSON.stringify({ error: 'Session not found' }));
111+
res.end(JSON.stringify({ error: 'Not found' }));
131112
return;
132113
}
133114

134-
// No session ID — new initialization request
135-
const { server: mcpServer, transport } = createSessionServer();
115+
const method = req.method?.toUpperCase();
136116

137-
// Connect server to transport
138-
await mcpServer.connect(transport);
117+
if (method === 'POST') {
118+
const sessionId = req.headers['mcp-session-id'] as string | undefined;
139119

140-
// Register onclose before handleRequest so cleanup always fires.
141-
transport.onclose = () => {
142-
const sid = transport.sessionId;
143-
if (sid && sessions.has(sid)) {
144-
sessions.delete(sid);
145-
console.error(`[HTTP] Session ${sid} disconnected`);
120+
if (sessionId && sessions.has(sessionId)) {
121+
// Existing session — delegate to its transport
122+
touchSession(sessionId);
123+
const session = sessions.get(sessionId)!;
124+
await session.transport.handleRequest(req, res);
125+
return;
146126
}
147-
};
148127

149-
// Handle the init request — this generates the session ID
150-
await transport.handleRequest(req, res);
128+
if (sessionId && !sessions.has(sessionId)) {
129+
// Unknown session ID — 404
130+
res.writeHead(404, { 'Content-Type': 'application/json' });
131+
res.end(JSON.stringify({ error: 'Session not found' }));
132+
return;
133+
}
151134

152-
// After handleRequest, the session ID is available
153-
const newSessionId = transport.sessionId;
154-
if (newSessionId) {
155-
sessions.set(newSessionId, {
156-
server: mcpServer,
157-
transport,
158-
lastActivity: Date.now()
159-
});
160-
console.error(`[HTTP] Session ${newSessionId} connected`);
135+
// No session ID — new initialization request
136+
const { server: mcpServer, transport } = createSessionServer();
161137

162-
// Notify caller so they can set up per-session handlers (roots refresh, etc.)
163-
options.onSessionReady?.(mcpServer);
138+
// Connect server to transport
139+
await mcpServer.connect(transport);
140+
141+
// Register onclose before handleRequest so cleanup always fires.
142+
transport.onclose = () => {
143+
const sid = transport.sessionId;
144+
if (sid && sessions.has(sid)) {
145+
sessions.delete(sid);
146+
console.error(`[HTTP] Session ${sid} disconnected`);
147+
}
148+
};
149+
150+
// Handle the init request — this generates the session ID
151+
await transport.handleRequest(req, res);
152+
153+
// After handleRequest, the session ID is available
154+
const newSessionId = transport.sessionId;
155+
if (newSessionId) {
156+
sessions.set(newSessionId, {
157+
server: mcpServer,
158+
transport,
159+
lastActivity: Date.now()
160+
});
161+
console.error(`[HTTP] Session ${newSessionId} connected`);
162+
163+
// Notify caller so they can set up per-session handlers (roots refresh, etc.)
164+
options.onSessionReady?.(mcpServer);
165+
} else {
166+
// Malformed init request — SDK didn't assign a session ID; clean up the orphan.
167+
void transport.close().catch(() => {
168+
/* best effort */
169+
});
170+
}
171+
172+
return;
164173
}
165174

166-
return;
167-
}
175+
if (method === 'GET') {
176+
// SSE streaming for server-initiated messages
177+
const sessionId = req.headers['mcp-session-id'] as string | undefined;
178+
if (!sessionId || !sessions.has(sessionId)) {
179+
res.writeHead(400, { 'Content-Type': 'application/json' });
180+
res.end(JSON.stringify({ error: 'Missing or invalid session ID' }));
181+
return;
182+
}
168183

169-
if (method === 'GET') {
170-
// SSE streaming for server-initiated messages
171-
const sessionId = req.headers['mcp-session-id'] as string | undefined;
172-
if (!sessionId || !sessions.has(sessionId)) {
173-
res.writeHead(400, { 'Content-Type': 'application/json' });
174-
res.end(JSON.stringify({ error: 'Missing or invalid session ID' }));
184+
touchSession(sessionId);
185+
const session = sessions.get(sessionId)!;
186+
await session.transport.handleRequest(req, res);
175187
return;
176188
}
177189

178-
touchSession(sessionId);
179-
const session = sessions.get(sessionId)!;
180-
await session.transport.handleRequest(req, res);
181-
return;
182-
}
190+
if (method === 'DELETE') {
191+
const sessionId = req.headers['mcp-session-id'] as string | undefined;
192+
if (!sessionId || !sessions.has(sessionId)) {
193+
res.writeHead(404, { 'Content-Type': 'application/json' });
194+
res.end(JSON.stringify({ error: 'Session not found' }));
195+
return;
196+
}
183197

184-
if (method === 'DELETE') {
185-
const sessionId = req.headers['mcp-session-id'] as string | undefined;
186-
if (!sessionId || !sessions.has(sessionId)) {
187-
res.writeHead(404, { 'Content-Type': 'application/json' });
188-
res.end(JSON.stringify({ error: 'Session not found' }));
198+
const session = sessions.get(sessionId)!;
199+
try {
200+
await session.transport.close();
201+
} finally {
202+
sessions.delete(sessionId);
203+
console.error(`[HTTP] Session ${sessionId} closed by client`);
204+
}
205+
res.writeHead(200, { 'Content-Type': 'application/json' });
206+
res.end(JSON.stringify({ status: 'session_closed' }));
189207
return;
190208
}
191209

192-
const session = sessions.get(sessionId)!;
193-
await session.transport.close();
194-
sessions.delete(sessionId);
195-
console.error(`[HTTP] Session ${sessionId} closed by client`);
196-
res.writeHead(200, { 'Content-Type': 'application/json' });
197-
res.end(JSON.stringify({ status: 'session_closed' }));
198-
return;
210+
// Method not allowed
211+
res.writeHead(405, { 'Content-Type': 'application/json' });
212+
res.end(JSON.stringify({ error: 'Method not allowed' }));
213+
} catch (err) {
214+
console.error('[HTTP] Unhandled request error:', err);
215+
if (!res.headersSent) {
216+
res.writeHead(500, { 'Content-Type': 'application/json' });
217+
res.end(JSON.stringify({ error: 'Internal server error' }));
218+
}
199219
}
200-
201-
// Method not allowed
202-
res.writeHead(405, { 'Content-Type': 'application/json' });
203-
res.end(JSON.stringify({ error: 'Method not allowed' }));
204220
});
205221

206222
// Handle server errors

0 commit comments

Comments
 (0)