Skip to content

Commit

Permalink
Merge bitcoin#29833: i2p: fix and improve logs
Browse files Browse the repository at this point in the history
7d3662f i2p: fix log when an interruption happens during `Accept` (brunoerg)
3d3a83f i2p: log errors properly according to their severity (brunoerg)

Pull request description:

  This PR improves and fixes i2p logs (joint work with vasild).

  - It replaces `LogPrint` to `LogPrintLevel` so we can log according to the severity.
  - Fix log when interruption happens during `Accept`. Before this PR, when an interruption happens, it just logs "Error accepting:", no reason is logged as it does for other situations. This PR changes it to log "Accept interrupted".
  - Log errors according to the severity. Stuff like creating SAM session, destroying SAM session, etc... are logged as 'debug'.

ACKs for top commit:
  achow101:
    ACK 7d3662f
  marcofleon:
    ACK 7d3662f.
  vasild:
    ACK 7d3662f

Tree-SHA512: 1c3d92108dbc22833f37a78e18b4efd723433d10f28166d17c74eab884cd97e908b4e0a0908fd16288df895eb2eb480f781de37b2ec6a6d414abfb71e0c86fe2
  • Loading branch information
achow101 committed Jun 26, 2024
2 parents 9ac4f69 + 7d3662f commit b27afb7
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 23 deletions.
28 changes: 13 additions & 15 deletions src/i2p.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ bool Session::Listen(Connection& conn)
conn.sock = StreamAccept();
return true;
} catch (const std::runtime_error& e) {
Log("Error listening: %s", e.what());
LogPrintLevel(BCLog::I2P, BCLog::Level::Error, "Couldn't listen: %s\n", e.what());
CheckControlSock();
}
return false;
Expand Down Expand Up @@ -204,7 +204,11 @@ bool Session::Accept(Connection& conn)
return true;
}

Log("Error accepting%s: %s", disconnect ? " (will close the session)" : "", errmsg);
if (*m_interrupt) {
LogPrintLevel(BCLog::I2P, BCLog::Level::Debug, "Accept was interrupted\n");
} else {
LogPrintLevel(BCLog::I2P, BCLog::Level::Debug, "Error accepting%s: %s\n", disconnect ? " (will close the session)" : "", errmsg);
}
if (disconnect) {
LOCK(m_mutex);
Disconnect();
Expand All @@ -219,7 +223,7 @@ bool Session::Connect(const CService& to, Connection& conn, bool& proxy_error)
// Refuse connecting to arbitrary ports. We don't specify any destination port to the SAM proxy
// when connecting (SAM 3.1 does not use ports) and it forces/defaults it to I2P_SAM31_PORT.
if (to.GetPort() != I2P_SAM31_PORT) {
Log("Error connecting to %s, connection refused due to arbitrary port %s", to.ToStringAddrPort(), to.GetPort());
LogPrintLevel(BCLog::I2P, BCLog::Level::Debug, "Error connecting to %s, connection refused due to arbitrary port %s\n", to.ToStringAddrPort(), to.GetPort());
proxy_error = false;
return false;
}
Expand Down Expand Up @@ -267,7 +271,7 @@ bool Session::Connect(const CService& to, Connection& conn, bool& proxy_error)

throw std::runtime_error(strprintf("\"%s\"", connect_reply.full));
} catch (const std::runtime_error& e) {
Log("Error connecting to %s: %s", to.ToStringAddrPort(), e.what());
LogPrintLevel(BCLog::I2P, BCLog::Level::Debug, "Error connecting to %s: %s\n", to.ToStringAddrPort(), e.what());
CheckControlSock();
return false;
}
Expand All @@ -285,12 +289,6 @@ std::string Session::Reply::Get(const std::string& key) const
return pos->second.value();
}

template <typename... Args>
void Session::Log(const std::string& fmt, const Args&... args) const
{
LogPrint(BCLog::I2P, "%s\n", tfm::format(fmt, args...));
}

Session::Reply Session::SendRequestAndGetReply(const Sock& sock,
const std::string& request,
bool check_result_ok) const
Expand Down Expand Up @@ -346,7 +344,7 @@ void Session::CheckControlSock()

std::string errmsg;
if (m_control_sock && !m_control_sock->IsConnected(errmsg)) {
Log("Control socket error: %s", errmsg);
LogPrintLevel(BCLog::I2P, BCLog::Level::Debug, "Control socket error: %s\n", errmsg);
Disconnect();
}
}
Expand Down Expand Up @@ -416,7 +414,7 @@ void Session::CreateIfNotCreatedAlready()
const auto session_type = m_transient ? "transient" : "persistent";
const auto session_id = GetRandHash().GetHex().substr(0, 10); // full is overkill, too verbose in the logs

Log("Creating %s SAM session %s with %s", session_type, session_id, m_control_host.ToString());
LogPrintLevel(BCLog::I2P, BCLog::Level::Debug, "Creating %s SAM session %s with %s\n", session_type, session_id, m_control_host.ToString());

auto sock = Hello();

Expand Down Expand Up @@ -453,7 +451,7 @@ void Session::CreateIfNotCreatedAlready()
m_session_id = session_id;
m_control_sock = std::move(sock);

Log("%s SAM session %s created, my address=%s",
LogPrintLevel(BCLog::I2P, BCLog::Level::Info, "%s SAM session %s created, my address=%s\n",
Capitalize(session_type),
m_session_id,
m_my_addr.ToStringAddrPort());
Expand Down Expand Up @@ -484,9 +482,9 @@ void Session::Disconnect()
{
if (m_control_sock) {
if (m_session_id.empty()) {
Log("Destroying incomplete SAM session");
LogPrintLevel(BCLog::I2P, BCLog::Level::Info, "Destroying incomplete SAM session\n");
} else {
Log("Destroying SAM session %s", m_session_id);
LogPrintLevel(BCLog::I2P, BCLog::Level::Info, "Destroying SAM session %s\n", m_session_id);
}
m_control_sock.reset();
}
Expand Down
8 changes: 0 additions & 8 deletions src/i2p.h
Original file line number Diff line number Diff line change
Expand Up @@ -156,14 +156,6 @@ class Session
std::string Get(const std::string& key) const;
};

/**
* Log a message in the `BCLog::I2P` category.
* @param[in] fmt printf(3)-like format string.
* @param[in] args printf(3)-like arguments that correspond to `fmt`.
*/
template <typename... Args>
void Log(const std::string& fmt, const Args&... args) const;

/**
* Send request and get a reply from the SAM proxy.
* @param[in] sock A socket that is connected to the SAM proxy.
Expand Down

0 comments on commit b27afb7

Please sign in to comment.