From 53ecda7730f2416fcf9b8a5bef1d1f02733c157b Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Sun, 18 Jan 2026 13:19:39 +0000 Subject: [PATCH] fix: Add comprehensive error handling and logging for MCP tools - Add try-catch around handle_jsonrpc_request to catch unexpected exceptions - Add detailed logging for tool execution success/failure - Add proper SQLite error checking in create_agent_run with error messages - Fix json_int/json_double to handle both numbers and numeric strings The json_int function was throwing exceptions when receiving numeric strings (e.g., "14" instead of 14) from clients, causing 500 errors. Now it handles both formats gracefully. Also added logging so tool failures are visible in logs instead of being silent 500 errors. --- lib/Discovery_Schema.cpp | 23 ++++++++++++-- lib/MCP_Endpoint.cpp | 65 ++++++++++++++++++++++++++++---------- lib/Query_Tool_Handler.cpp | 35 ++++++++++++++++++-- 3 files changed, 101 insertions(+), 22 deletions(-) diff --git a/lib/Discovery_Schema.cpp b/lib/Discovery_Schema.cpp index 25cb8bbdb..4c1fe59ac 100644 --- a/lib/Discovery_Schema.cpp +++ b/lib/Discovery_Schema.cpp @@ -540,17 +540,34 @@ int Discovery_Schema::create_agent_run( const char* sql = "INSERT INTO agent_runs(run_id, model_name, prompt_hash, budget_json) VALUES(?1, ?2, ?3, ?4);"; int rc = db->prepare_v2(sql, &stmt); - if (rc != SQLITE_OK) return -1; + if (rc != SQLITE_OK) { + proxy_error("Failed to prepare agent_runs insert: %s\n", sqlite3_errstr(rc)); + return -1; + } (*proxy_sqlite3_bind_int)(stmt, 1, run_id); (*proxy_sqlite3_bind_text)(stmt, 2, model_name.c_str(), -1, SQLITE_TRANSIENT); (*proxy_sqlite3_bind_text)(stmt, 3, prompt_hash.c_str(), -1, SQLITE_TRANSIENT); (*proxy_sqlite3_bind_text)(stmt, 4, budget_json.c_str(), -1, SQLITE_TRANSIENT); - SAFE_SQLITE3_STEP2(stmt); - int agent_run_id = (int)sqlite3_last_insert_rowid(db->get_db()); + // Execute with proper error checking + int step_rc = SQLITE_OK; + do { + step_rc = (*proxy_sqlite3_step)(stmt); + if (step_rc == SQLITE_LOCKED || step_rc == SQLITE_BUSY) { + usleep(100); + } + } while (step_rc == SQLITE_LOCKED || step_rc == SQLITE_BUSY); + (*proxy_sqlite3_finalize)(stmt); + if (step_rc != SQLITE_DONE) { + proxy_error("Failed to insert into agent_runs (run_id=%d): %s\n", run_id, sqlite3_errstr(step_rc)); + return -1; + } + + int agent_run_id = (int)sqlite3_last_insert_rowid(db->get_db()); + proxy_info("Created agent_run_id=%d for run_id=%d\n", agent_run_id, run_id); return agent_run_id; } diff --git a/lib/MCP_Endpoint.cpp b/lib/MCP_Endpoint.cpp index 3112224cc..bc8f3552d 100644 --- a/lib/MCP_Endpoint.cpp +++ b/lib/MCP_Endpoint.cpp @@ -127,22 +127,24 @@ std::string MCP_JSONRPC_Resource::create_jsonrpc_error( std::shared_ptr MCP_JSONRPC_Resource::handle_jsonrpc_request( const httpserver::http_request& req ) { - // Update statistics - if (handler) { - handler->status_variables.total_requests++; - } + // Wrap entire request handling in try-catch to catch any unexpected exceptions + try { + // Update statistics + if (handler) { + handler->status_variables.total_requests++; + } - // Get request body - std::string req_body = req.get_content(); - std::string req_path = req.get_path(); + // Get request body + std::string req_body = req.get_content(); + std::string req_path = req.get_path(); - proxy_debug(PROXY_DEBUG_GENERIC, 2, "MCP request on %s: %s\n", req_path.c_str(), req_body.c_str()); + proxy_debug(PROXY_DEBUG_GENERIC, 2, "MCP request on %s: %s\n", req_path.c_str(), req_body.c_str()); - // Validate JSON - json req_json; - try { - req_json = json::parse(req_body); - } catch (json::parse_error& e) { + // Validate JSON + json req_json; + try { + req_json = json::parse(req_body); + } catch (json::parse_error& e) { proxy_error("MCP request on %s: Invalid JSON - %s\n", req_path.c_str(), e.what()); if (handler) { handler->status_variables.failed_requests++; @@ -251,6 +253,34 @@ std::shared_ptr MCP_JSONRPC_Resource::handle_jsonrpc_request( )); response->with_header("Content-Type", "application/json"); return response; + + } catch (const std::exception& e) { + // Catch any unexpected exceptions and return a proper error response + std::string req_path = req.get_path(); + proxy_error("MCP request on %s: Unexpected exception - %s\n", req_path.c_str(), e.what()); + if (handler) { + handler->status_variables.failed_requests++; + } + auto response = std::shared_ptr(new string_response( + create_jsonrpc_error(-32603, "Internal error: " + std::string(e.what()), ""), + http::http_utils::http_internal_server_error + )); + response->with_header("Content-Type", "application/json"); + return response; + } catch (...) { + // Catch any other exceptions + std::string req_path = req.get_path(); + proxy_error("MCP request on %s: Unknown exception\n", req_path.c_str()); + if (handler) { + handler->status_variables.failed_requests++; + } + auto response = std::shared_ptr(new string_response( + create_jsonrpc_error(-32603, "Internal error: Unknown exception", ""), + http::http_utils::http_internal_server_error + )); + response->with_header("Content-Type", "application/json"); + return response; + } } const std::shared_ptr MCP_JSONRPC_Resource::render_POST( @@ -349,18 +379,21 @@ json MCP_JSONRPC_Resource::handle_tools_call(const json& req_json) { if (response.is_object() && response.contains("success") && response.contains("result")) { bool success = response["success"].get(); if (!success) { - // Tool execution failed - return error in MCP format + // Tool execution failed - log the error and return in MCP format + std::string error_msg = response.contains("error") ? response["error"].get() : "Tool execution failed"; + proxy_error("MCP TOOL CALL FAILED: endpoint='%s' tool='%s' error='%s'\n", + endpoint_name.c_str(), tool_name.c_str(), error_msg.c_str()); json mcp_result; mcp_result["content"] = json::array(); json error_content; error_content["type"] = "text"; - std::string error_msg = response.contains("error") ? response["error"].get() : "Tool execution failed"; error_content["text"] = error_msg; mcp_result["content"].push_back(error_content); mcp_result["isError"] = true; return mcp_result; } - // Success - use the "result" field as the content to be wrapped + // Success - log and use the "result" field as the content to be wrapped + proxy_info("MCP TOOL CALL SUCCESS: endpoint='%s' tool='%s'\n", endpoint_name.c_str(), tool_name.c_str()); response = response["result"]; } diff --git a/lib/Query_Tool_Handler.cpp b/lib/Query_Tool_Handler.cpp index 14586000e..90c3d13a0 100644 --- a/lib/Query_Tool_Handler.cpp +++ b/lib/Query_Tool_Handler.cpp @@ -24,17 +24,46 @@ static std::string json_string(const json& j, const std::string& key, const std: return default_val; } -// Helper to safely get int from JSON +// Helper to safely get int from JSON - handles both numbers and numeric strings static int json_int(const json& j, const std::string& key, int default_val = 0) { if (j.contains(key) && !j[key].is_null()) { - return j[key].get(); + const json& val = j[key]; + // If it's already a number, return it + if (val.is_number()) { + return val.get(); + } + // If it's a string, try to parse it as an int + if (val.is_string()) { + std::string s = val.get(); + try { + return std::stoi(s); + } catch (...) { + // Parse failed, return default + return default_val; + } + } } return default_val; } +// Helper to safely get double from JSON - handles both numbers and numeric strings static double json_double(const json& j, const std::string& key, double default_val = 0.0) { if (j.contains(key) && !j[key].is_null()) { - return j[key].get(); + const json& val = j[key]; + // If it's already a number, return it + if (val.is_number()) { + return val.get(); + } + // If it's a string, try to parse it as a double + if (val.is_string()) { + std::string s = val.get(); + try { + return std::stod(s); + } catch (...) { + // Parse failed, return default + return default_val; + } + } } return default_val; }