test: Retry all RPC commands to fix MacOS CI jobs

include/xrpl/protocol/TxFlags.h

@@ -134,7 +134,7 @@ constexpr std::uint32_t const tfTrustLine                  = 0x00000004;
 constexpr std::uint32_t const tfTransferable               = 0x00000008;
 
 // MPTokenIssuanceCreate flags:
-// NOTE - there is intentionally no flag here for lsfMPTLocked, which this transaction cannot mutate. 
+// NOTE - there is intentionally no flag here for lsfMPTLocked, which this transaction cannot mutate.
 constexpr std::uint32_t const tfMPTCanLock                 = lsfMPTCanLock;
 constexpr std::uint32_t const tfMPTRequireAuth             = lsfMPTRequireAuth;
 constexpr std::uint32_t const tfMPTCanEscrow               = lsfMPTCanEscrow;

src/test/app/MultiSign_test.cpp

@@ -644,6 +644,7 @@ class MultiSign_test : public beast::unit_test::suite
             Json::Value jv_submit;
             jv_submit[jss::tx_json] = jrr[jss::tx_json];
             jrr = env.rpc(
+                env.expectInternalRPCError,
                 "json",
                 "submit_multisigned",
                 to_string(jv_submit))[jss::result];

src/test/jtx/Env.h

@@ -292,6 +292,17 @@ class Env
         The command is examined and used to build
         the correct JSON as per the arguments.
     */
