rpc: refactor single/batch requests

Simplify the request handling flow so that errors and results only
come from JSONRPCExec()

3 files changed, 24 insertions, 35 deletions

diff --git a/src/httprpc.cpp b/src/httprpc.cpp
index aef20c24fc..53f994efd7 100644
--- a/src/httprpc.cpp
+++ b/src/httprpc.cpp
@@ -185,7 +185,7 @@ static bool HTTPReq_JSONRPC(const std::any& context, HTTPRequest* req)
         // Set the URI
         jreq.URI = req->GetURI();
 
-        std::string strReply;
+        UniValue reply;
         bool user_has_whitelist = g_rpc_whitelist.count(jreq.authUser);
         if (!user_has_whitelist && g_rpc_whitelist_default) {
             LogPrintf("RPC User %s not allowed to call any methods\n", jreq.authUser);
@@ -200,13 +200,12 @@ static bool HTTPReq_JSONRPC(const std::any& context, HTTPRequest* req)
                 req->WriteReply(HTTP_FORBIDDEN);
                 return false;
             }
-            UniValue result = tableRPC.execute(jreq);
 
-            // Send reply
-            strReply = JSONRPCReplyObj(std::move(result), NullUniValue, jreq.id).write() + "\n";
+            reply = JSONRPCExec(jreq);
 
         // array of requests
         } else if (valRequest.isArray()) {
+            // Check authorization for each request's method
             if (user_has_whitelist) {
                 for (unsigned int reqIdx = 0; reqIdx < valRequest.size(); reqIdx++) {
                     if (!valRequest[reqIdx].isObject()) {
@@ -223,13 +222,26 @@ static bool HTTPReq_JSONRPC(const std::any& context, HTTPRequest* req)
                     }
                 }
             }
-            strReply = JSONRPCExecBatch(jreq, valRequest.get_array());
+
+            // Execute each request
+            reply = UniValue::VARR;
+            for (size_t i{0}; i < valRequest.size(); ++i) {
+                // Batches include errors in the batch response, they do not throw
+                try {
+                    jreq.parse(valRequest[i]);
+                    reply.push_back(JSONRPCExec(jreq));
+                } catch (UniValue& e) {
+                    reply.push_back(JSONRPCReplyObj(NullUniValue, std::move(e), jreq.id));
+                } catch (const std::exception& e) {
+                    reply.push_back(JSONRPCReplyObj(NullUniValue, JSONRPCError(RPC_PARSE_ERROR, e.what()), jreq.id));
+                }
+            }
         }
         else
             throw JSONRPCError(RPC_PARSE_ERROR, "Top-level object parse error");
 
         req->WriteHeader("Content-Type", "application/json");
-        req->WriteReply(HTTP_OK, strReply);
+        req->WriteReply(HTTP_OK, reply.write() + "\n");
     } catch (UniValue& e) {
         JSONErrorReply(req, std::move(e), jreq.id);
         return false;
diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp
index fd18a7f9ec..f199914095 100644
--- a/src/rpc/server.cpp
+++ b/src/rpc/server.cpp
@@ -358,36 +358,13 @@ bool IsDeprecatedRPCEnabled(const std::string& method)
     return find(enabled_methods.begin(), enabled_methods.end(), method) != enabled_methods.end();
 }
 
-static UniValue JSONRPCExecOne(JSONRPCRequest jreq, const UniValue& req)
+UniValue JSONRPCExec(const JSONRPCRequest& jreq)
 {
-    UniValue rpc_result(UniValue::VOBJ);
+    // Might throw exception. Single requests will throw and send HTTP error codes
+    // but inside a batch, we just include the error object and return HTTP 200
+    UniValue result = tableRPC.execute(jreq);
 
-    try {
-        jreq.parse(req);
-
-        UniValue result = tableRPC.execute(jreq);
-        rpc_result = JSONRPCReplyObj(std::move(result), NullUniValue, jreq.id);
-    }
-    catch (UniValue& e)
-    {
-        rpc_result = JSONRPCReplyObj(NullUniValue, std::move(e), jreq.id);
-    }
-    catch (const std::exception& e)
-    {
-        rpc_result = JSONRPCReplyObj(NullUniValue,
-                                     JSONRPCError(RPC_PARSE_ERROR, e.what()), jreq.id);
-    }
-
-    return rpc_result;
-}
-
-std::string JSONRPCExecBatch(const JSONRPCRequest& jreq, const UniValue& vReq)
-{
-    UniValue ret(UniValue::VARR);
-    for (unsigned int reqIdx = 0; reqIdx < vReq.size(); reqIdx++)
-        ret.push_back(JSONRPCExecOne(jreq, vReq[reqIdx]));
-
-    return ret.write() + "\n";
+    return JSONRPCReplyObj(std::move(result), NullUniValue, jreq.id);
 }
 
 /**
diff --git a/src/rpc/server.h b/src/rpc/server.h
index 9a49d82570..2761bcd9db 100644
--- a/src/rpc/server.h
+++ b/src/rpc/server.h
@@ -181,7 +181,7 @@ extern CRPCTable tableRPC;
 void StartRPC();
 void InterruptRPC();
 void StopRPC();
-std::string JSONRPCExecBatch(const JSONRPCRequest& jreq, const UniValue& vReq);
+UniValue JSONRPCExec(const JSONRPCRequest& jreq);
 
 // Drop witness when serializing for RPC?
 bool RPCSerializationWithoutWitness();