Skip to content

Commit c3abab2

Browse files
committed
refactor: Remove debug logging and SDK overrides
Removed temporary debugging logs and SDK method overrides that were added to diagnose mcp-inspector connection issues with the /mcp (Streamable HTTP) endpoint. Specifically: - Removed overrides for `mainHttpTransport.send` and `mainHttpTransport.onmessage` in `src/sse.ts`. - Removed verbose logging around `mainHttpTransport.handleRequest` in `src/sse.ts`. - Removed the explicit request handler for `initialize` in `src/mcp-proxy.ts`, reverting to the SDK's default handling. The core functionality for Streamable HTTP connections is now stable, and these debugging aids are no longer necessary.
1 parent f1aedc3 commit c3abab2

File tree

5 files changed

+67
-85
lines changed

5 files changed

+67
-85
lines changed

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ config/.session_secret
1111
browser-*
1212
chatmcp
1313
typescript-sdk
14+
inspector
1415
tools/**
1516
config/mcp_server.json
1617
config/tool_config.json

package-lock.json

Lines changed: 5 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "mcp-proxy-server",
3-
"version": "0.2.1",
3+
"version": "0.3.0",
44
"author": "ptbsare",
55
"license": "MIT",
66
"description": "An MCP proxy server that aggregates and serves multiple MCP resource servers through a single interface with stdio/sse support",
@@ -21,7 +21,7 @@
2121
"inspector": "npx @modelcontextprotocol/inspector build/index.js"
2222
},
2323
"dependencies": {
24-
"@modelcontextprotocol/sdk": "^1.09.0",
24+
"@modelcontextprotocol/sdk": "^1.11.3",
2525
"@types/cors": "^2.8.17",
2626
"@types/express-session": "^1.18.1",
2727
"cors": "^2.8.5",

src/mcp-proxy.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,7 @@ export const createServer = async () => {
197197

198198
// --- Request Handlers ---
199199
// These handlers now rely on the maps populated by updateBackendConnections
200+
// Note: InitializeRequest is handled by the SDK's Server default behavior.
200201

201202
server.setRequestHandler(ListToolsRequestSchema, async (request) => {
202203
console.log("Received tools/list request - applying overrides from config");

src/sse.ts

Lines changed: 58 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { SSEServerTransport } from "@modelcontextprotocol/sdk/server/sse.js";
2-
import { StreamableHTTPServerTransport } from "@modelcontextprotocol/sdk/server/streamableHttp.js"; // Import StreamableHTTPServerTransport
2+
import { StreamableHTTPServerTransport, StreamableHTTPServerTransportOptions } from "@modelcontextprotocol/sdk/server/streamableHttp.js"; // Import StreamableHTTPServerTransport and options
33
import express, { Request, Response, NextFunction } from "express";
44
import session from 'express-session';
55
import { ServerResponse } from "node:http"; // Import ServerResponse
@@ -38,9 +38,44 @@ const TOOL_CONFIG_PATH = path.resolve(__dirname, '..', 'config', 'tool_config.js
3838
const SECRET_FILE_PATH = path.resolve(__dirname, '..', 'config', '.session_secret');
3939
const publicPath = path.join(__dirname, '..', 'public');
4040

41+
const sseTransports = new Map<string, SSEServerTransport>();
42+
const streamableHttpTransports = new Map<string, StreamableHTTPServerTransport>(); // Define this map earlier
43+
4144
// createServer no longer returns connectedClients
4245
const { server, cleanup } = await createServer();
4346

47+
// Create and connect the main StreamableHTTPServerTransport for the /mcp endpoint at startup
48+
const mcpEndpointTransportKey = "main_mcp_transport";
49+
const mcpTransportOptions: StreamableHTTPServerTransportOptions = {
50+
sessionIdGenerator: undefined, // Stateless, as per current setup
51+
onsessioninitialized: (sessionId: string) => { // Added type for sessionId
52+
// This should not be called if sessionIdGenerator is undefined
53+
console.log(`[MCP Endpoint] Main transport session initialized: ${sessionId}`);
54+
},
55+
enableJsonResponse: false, // Revert to default SSE streaming for POST responses
56+
};
57+
const mainHttpTransport = new StreamableHTTPServerTransport(mcpTransportOptions);
58+
59+
try {
60+
await server.connect(mainHttpTransport);
61+
streamableHttpTransports.set(mcpEndpointTransportKey, mainHttpTransport);
62+
console.log("Main StreamableHTTPServerTransport for /mcp endpoint connected and ready.");
63+
64+
// Standard onclose and onerror handlers
65+
mainHttpTransport.onclose = () => {
66+
console.log("Main StreamableHTTPServerTransport for /mcp endpoint closed."); // Restored simpler log
67+
streamableHttpTransports.delete(mcpEndpointTransportKey);
68+
};
69+
mainHttpTransport.onerror = (error: Error) => {
70+
console.error("Main StreamableHTTPServerTransport for /mcp endpoint error:", error); // Restored simpler log
71+
streamableHttpTransports.delete(mcpEndpointTransportKey);
72+
};
73+
74+
} catch (e) {
75+
console.error("FATAL: Could not connect main StreamableHTTPServerTransport for /mcp endpoint at startup:", e);
76+
process.exit(1); // Exit if the main transport cannot be set up
77+
}
78+
4479
const allowedKeysRaw = process.env.ALLOWED_KEYS || ""; // Renamed
4580
const allowedKeys = new Set(allowedKeysRaw.split(',').map(k => k.trim()).filter(k => k.length > 0));
4681

@@ -498,8 +533,6 @@ if (enableAdminUI) {
498533
} // End of the main if (enableAdminUI) block
499534

500535

501-
const sseTransports = new Map<string, SSEServerTransport>();
502-
503536
app.get("/sse", async (req, res) => {
504537
const clientId = req.ip || `client-${Date.now()}`;
505538
console.log(`[${clientId}] SSE connection received`);
@@ -626,12 +659,6 @@ app.post("/mcp", async (req, res) => {
626659
const clientId = req.ip || `client-http-${Date.now()}`;
627660
console.log(`[${clientId}] Received POST request on /mcp`);
628661

629-
// Set headers for streaming JSON response
630-
res.setHeader('Content-Type', 'application/json');
631-
res.setHeader('Transfer-Encoding', 'chunked');
632-
res.setHeader('Cache-Control', 'no-cache');
633-
res.setHeader('Connection', 'keep-alive');
634-
635662
// Authentication check (similar to /sse)
636663
if (authEnabled) { // authEnabled is defined globally
637664
let authenticated = false;
@@ -665,84 +692,37 @@ app.post("/mcp", async (req, res) => {
665692
// If authentication is enabled but no valid credentials were provided
666693
if (!authenticated) {
667694
console.warn(`[${clientId}] Unauthorized /mcp connection attempt. No valid credentials provided.`);
668-
res.status(401).send('Unauthorized');
695+
res.status(401).send('Unauthorized'); // Send 401 and return
669696
return;
670697
}
671698
}
672699

700+
// Use the pre-initialized mainHttpTransport
701+
const httpTransport = streamableHttpTransports.get(mcpEndpointTransportKey);
673702

674-
// Create a new StreamableHTTPServerTransport for this request
675-
// Use undefined sessionIdGenerator for stateless proxy
676-
const httpTransport = new StreamableHTTPServerTransport({
677-
sessionIdGenerator: undefined, // Stateless
678-
enableJsonResponse: false, // Use streaming (default)
679-
// eventStore: undefined // No resumability needed for proxy
680-
});
681-
682-
// Set up the onmessage handler to forward messages to the internal server
683-
httpTransport.onmessage = async (message: JSONRPCMessage) => {
684-
try {
685-
// Forward the message to the internal MCP server instance
686-
// The internal server will call httpTransport.send() with responses/notifications
687-
// The server instance created in mcp-proxy.ts should have an onmessage handler
688-
// that processes incoming messages and uses its connected transports (including httpTransport)
689-
// to send responses back.
690-
// We don't directly call server.handleMessage here, as the transport is already connected
691-
// to the server instance and will trigger the server's onmessage handler.
692-
console.log(`[${clientId}] Forwarding message to internal server:`, JSON.stringify(message));
693-
// The server instance's onmessage handler is set up in mcp-proxy.ts
694-
// It will receive this message and process it.
695-
} catch (error) {
696-
console.error(`[${clientId}] Error handling message via internal server (should not happen if onmessage is set up correctly):`, error);
697-
// The transport's send method should handle writing errors back if possible
698-
// Or the transport's onerror might be triggered
699-
}
700-
};
701-
702-
// Set up onerror handler for the transport
703-
httpTransport.onerror = (error: Error) => {
704-
console.error(`[${clientId}] StreamableHTTP Transport error:`, error);
705-
// The transport should ideally handle closing the response on error
706-
if (!res.writableEnded) {
707-
try {
708-
// Attempt to send a JSON-RPC error response if headers haven't been sent
709-
if (!res.headersSent) {
710-
res.writeHead(500, { 'Content-Type': 'application/json' });
711-
}
712-
// Construct a generic JSON-RPC error response
713-
const errorResponse = {
714-
jsonrpc: "2.0",
715-
error: {
716-
code: -32603, // Internal error
717-
message: `Internal server error: ${error.message || error}`
718-
},
719-
id: null // Cannot determine original request id here easily
720-
};
721-
res.end(JSON.stringify(errorResponse) + '\n');
722-
} catch (e) {
723-
console.error(`[${clientId}] Failed to send error response after transport error:`, e);
724-
if (!res.writableEnded) {
725-
res.end(); // Just close the connection as a fallback
726-
}
727-
}
728-
}
729-
};
730-
731-
// Set up onclose handler for the transport (client disconnect)
732-
httpTransport.onclose = () => {
733-
console.log(`[${clientId}] StreamableHTTP Transport closed.`);
734-
// The transport should handle ending the response stream
735-
};
736-
703+
if (!httpTransport) {
704+
console.error(`[${clientId}] FATAL: Main StreamableHTTPServerTransport for /mcp not found during request! This should have been initialized at startup.`);
705+
if (!res.headersSent) {
706+
res.status(500).send("MCP transport not available");
707+
}
708+
return;
709+
}
710+
711+
// The mainHttpTransport's onmessage, onerror, onclose are already set up during startup
712+
// and connected to the main 'server' instance.
713+
// We don't need to (and shouldn't) override them here.
737714

715+
console.log(`[${clientId}] About to call mainHttpTransport.handleRequest for ${req.method} ${req.originalUrl}`);
738716
try {
739-
// Handle the incoming HTTP request using the transport
740-
// The transport will parse the body and call onmessage
717+
// Handle the incoming HTTP request using the pre-configured mainHttpTransport.
718+
// This will parse the body and, if successful, trigger the mainHttpTransport.onmessage
719+
// (which is our wrapper that calls the Server instance's handler).
741720
await httpTransport.handleRequest(req, res, req.body);
742721

743-
// Note: The response stream is managed by the httpTransport.
744-
// We do NOT call res.end() here. The transport will end the stream
745-
// when all responses are sent or on close/error.
722+
// The response stream (res) is now managed by httpTransport.
723+
// It will send SSE events (or a direct JSON if it were configured for it)
724+
// and will end the response when appropriate.
725+
console.log(`[${clientId}] mainHttpTransport.handleRequest completed for ${req.method} ${req.originalUrl}. Response stream is now managed by the transport.`);
746726

747727
} catch (error: any) {
748728
console.error(`[${clientId}] Error during StreamableHTTP Transport handling:`, error);

0 commit comments

Comments
 (0)