From e31866c4a2fd828380ff9710b7bf2dd17190e067 Mon Sep 17 00:00:00 2001 From: Roy Keene Date: Wed, 10 Oct 2018 22:05:34 -0500 Subject: [PATCH] Better error messages (#1225) --- rai/node/common.cpp | 59 ++++++++++++++++++++++ rai/node/common.hpp | 1 + rai/node/node.cpp | 118 ++++++++++++++++---------------------------- rai/node/stats.cpp | 24 +++++++++ rai/node/stats.hpp | 8 +++ 5 files changed, 134 insertions(+), 76 deletions(-) diff --git a/rai/node/common.cpp b/rai/node/common.cpp index 0496639c..4efb4c80 100644 --- a/rai/node/common.cpp +++ b/rai/node/common.cpp @@ -87,6 +87,65 @@ void rai::message_header::ipv4_only_set (bool value_a) // MTU - IP header - UDP header const size_t rai::message_parser::max_safe_udp_message_size = 508; +std::string rai::message_parser::status_string () +{ + switch (status) + { + case rai::message_parser::parse_status::success: + { + return "success"; + } + case rai::message_parser::parse_status::insufficient_work: + { + return "insufficient_work"; + } + case rai::message_parser::parse_status::invalid_header: + { + return "invalid_header"; + } + case rai::message_parser::parse_status::invalid_message_type: + { + return "invalid_message_type"; + } + case rai::message_parser::parse_status::invalid_keepalive_message: + { + return "invalid_keepalive_message"; + } + case rai::message_parser::parse_status::invalid_publish_message: + { + return "invalid_publish_message"; + } + case rai::message_parser::parse_status::invalid_confirm_req_message: + { + return "invalid_confirm_req_message"; + } + case rai::message_parser::parse_status::invalid_confirm_ack_message: + { + return "invalid_confirm_ack_message"; + } + case rai::message_parser::parse_status::invalid_node_id_handshake_message: + { + return "invalid_node_id_handshake_message"; + } + case rai::message_parser::parse_status::outdated_version: + { + return "outdated_version"; + } + case rai::message_parser::parse_status::invalid_magic: + { + return "invalid_magic"; + } + case rai::message_parser::parse_status::invalid_network: + { + return "invalid_network"; + } + } + + assert (false); + + return "[unknown parse_status]"; +} + rai::message_parser::message_parser (rai::message_visitor & visitor_a, rai::work_pool & pool_a) : visitor (visitor_a), pool (pool_a), diff --git a/rai/node/common.hpp b/rai/node/common.hpp index a5200ec4..892848b1 100644 --- a/rai/node/common.hpp +++ b/rai/node/common.hpp @@ -225,6 +225,7 @@ public: rai::message_visitor & visitor; rai::work_pool & pool; parse_status status; + std::string status_string (); static const size_t max_safe_udp_message_size; }; class keepalive : public message diff --git a/rai/node/node.cpp b/rai/node/node.cpp index 76407c5c..3c4a7a03 100644 --- a/rai/node/node.cpp +++ b/rai/node/node.cpp @@ -609,84 +609,50 @@ void rai::network::receive_action (rai::udp_data * data_a) { node.stats.inc (rai::stat::type::error); - if (parser.status == rai::message_parser::parse_status::insufficient_work) + switch (parser.status) { - if (node.config.logging.insufficient_work_logging ()) - { - BOOST_LOG (node.log) << "Insufficient work in message"; - } + case rai::message_parser::parse_status::insufficient_work: + // We've already increment error count, update detail only + node.stats.inc_detail_only (rai::stat::type::error, rai::stat::detail::insufficient_work); + break; + case rai::message_parser::parse_status::invalid_magic: + node.stats.inc (rai::stat::type::udp, rai::stat::detail::invalid_magic); + break; + case rai::message_parser::parse_status::invalid_network: + node.stats.inc (rai::stat::type::udp, rai::stat::detail::invalid_network); + break; + case rai::message_parser::parse_status::invalid_header: + node.stats.inc (rai::stat::type::udp, rai::stat::detail::invalid_header); + break; + case rai::message_parser::parse_status::invalid_message_type: + node.stats.inc (rai::stat::type::udp, rai::stat::detail::invalid_message_type); + break; + case rai::message_parser::parse_status::invalid_keepalive_message: + node.stats.inc (rai::stat::type::udp, rai::stat::detail::invalid_keepalive_message); + break; + case rai::message_parser::parse_status::invalid_publish_message: + node.stats.inc (rai::stat::type::udp, rai::stat::detail::invalid_publish_message); + break; + case rai::message_parser::parse_status::invalid_confirm_req_message: + node.stats.inc (rai::stat::type::udp, rai::stat::detail::invalid_confirm_req_message); + break; + case rai::message_parser::parse_status::invalid_confirm_ack_message: + node.stats.inc (rai::stat::type::udp, rai::stat::detail::invalid_confirm_ack_message); + break; + case rai::message_parser::parse_status::invalid_node_id_handshake_message: + node.stats.inc (rai::stat::type::udp, rai::stat::detail::invalid_node_id_handshake_message); + break; + case rai::message_parser::parse_status::outdated_version: + node.stats.inc (rai::stat::type::udp, rai::stat::detail::outdated_version); + break; + case rai::message_parser::parse_status::success: + /* Already checked, unreachable */ + break; + } - // We've already increment error count, update detail only - node.stats.inc_detail_only (rai::stat::type::error, rai::stat::detail::insufficient_work); - } - else if (parser.status == rai::message_parser::parse_status::invalid_message_type) + if (node.config.logging.network_logging ()) { - if (node.config.logging.network_logging ()) - { - BOOST_LOG (node.log) << "Invalid message type in message"; - } - } - else if (parser.status == rai::message_parser::parse_status::invalid_header) - { - if (node.config.logging.network_logging ()) - { - BOOST_LOG (node.log) << "Invalid header in message"; - } - } - else if (parser.status == rai::message_parser::parse_status::invalid_keepalive_message) - { - if (node.config.logging.network_logging ()) - { - BOOST_LOG (node.log) << "Invalid keepalive message"; - } - } - else if (parser.status == rai::message_parser::parse_status::invalid_publish_message) - { - if (node.config.logging.network_logging ()) - { - BOOST_LOG (node.log) << "Invalid publish message"; - } - } - else if (parser.status == rai::message_parser::parse_status::invalid_confirm_req_message) - { - if (node.config.logging.network_logging ()) - { - BOOST_LOG (node.log) << "Invalid confirm_req message"; - } - } - else if (parser.status == rai::message_parser::parse_status::invalid_confirm_ack_message) - { - if (node.config.logging.network_logging ()) - { - BOOST_LOG (node.log) << "Invalid confirm_ack message"; - } - } - else if (parser.status == rai::message_parser::parse_status::invalid_node_id_handshake_message) - { - if (node.config.logging.network_logging ()) - { - BOOST_LOG (node.log) << "Invalid node_id_handshake message"; - } - } - else if (parser.status == rai::message_parser::parse_status::invalid_magic) - { - if (node.config.logging.network_logging ()) - { - BOOST_LOG (node.log) << "Invalid magic in header"; - } - node.stats.inc (rai::stat::type::udp, rai::stat::detail::invalid_magic); - } - else if (parser.status == rai::message_parser::parse_status::invalid_network) - { - if (node.config.logging.network_logging ()) - { - BOOST_LOG (node.log) << "Invalid network in header"; - } - node.stats.inc (rai::stat::type::udp, rai::stat::detail::invalid_network); - } - else - { - BOOST_LOG (node.log) << "Could not deserialize buffer"; + BOOST_LOG (node.log) << "Could not parse message. Error: " << parser.status_string (); } } else @@ -1954,7 +1920,7 @@ void rai::gap_cache::vote (std::shared_ptr vote_a) { if (!node_l->bootstrap_initiator.in_progress ()) { - BOOST_LOG (node_l->log) << boost::str (boost::format ("Missing confirmed block %1%") % hash.to_string ()); + BOOST_LOG (node_l->log) << boost::str (boost::format ("Missing block %1% which has enough votes to warrant bootstrapping it") % hash.to_string ()); } node_l->bootstrap_initiator.bootstrap (); } diff --git a/rai/node/stats.cpp b/rai/node/stats.cpp index 4004d156..66a6051e 100644 --- a/rai/node/stats.cpp +++ b/rai/node/stats.cpp @@ -445,6 +445,30 @@ std::string rai::stat::detail_to_string (uint32_t key) case rai::stat::detail::invalid_network: res = "invalid_network"; break; + case rai::stat::detail::invalid_header: + res = "invalid_header"; + break; + case rai::stat::detail::invalid_message_type: + res = "invalid_message_type"; + break; + case rai::stat::detail::invalid_keepalive_message: + res = "invalid_keepalive_message"; + break; + case rai::stat::detail::invalid_publish_message: + res = "invalid_publish_message"; + break; + case rai::stat::detail::invalid_confirm_req_message: + res = "invalid_confirm_req_message"; + break; + case rai::stat::detail::invalid_confirm_ack_message: + res = "invalid_confirm_ack_message"; + break; + case rai::stat::detail::invalid_node_id_handshake_message: + res = "invalid_node_id_handshake_message"; + break; + case rai::stat::detail::outdated_version: + res = "outdated_version"; + break; } return res; } diff --git a/rai/node/stats.hpp b/rai/node/stats.hpp index d24322c8..f312f213 100644 --- a/rai/node/stats.hpp +++ b/rai/node/stats.hpp @@ -233,6 +233,14 @@ public: overflow, invalid_magic, invalid_network, + invalid_header, + invalid_message_type, + invalid_keepalive_message, + invalid_publish_message, + invalid_confirm_req_message, + invalid_confirm_ack_message, + invalid_node_id_handshake_message, + outdated_version, // peering handshake,