Fix: Address code review feedback from gemini-code-assist

Python bridge (scripts/mcp/proxysql_mcp_stdio_bridge.py):
- Make log file path configurable via PROXYSQL_MCP_BRIDGE_LOG env var
- Add httpx.RequestError exception handling for network issues
- Fix asyncio.CancelledError not being re-raised (HIGH priority)
- Replace deprecated asyncio.get_event_loop() with get_running_loop()

C++ server (lib/MCP_Endpoint.cpp):
- Refactor handle_tools_call() to reduce code duplication
- Handle string responses directly without calling .dump()
- Single shared wrapping block for all response types

Per review: https://github.com/ProxySQL/proxysql-vec/pull/11
pull/5310/head
Rene Cannao 3 months ago
parent 2ceaac049c
commit 606fe2e93c

@ -359,26 +359,23 @@ json MCP_JSONRPC_Resource::handle_tools_call(const json& req_json) {
mcp_result["isError"] = true;
return mcp_result;
}
// Success - wrap result in MCP-compliant format with content array
// Per MCP spec: https://modelcontextprotocol.io/specification/2025-11-25/server/tools
json actual_result = response["result"];
json mcp_result;
mcp_result["content"] = json::array();
json text_content;
text_content["type"] = "text";
text_content["text"] = actual_result.dump(2); // Pretty-print JSON with 2-space indent
mcp_result["content"].push_back(text_content);
mcp_result["isError"] = false;
return mcp_result;
// Success - use the "result" field as the content to be wrapped
response = response["result"];
}
// Fallback: wrap response in MCP format (for compatibility with non-standard handlers)
// Wrap the response (or the 'result' field) in MCP-compliant format
// Per MCP spec: https://modelcontextprotocol.io/specification/2025-11-25/server/tools
json mcp_result;
mcp_result["content"] = json::array();
json text_content;
text_content["type"] = "text";
text_content["text"] = response.dump(2);
mcp_result["content"].push_back(text_content);
if (response.is_string()) {
text_content["text"] = response.get<std::string>();
} else {
text_content["text"] = response.dump(2); // Pretty-print JSON with 2-space indent
}
mcp_result["content"] = json::array({text_content});
mcp_result["isError"] = false;
return mcp_result;
}

@ -34,7 +34,9 @@ from datetime import datetime
import httpx
# Minimal logging to file for debugging
_log_file = open("/tmp/proxysql_mcp_bridge.log", "a", buffering=1)
# Log path can be configured via PROXYSQL_MCP_BRIDGE_LOG environment variable
_log_file_path = os.getenv("PROXYSQL_MCP_BRIDGE_LOG", "/tmp/proxysql_mcp_bridge.log")
_log_file = open(_log_file_path, "a", buffering=1)
def _log(msg):
_log_file.write(f"[{datetime.now().strftime('%H:%M:%S.%f')[:-3]}] {msg}\n")
_log_file.flush()
@ -105,6 +107,15 @@ class ProxySQLMCPEndpoint:
},
"id": request.get("id", "")
}
except httpx.RequestError as e:
return {
"jsonrpc": "2.0",
"error": {
"code": -32002,
"message": f"Request to ProxySQL failed: {e}"
},
"id": request.get("id", "")
}
except Exception as e:
return {
"jsonrpc": "2.0",
@ -172,12 +183,14 @@ class StdioMCPServer:
except json.JSONDecodeError as e:
await self._write_error(-32700, f"Parse error: {e}", "")
except asyncio.CancelledError:
raise # Re-raise to allow proper task cancellation
except Exception as e:
await self._write_error(-32603, f"Internal error: {e}", "")
async def _readline(self) -> Optional[str]:
"""Read a line from stdin."""
loop = asyncio.get_event_loop()
loop = asyncio.get_running_loop()
line = await loop.run_in_executor(None, sys.stdin.readline)
if not line:
return None
@ -185,7 +198,7 @@ class StdioMCPServer:
async def _writeline(self, data: Any):
"""Write JSON data to stdout."""
loop = asyncio.get_event_loop()
loop = asyncio.get_running_loop()
output = json.dumps(data, ensure_ascii=False) + "\n"
_log(f"WRITE stdout: {len(output)} bytes: {repr(output[:200])}")
await loop.run_in_executor(None, sys.stdout.write, output)

Loading…
Cancel
Save