Address PR feedback

Move some log messages from error to warning verbosity, make boost log attribute configuration more compact, and use m_id.hex() rather than first writing the value to a stream.
ethereum · May 23, 2019 · e8a7f36 · e8a7f36
1 parent d025427
commit e8a7f36
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 23 deletions.
diff --git a/libethereum/EthereumCapability.cpp b/libethereum/EthereumCapability.cpp
@@ -842,13 +842,13 @@ bool EthereumCapability::interpretCapabilityPacket(
     }
     catch (Exception const&)
     {
-        LOG(m_loggerError) << "Peer " << _peerID << " causing an exception: "
-                           << boost::current_exception_diagnostic_information() << " " << _r;
+        LOG(m_loggerWarn) << "Peer " << _peerID << " causing an exception: "
+                          << boost::current_exception_diagnostic_information() << " " << _r;
     }
     catch (std::exception const& _e)
     {
-        LOG(m_loggerError) << "Peer " << _peerID << " causing an exception: " << _e.what() << " "
-                           << _r;
+        LOG(m_loggerWarn) << "Peer " << _peerID << " causing an exception: " << _e.what() << " "
+                          << _r;
     }
 
     return true;

diff --git a/libethereum/EthereumCapability.h b/libethereum/EthereumCapability.h
@@ -191,6 +191,7 @@ class EthereumCapability : public p2p::CapabilityFace
     Logger m_logger{createLogger(VerbosityDebug, "ethcap")};
     Logger m_loggerDetail{createLogger(VerbosityTrace, "ethcap")};
     Logger m_loggerError{createLogger(VerbosityError, "ethcap")};
+    Logger m_loggerWarn{createLogger(VerbosityWarning, "ethcap")};
     /// Logger for messages about impolite behaivour of peers.
     Logger m_loggerImpolite{createLogger(VerbosityDebug, "impolite")};
 };

diff --git a/libethereum/EthereumPeer.h b/libethereum/EthereumPeer.h
@@ -22,10 +22,7 @@ class EthereumPeer
         u256 const& /*_capabilityVersion*/)
       : m_host(std::move(_host)), m_id(_peerID)
     {
-        std::stringstream remoteInfoStream;
-        remoteInfoStream << m_id;
-        m_logger.add_attribute(
-            "Suffix", boost::log::attributes::constant<std::string>(remoteInfoStream.str()));
+        m_logger.add_attribute("Suffix", boost::log::attributes::constant<std::string>{m_id.hex()});
     }
 
     bool statusReceived() const { return m_protocolVersion != 0; }

diff --git a/libp2p/Session.cpp b/libp2p/Session.cpp
@@ -27,17 +27,13 @@ Session::Session(Host* _h, unique_ptr<RLPXFrameCoder>&& _io, std::shared_ptr<RLP
     stringstream remoteInfoStream;
     remoteInfoStream << "(" << m_info.id << "@" << m_socket->remoteEndpoint() << ")";
 
-    m_netLogger.add_attribute(
-        "Suffix", boost::log::attributes::constant<std::string>(remoteInfoStream.str()));
-    m_netLoggerDetail.add_attribute(
-        "Suffix", boost::log::attributes::constant<std::string>(remoteInfoStream.str()));
-    m_netLoggerError.add_attribute(
-        "Suffix", boost::log::attributes::constant<std::string>(remoteInfoStream.str()));
-
-    m_capLogger.add_attribute(
-        "Suffix", boost::log::attributes::constant<std::string>(remoteInfoStream.str()));
-    m_capLoggerDetail.add_attribute(
-        "Suffix", boost::log::attributes::constant<std::string>(remoteInfoStream.str()));
+    auto const attr = boost::log::attributes::constant<std::string>{remoteInfoStream.str()};
+    m_netLogger.add_attribute("Suffix", attr);
+    m_netLoggerDetail.add_attribute("Suffix", attr);
+    m_netLoggerError.add_attribute("Suffix", attr);
+
+    m_capLogger.add_attribute("Suffix", attr);
+    m_capLoggerDetail.add_attribute("Suffix", attr);
 
     m_peer->m_lastDisconnect = NoDisconnect;
     m_lastReceived = m_connect = chrono::steady_clock::now();
@@ -126,9 +122,9 @@ bool Session::readPacket(uint16_t _capId, unsigned _packetType, RLP const& _r)
     }
     catch (std::exception const& _e)
     {
-        LOG(m_netLoggerError) << "Exception caught in p2p::Session::readPacket(): " << _e.what()
-                              << ". PacketType: " << capabilityPacketTypeToString(_packetType)
-                              << " (" << _packetType << "). RLP: " << _r;
+        LOG(m_netLogger) << "Exception caught in p2p::Session::readPacket(): " << _e.what()
+                         << ". PacketType: " << capabilityPacketTypeToString(_packetType) << " ("
+                         << _packetType << "). RLP: " << _r;
         disconnect(BadProtocol);
         return true;
     }
@@ -330,7 +326,7 @@ void Session::doRead()
                 return;
             else if (!m_io->authAndDecryptHeader(bytesRef(m_data.data(), length)))
             {
-                LOG(m_netLoggerError) << "Header decrypt failed";
+                LOG(m_netLogger) << "Header decrypt failed";
                 drop(BadProtocol);  // todo: better error
                 return;
             }