+    using rpcCallback = std::function<bool(Json::Value const&)>;
+    const rpcCallback useDefaultCallback{};
+    const rpcCallback expectInternalRPCError = [](Json::Value const&) {
+        return true;
+    };
+    rpcCallback
+    getInternalFailureCallback(bool shouldFail)
+    {
+        return shouldFail ? expectInternalRPCError : useDefaultCallback;
+    }
+
     template <class... Args>
     Json::Value
     rpc(unsigned apiVersion,
@@ -303,12 +314,23 @@ class Env
     Json::Value
     rpc(unsigned apiVersion, std::string const& cmd, Args&&... args);
 
+    template <class... Args>
+    Json::Value
+    rpc(rpcCallback cb,
+        std::unordered_map<std::string, std::string> const& headers,
+        std::string const& cmd,
+        Args&&... args);
+
     template <class... Args>
     Json::Value
     rpc(std::unordered_map<std::string, std::string> const& headers,
         std::string const& cmd,
         Args&&... args);
 
+    template <class... Args>
+    Json::Value
+    rpc(rpcCallback cb, std::string const& cmd, Args&&... args);
+
     template <class... Args>
     Json::Value
     rpc(std::string const& cmd, Args&&... args);
@@ -514,7 +536,7 @@ class Env
     /** Gets the TER result and `didApply` flag from a RPC Json result object.
      */
     static ParsedResult
-    parseResult(Json::Value const& jr);
+    parseResult(Json::Value const& jr, bool checkTER);
 
     /** Submit an existing JTx.
         This calls postconditions.
@@ -696,6 +718,7 @@ class Env
 
     Json::Value
     do_rpc(
+        rpcCallback cb,
         unsigned apiVersion,
         std::vector<std::string> const& args,
         std::unordered_map<std::string, std::string> const& headers = {});
@@ -746,6 +769,7 @@ Env::rpc(
     Args&&... args)
 {
     return do_rpc(
+        useDefaultCallback,
         apiVersion,
         std::vector<std::string>{cmd, std::forward<Args>(args)...},
         headers);
@@ -765,16 +789,39 @@ Env::rpc(unsigned apiVersion, std::string const& cmd, Args&&... args)
 template <class... Args>
 Json::Value
 Env::rpc(
+    rpcCallback cb,
     std::unordered_map<std::string, std::string> const& headers,
     std::string const& cmd,
     Args&&... args)
 {
     return do_rpc(
+        cb,
         RPC::apiCommandLineVersion,
         std::vector<std::string>{cmd, std::forward<Args>(args)...},
         headers);
 }
 
+template <class... Args>
+Json::Value
+Env::rpc(
+    std::unordered_map<std::string, std::string> const& headers,
+    std::string const& cmd,
+    Args&&... args)
+{
+    return rpc(useDefaultCallback, headers, cmd, std::forward<Args>(args)...);
+}
+
+template <class... Args>
+Json::Value
+Env::rpc(rpcCallback cb, std::string const& cmd, Args&&... args)
+{
+    return rpc(
+        cb,
+        std::unordered_map<std::string, std::string>(),
+        cmd,
+        std::forward<Args>(args)...);
+}
+
 template <class... Args>
 Json::Value
 Env::rpc(std::string const& cmd, Args&&... args)

src/test/jtx/impl/Env.cpp

@@ -273,7 +273,7 @@ Env::trust(STAmount const& amount, Account const& account)
 }
 
 Env::ParsedResult
-Env::parseResult(Json::Value const& jr)
+Env::parseResult(Json::Value const& jr, bool checkTER)
 {
     auto error = [](ParsedResult& parsed, Json::Value const& object) {
         // Use an error code that is not used anywhere in the transaction
@@ -296,14 +296,19 @@ Env::parseResult(Json::Value const& jr)
     if (jr.isObject() && jr.isMember(jss::result))
     {
         auto const& result = jr[jss::result];
-        if (result.isMember(jss::engine_result_code))
+        if (checkTER && result.isMember(jss::engine_result_code))
         {
             parsed.ter = TER::fromInt(result[jss::engine_result_code].asInt());
             parsed.rpcCode.emplace(rpcSUCCESS);
         }
+        else if (!checkTER && !result.isMember(jss::error))
+            parsed.rpcCode.emplace(rpcSUCCESS);
         else
             error(parsed, result);
     }
+    else if (
+        jr.isObject() && jr.isMember(jss::error) && jr[jss::error].isObject())
+        error(parsed, jr[jss::error]);
     else
         error(parsed, jr);
 
@@ -317,24 +322,20 @@ Env::submit(JTx const& jt)
     auto const jr = [&]() {
         if (jt.stx)
         {
-            // We shouldn't need to retry, but it fixes the test on macOS for
-            // the moment.
-            int retries = 3;
-            do
-            {
-                txid_ = jt.stx->getTransactionID();
-                Serializer s;
-                jt.stx->add(s);
-                auto const jr = rpc("submit", strHex(s.slice()));
-
-                parsedResult = parseResult(jr);
+            txid_ = jt.stx->getTransactionID();
+            Serializer s;
+            jt.stx->add(s);
+            auto const cb = [&](Json::Value const& jr) {
+                parsedResult = parseResult(jr, true);
                 test.expect(parsedResult.ter, "ter uninitialized!");
                 ter_ = parsedResult.ter.value_or(telENV_RPC_FAILED);
-                if (ter_ != telENV_RPC_FAILED ||
+                return (
+                    ter_ != telENV_RPC_FAILED ||
                     parsedResult.rpcCode != rpcINTERNAL ||
-                    jt.ter == telENV_RPC_FAILED || --retries <= 0)
-                    return jr;
-            } while (true);
+                    jt.ter == telENV_RPC_FAILED);
+            };
+            // rpc() will call cb(), which does all the parsing
+            return rpc(cb, "submit", strHex(s.slice()));
         }
         else
         {
@@ -379,7 +380,7 @@ Env::sign_and_submit(JTx const& jt, Json::Value params)
     if (!txid_.parseHex(jr[jss::result][jss::tx_json][jss::hash].asString()))
         txid_.zero();
 
-    ParsedResult const parsedResult = parseResult(jr);
+    ParsedResult const parsedResult = parseResult(jr, true);
     test.expect(parsedResult.ter, "ter uninitialized!");
     ter_ = parsedResult.ter.value_or(telENV_RPC_FAILED);
 
@@ -562,12 +563,37 @@ Env::ust(JTx const& jt)
 
 Json::Value
 Env::do_rpc(
+    rpcCallback cb,
     unsigned apiVersion,
     std::vector<std::string> const& args,
     std::unordered_map<std::string, std::string> const& headers)
 {
-    return rpcClient(args, app().config(), app().logs(), apiVersion, headers)
-        .second;
+    // We shouldn't need to retry, but it fixes the test on macOS for
+    // the moment.
+    if (!cb)
+    {
+        static auto const defaultCallback = [](Json::Value const& jr) {
+            auto const parsedResult = parseResult(jr, false);
+            return parsedResult.rpcCode != rpcINTERNAL;
+        };
+        cb = defaultCallback;
+    }
+    int retries = 3;
+    do
+    {
+        auto const ret =
+            rpcClient(args, app().config(), app().logs(), apiVersion, headers)
+                .second;
+        if (cb(ret) || --retries <= 0)
+            return ret;
+        std::stringstream ss;
+        for (auto const& arg : args)
+        {
+            ss << arg << ", ";
+        }
+        test.log << "RPC failed: " << ss.str() << " -> " << to_string(ret)
+                 << std::endl;
+    } while (true);
 }
 
 void

src/test/rpc/LedgerRPC_test.cpp

@@ -1638,7 +1638,10 @@ class LedgerRPC_test : public beast::unit_test::suite
                 });
 
             Json::Value const jrr = env.rpc(
-                "json", "ledger_entry", to_string(jvParams))[jss::result];
+                env.getInternalFailureCallback(apiVersion < 2u),
+                "json",
+                "ledger_entry",
+                to_string(jvParams))[jss::result];
 
             if (apiVersion < 2u)
                 checkErrorValue(jrr, "internal", "Internal error.");
@@ -1675,7 +1678,10 @@ class LedgerRPC_test : public beast::unit_test::suite
                     });
 
                 Json::Value const jrr = env.rpc(
-                    "json", "ledger_entry", to_string(jvParams))[jss::result];
+                    env.getInternalFailureCallback(apiVersion < 2u),
+                    "json",
+                    "ledger_entry",
+                    to_string(jvParams))[jss::result];
 
                 if (apiVersion < 2u)
                     checkErrorValue(jrr, "internal", "Internal error.");
@@ -1691,7 +1697,10 @@ class LedgerRPC_test : public beast::unit_test::suite
                     });
 
                 Json::Value const jrr = env.rpc(
-                    "json", "ledger_entry", to_string(jvParams))[jss::result];
+                    env.getInternalFailureCallback(apiVersion < 2u),
+                    "json",
+                    "ledger_entry",
+                    to_string(jvParams))[jss::result];
 
                 if (apiVersion < 2u)
                     checkErrorValue(jrr, "internal", "Internal error.");
@@ -1707,7 +1716,10 @@ class LedgerRPC_test : public beast::unit_test::suite
                     });
 
                 Json::Value const jrr = env.rpc(
-                    "json", "ledger_entry", to_string(jvParams))[jss::result];
+                    env.getInternalFailureCallback(apiVersion < 2u),
+                    "json",
+                    "ledger_entry",
+                    to_string(jvParams))[jss::result];
 
                 if (apiVersion < 2u)
                     checkErrorValue(jrr, "internal", "Internal error.");
@@ -1723,7 +1735,10 @@ class LedgerRPC_test : public beast::unit_test::suite
                     });
 
                 Json::Value const jrr = env.rpc(
-                    "json", "ledger_entry", to_string(jvParams))[jss::result];
+                    env.getInternalFailureCallback(apiVersion < 2u),
+                    "json",
+                    "ledger_entry",
+                    to_string(jvParams))[jss::result];
 
                 if (apiVersion < 2u)
                     checkErrorValue(jrr, "internal", "Internal error.");
@@ -1746,7 +1761,10 @@ class LedgerRPC_test : public beast::unit_test::suite
                     });
 
                 Json::Value const jrr = env.rpc(
-                    "json", "ledger_entry", to_string(jvParams))[jss::result];
+                    env.getInternalFailureCallback(apiVersion < 2u),
+                    "json",
+                    "ledger_entry",
+                    to_string(jvParams))[jss::result];
 
                 if (apiVersion < 2u)
                     checkErrorValue(jrr, "internal", "Internal error.");
@@ -1762,7 +1780,10 @@ class LedgerRPC_test : public beast::unit_test::suite
                     });
 
                 Json::Value const jrr = env.rpc(
-                    "json", "ledger_entry", to_string(jvParams))[jss::result];
+                    env.getInternalFailureCallback(apiVersion < 2u),
+                    "json",
+                    "ledger_entry",
+                    to_string(jvParams))[jss::result];
 
                 if (apiVersion < 2u)
                     checkErrorValue(jrr, "internal", "Internal error.");

src/test/rpc/LedgerRequestRPC_test.cpp

@@ -347,7 +347,8 @@ class LedgerRequestRPC_test : public beast::unit_test::suite
         auto const USD = gw["USD"];
         env.fund(XRP(100000), gw);
 
-        auto const result = env.rpc("ledger_request", "1")[jss::result];
+        auto const result = env.rpc(
+            env.expectInternalRPCError, "ledger_request", "1")[jss::result];
         // The current HTTP/S ServerHandler returns an HTTP 403 error code here
         // rather than a noPermission JSON error.  The JSONRPCClient just eats
         // that error and returns an null result.

src/test/rpc/ValidatorInfo_test.cpp

@@ -50,7 +50,8 @@ class ValidatorInfo_test : public beast::unit_test::suite
     {
         using namespace test::jtx;
         Env env{*this, envconfig(no_admin)};
-        auto const info = env.rpc("validator_info")[jss::result];
+        auto const info =
+            env.rpc(env.expectInternalRPCError, "validator_info")[jss::result];
         BEAST_EXPECT(info.isNull());
     }
 

src/test/rpc/ValidatorRPC_test.cpp

@@ -49,7 +49,8 @@ class ValidatorRPC_test : public beast::unit_test::suite
             for (std::string cmd : {"validators", "validator_list_sites"})
             {
                 Env env{*this, isAdmin ? envconfig() : envconfig(no_admin)};
-                auto const jrr = env.rpc(cmd)[jss::result];
+                auto const jrr = env.rpc(
+                    env.getInternalFailureCallback(!isAdmin), cmd)[jss::result];
                 if (isAdmin)
                 {
                     BEAST_EXPECT(!jrr.isMember(jss::error));