From 45930a79412dc45f9d391cd7689d029fa4f0189e Mon Sep 17 00:00:00 2001 From: furszy Date: Wed, 28 Jan 2026 12:40:02 -0500 Subject: [PATCH] http-server: guard against crashes from unhandled exceptions Currently, if an exception is thrown at the top-level HTTP request handler (prior to invoking the command), the program crashes. Ideally, each handler should catch all exceptions internally and be responsible for sanitizing them and crafting the client response. This is because only the handler knows the correct response format, which differs per server type. However, because this cannot always be guaranteed, it is safer to also catch exceptions in the top-level server code, log the unexpected error, and disconnect the socket. This both guards against crashes caused by uncaught exceptions and prevents the client from hanging indefinitely while waiting for a response that will never arrive. The following diff can be used to trigger the crash in master (just run single node functional tests like feature_shutdown.py): ``` diff --git a/src/httprpc.cpp b/src/httprpc.cpp --- a/src/httprpc.cpp +++ b/src/httprpc.cpp @@ -103,6 +103,9 @@ static bool HTTPReq_JSONRPC(const std::any& context, HTTPRequest* req) { + static int i = 0; // skip initial requests as they are used in the RPC warmup phase. + if (i++ > 3) throw std::runtime_error("error from json rpc handler"); + // JSONRPC handles only POST if (req->GetRequestMethod() != HTTPRequest::POST) { req->WriteReply(HTTP_BAD_METHOD, "JSONRPC server handles only POST requests"); ``` Note: This leaves a TODO in the code because error responses should eventually be specialized per server type. REST clients expect plain text responses, while JSON-RPC clients expect a JSON error object. The TODO is there because this is not consistently enforced everywhere in the current codebase, and we should tackle them all at once. --- src/httpserver.cpp | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/httpserver.cpp b/src/httpserver.cpp index 71c6f5b1ee2..61df454af8e 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -327,7 +327,24 @@ static void http_request_cb(struct evhttp_request* req, void* arg) // Dispatch to worker thread if (i != iend) { - std::unique_ptr item(new HTTPWorkItem(std::move(hreq), path, i->handler)); + auto item = std::make_unique(std::move(hreq), path, [fn = i->handler](HTTPRequest* req, const std::string& path_inner) { + std::string err_msg; + try { + return fn(req, path_inner); + } catch (const std::exception& e) { + LogWarning("Unexpected error while processing request for '%s'. Error msg: '%s'", req->GetURI(), e.what()); + err_msg = e.what(); + } catch (...) { + LogWarning("Unknown error while processing request for '%s'", req->GetURI()); + err_msg = "unknown error"; + } + // Reply so the client doesn't hang waiting for the response. + req->WriteHeader("Connection", "close"); + // TODO: Implement specific error formatting for the REST and JSON-RPC servers responses. + req->WriteReply(HTTP_INTERNAL_SERVER_ERROR, err_msg); + return false; + }); + assert(g_work_queue); if (g_work_queue->Enqueue(item.get())) { (void)item.release(); /* if true, queue took ownership */