From 08a76c86dc58b67a03e750a05c81b78f21895928 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Wed, 30 Oct 2024 18:08:45 -0400 Subject: [PATCH] test: Retry all RPC commands to fix MacOS CI jobs * Follow up to #5120 (23991c9), which added a retry for submit commands. It improved MacOS test reliability, but other tests are failing now. * Retry all failed RPC connections / commands in unit tests. --- include/xrpl/protocol/TxFlags.h | 2 +- src/test/jtx/Env.h | 41 ++++++++++++++++++++- src/test/jtx/impl/Env.cpp | 65 +++++++++++++++++++++++---------- 3 files changed, 86 insertions(+), 22 deletions(-) diff --git a/include/xrpl/protocol/TxFlags.h b/include/xrpl/protocol/TxFlags.h index 4894f48a7f9..c83f2a964a8 100644 --- a/include/xrpl/protocol/TxFlags.h +++ b/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; diff --git a/src/test/jtx/Env.h b/src/test/jtx/Env.h index d90d2bc1228..4c33d4a681e 100644 --- a/src/test/jtx/Env.h +++ b/src/test/jtx/Env.h @@ -292,6 +292,9 @@ class Env The command is examined and used to build the correct JSON as per the arguments. */ + using rpcCallback = std::function; + const rpcCallback useDefaultCallback{}; + template Json::Value rpc(unsigned apiVersion, @@ -303,12 +306,23 @@ class Env Json::Value rpc(unsigned apiVersion, std::string const& cmd, Args&&... args); + template + Json::Value + rpc(rpcCallback cb, + std::unordered_map const& headers, + std::string const& cmd, + Args&&... args); + template Json::Value rpc(std::unordered_map const& headers, std::string const& cmd, Args&&... args); + template + Json::Value + rpc(rpcCallback cb, std::string const& cmd, Args&&... args); + template Json::Value rpc(std::string const& cmd, Args&&... args); @@ -514,7 +528,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 = true); /** Submit an existing JTx. This calls postconditions. @@ -696,6 +710,7 @@ class Env Json::Value do_rpc( + rpcCallback cb, unsigned apiVersion, std::vector const& args, std::unordered_map const& headers = {}); @@ -746,6 +761,7 @@ Env::rpc( Args&&... args) { return do_rpc( + useDefaultCallback, apiVersion, std::vector{cmd, std::forward(args)...}, headers); @@ -765,16 +781,39 @@ Env::rpc(unsigned apiVersion, std::string const& cmd, Args&&... args) template Json::Value Env::rpc( + rpcCallback cb, std::unordered_map const& headers, std::string const& cmd, Args&&... args) { return do_rpc( + cb, RPC::apiCommandLineVersion, std::vector{cmd, std::forward(args)...}, headers); } +template +Json::Value +Env::rpc( + std::unordered_map const& headers, + std::string const& cmd, + Args&&... args) +{ + return rpc(useDefaultCallback, headers, cmd, std::forward(args)...); +} + +template +Json::Value +Env::rpc(rpcCallback cb, std::string const& cmd, Args&&... args) +{ + return rpc( + cb, + std::unordered_map(), + cmd, + std::forward(args)...); +} + template Json::Value Env::rpc(std::string const& cmd, Args&&... args) diff --git a/src/test/jtx/impl/Env.cpp b/src/test/jtx/impl/Env.cpp index ef5a2124e24..2d3d177590b 100644 --- a/src/test/jtx/impl/Env.cpp +++ b/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,36 @@ Env::ust(JTx const& jt) Json::Value Env::do_rpc( + rpcCallback cb, unsigned apiVersion, std::vector const& args, std::unordered_map 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); + } while (true); } void