From 1766276aad6da67ce4104517c10a7819c15c691f Mon Sep 17 00:00:00 2001 From: Lee Clagett Date: Wed, 3 Mar 2021 23:27:22 -0500 Subject: [PATCH] Check for JSON-RPC errors, and add better logging for it --- src/rest_server.cpp | 12 +++++++----- src/rpc/client.cpp | 27 +++++++++++++++++++++++++++ src/rpc/client.h | 19 +++++++++---------- src/scanner.cpp | 3 ++- src/util/CMakeLists.txt | 4 ++-- 5 files changed, 47 insertions(+), 18 deletions(-) diff --git a/src/rest_server.cpp b/src/rest_server.cpp index 256a193..5123d5c 100644 --- a/src/rest_server.cpp +++ b/src/rest_server.cpp @@ -43,12 +43,14 @@ #include "lmdb/util.h" // monero/src #include "net/http_base.h" // monero/contrib/epee/include #include "net/net_parse_helpers.h" // monero/contrib/epee/include +#include "rpc/client.h" #include "rpc/daemon_messages.h" // monero/src #include "rpc/light_wallet.h" #include "rpc/rates.h" #include "util/http_server.h" #include "util/gamma_picker.h" #include "util/random_outputs.h" +#include "util/source_location.h" #include "wire/json.h" namespace lws @@ -367,7 +369,7 @@ namespace lws epee::byte_slice msg = rpc::client::make_message("get_output_histogram", histogram_req); MONERO_CHECK(client->send(std::move(msg), std::chrono::seconds{10})); - auto histogram_resp = client->receive(std::chrono::minutes{3}); + auto histogram_resp = client->receive(std::chrono::minutes{3}, MLWS_CURRENT_LOCATION); if (!histogram_resp) return histogram_resp.error(); if (histogram_resp->histogram.size() != histogram_req.amounts.size()) @@ -396,7 +398,7 @@ namespace lws MONERO_CHECK(client->send(std::move(msg), std::chrono::seconds{10})); auto distribution_resp = - client->receive(std::chrono::minutes{3}); + client->receive(std::chrono::minutes{3}, MLWS_CURRENT_LOCATION); if (!distribution_resp) return distribution_resp.error(); @@ -445,7 +447,7 @@ namespace lws epee::byte_slice msg = rpc::client::make_message("get_output_keys", keys_req); MONERO_CHECK(client->send(std::move(msg), std::chrono::seconds{10})); - auto keys_resp = client->receive(std::chrono::seconds{10}); + auto keys_resp = client->receive(std::chrono::seconds{10}, MLWS_CURRENT_LOCATION); if (!keys_resp) return keys_resp.error(); @@ -526,7 +528,7 @@ namespace lws if (received < std::uint64_t(req.amount)) return {lws::error::account_not_found}; - const auto resp = client->receive(std::chrono::seconds{20}); + const auto resp = client->receive(std::chrono::seconds{20}, MLWS_CURRENT_LOCATION); if (!resp) return resp.error(); @@ -638,7 +640,7 @@ namespace lws epee::byte_slice message = rpc::client::make_message("send_raw_tx_hex", daemon_req); MONERO_CHECK(client->send(std::move(message), std::chrono::seconds{10})); - const auto daemon_resp = client->receive(std::chrono::seconds{20}); + const auto daemon_resp = client->receive(std::chrono::seconds{20}, MLWS_CURRENT_LOCATION); if (!daemon_resp) return daemon_resp.error(); if (!daemon_resp->relayed) diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 13a399d..4ce7048 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -37,6 +37,7 @@ #include "misc_log_ex.h" // monero/contrib/epee/include #include "net/http_client.h" // monero/contrib/epee/include #include "net/zmq.h" // monero/src +#include "serialization/json_object.h" // monero/src namespace lws { @@ -160,6 +161,32 @@ namespace rpc }; } // detail + expect client::get_response(cryptonote::rpc::Message& response, const std::chrono::seconds timeout, const source_location loc) + { + expect message = get_message(timeout); + if (!message) + return message.error(); + + try + { + cryptonote::rpc::FullMessage fm{std::move(*message)}; + const cryptonote::rpc::error json_error = fm.getError(); + if (!json_error.use) + { + response.fromJson(fm.getMessage()); + return success(); + } + + MERROR("Server returned RPC error: " << json_error.message << " with code " << json_error.code << " called from " << loc); + } + catch (const cryptonote::json::JSON_ERROR& error) + { + MERROR("Failed to parse json response: " << error.what() << " called from " << loc); + } + + return {lws::error::bad_daemon_response}; + } + expect client::get_message(std::chrono::seconds timeout) { MONERO_PRECOND(ctx != nullptr); diff --git a/src/rpc/client.h b/src/rpc/client.h index d403641..08d37a0 100644 --- a/src/rpc/client.h +++ b/src/rpc/client.h @@ -38,6 +38,7 @@ #include "rpc/message.h" // monero/src #include "rpc/daemon_pub.h" #include "rpc/rates.h" +#include "util/source_location.h" namespace lws { @@ -70,6 +71,9 @@ namespace rpc : ctx(std::move(ctx)), daemon(), daemon_sub(), signal_sub() {} + //! Expect `response` as the next message payload unless error. + expect get_response(cryptonote::rpc::Message& response, std::chrono::seconds timeout, source_location loc); + public: //! A client with no connection (all send/receive functions fail). explicit client() noexcept @@ -125,18 +129,13 @@ namespace rpc //! \return Next available RPC message response from server expect get_message(std::chrono::seconds timeout); - //! \return RPC response `M`, waiting a max of `timeout` seconds. + //! \return RPC response `M`, waiting a max of `timeout` seconds. Log errors as from `loc`. template - expect receive(std::chrono::seconds timeout) + expect receive(const std::chrono::seconds timeout, const source_location loc = {}) { - expect message = get_message(timeout); - if (!message) - return message.error(); - - cryptonote::rpc::FullMessage fm{std::move(*message)}; - M out{}; - out.fromJson(fm.getMessage()); - return out; + M response{}; + MONERO_CHECK(get_response(response, timeout, loc)); + return response; } /*! diff --git a/src/scanner.cpp b/src/scanner.cpp index c99d969..b9ac71c 100644 --- a/src/scanner.cpp +++ b/src/scanner.cpp @@ -50,6 +50,7 @@ #include "rpc/daemon_messages.h" // monero/src #include "rpc/daemon_zmq.h" #include "rpc/json.h" +#include "util/source_location.h" #include "util/transactions.h" #include "wire/json.h" @@ -705,7 +706,7 @@ namespace lws expect resp{lws::error::daemon_timeout}; start = std::chrono::steady_clock::now(); - while (!(resp = client.receive(std::chrono::seconds{1}))) + while (!(resp = client.receive(std::chrono::seconds{1}, MLWS_CURRENT_LOCATION))) { if (!scanner::is_running()) return {lws::error::signal_abort_process}; diff --git a/src/util/CMakeLists.txt b/src/util/CMakeLists.txt index 66816cf..009a8ff 100644 --- a/src/util/CMakeLists.txt +++ b/src/util/CMakeLists.txt @@ -26,8 +26,8 @@ # STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF # THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -set(monero-lws-util_sources gamma_picker.cpp random_outputs.cpp transactions.cpp) -set(monero-lws-util_headers fwd.h gamma_picker.h http_server.h random_outputs.h transactions.h) +set(monero-lws-util_sources gamma_picker.cpp random_outputs.cpp source_location.cpp transactions.cpp) +set(monero-lws-util_headers fwd.h gamma_picker.h http_server.h random_outputs.h source_location.h transactions.h) add_library(monero-lws-util ${monero-lws-util_sources} ${monero-lws-util_headers}) target_link_libraries(monero-lws-util monero::libraries)