Skip to content

Commit

Permalink
Merge bitcoin/bitcoin#30255: log: use error level for critical log me…
Browse files Browse the repository at this point in the history
…ssages

fae3a1f log: use error level for critical log messages (MarcoFalke)

Pull request description:

  This picks up the first commit from bitcoin/bitcoin#29231, but extends it to also cover cases that were missed in it.

  As per https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#logging, LogError should be used for severe problems that require the node to shut down.

ACKs for top commit:
  stickies-v:
    re-ACK fae3a1f, I'm ~0 on the latest force push as `user_error` was already logged at the right level through `GetNotifications().fatalError(user_error);` so I'd be in favour of deduplicating/cleaning up this logging logic but can be done in follow-up.
  kevkevinpal:
    ACK [fae3a1f](bitcoin/bitcoin@fae3a1f)
  achow101:
    ACK fae3a1f

Tree-SHA512: 3f99fd25d5a204d570a42d8fb2b450439aad7685692f9594cc813d97253c4df172a6ff3cf818959bfcf25dfcf8ee9a9c9ccc6028fcfcecdb47591e18c77ef246
  • Loading branch information
achow101 committed Jun 14, 2024
2 parents 0b94fb8 + fae3a1f commit 538497b
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 14 deletions.
2 changes: 1 addition & 1 deletion src/coins.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ static bool ExecuteBackedWrapper(Func func, const std::vector<std::function<void
for (const auto& f : err_callbacks) {
f();
}
LogPrintf("Error reading from database: %s\n", e.what());
LogError("Error reading from database: %s\n", e.what());
// Starting the shutdown sequence and returning false to the caller would be
// interpreted as 'entry not found' (as opposed to unable to read data), and
// could lead to invalid interpretation. Just exit immediately, as we can't
Expand Down
16 changes: 8 additions & 8 deletions src/init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ static void HandleSIGHUP(int)
static BOOL WINAPI consoleCtrlHandler(DWORD dwCtrlType)
{
if (!(*Assert(g_shutdown))()) {
LogPrintf("Error: failed to send shutdown signal on Ctrl-C\n");
LogError("Failed to send shutdown signal on Ctrl-C\n");
return false;
}
Sleep(INFINITE);
Expand Down Expand Up @@ -840,7 +840,7 @@ std::set<BlockFilterType> g_enabled_filter_types;
// Since LogPrintf may itself allocate memory, set the handler directly
// to terminate first.
std::set_new_handler(std::terminate);
LogPrintf("Error: Out of memory. Terminating.\n");
LogError("Out of memory. Terminating.\n");

// The log was successful, terminate now.
std::terminate();
Expand Down Expand Up @@ -1175,9 +1175,9 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
scheduler.scheduleEvery([&args, &node]{
constexpr uint64_t min_disk_space = 50 << 20; // 50 MB
if (!CheckDiskSpace(args.GetBlocksDirPath(), min_disk_space)) {
LogPrintf("Shutting down due to lack of disk space!\n");
LogError("Shutting down due to lack of disk space!\n");
if (!(*Assert(node.shutdown))()) {
LogPrintf("Error: failed to send shutdown signal after disk space check\n");
LogError("Failed to send shutdown signal after disk space check\n");
}
}
}, std::chrono::minutes{5});
Expand Down Expand Up @@ -1582,7 +1582,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
try {
return f();
} catch (const std::exception& e) {
LogPrintf("%s\n", e.what());
LogError("%s\n", e.what());
return std::make_tuple(node::ChainstateLoadStatus::FAILURE, _("Error opening block database"));
}
};
Expand Down Expand Up @@ -1614,10 +1614,10 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
if (fRet) {
do_reindex = true;
if (!Assert(node.shutdown)->reset()) {
LogPrintf("Internal error: failed to reset shutdown signal.\n");
LogError("Internal error: failed to reset shutdown signal.\n");
}
} else {
LogPrintf("Aborted block database rebuild. Exiting.\n");
LogError("Aborted block database rebuild. Exiting.\n");
return false;
}
} else {
Expand Down Expand Up @@ -1752,7 +1752,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
if (args.GetBoolArg("-stopafterblockimport", DEFAULT_STOPAFTERBLOCKIMPORT)) {
LogPrintf("Stopping after block import\n");
if (!(*Assert(node.shutdown))()) {
LogPrintf("Error: failed to send shutdown signal after finishing block import\n");
LogError("Failed to send shutdown signal after finishing block import\n");
}
return;
}
Expand Down
2 changes: 1 addition & 1 deletion src/node/interfaces.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ class NodeImpl : public Node
void startShutdown() override
{
if (!(*Assert(Assert(m_context)->shutdown))()) {
LogPrintf("Error: failed to send shutdown signal\n");
LogError("Failed to send shutdown signal\n");
}
// Stop RPC for clean shutdown if any of waitfor* commands is executed.
if (args().GetBoolArg("-server", false)) {
Expand Down
2 changes: 1 addition & 1 deletion src/node/kernel_notifications.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ kernel::InterruptResult KernelNotifications::blockTip(SynchronizationState state
uiInterface.NotifyBlockTip(state, &index);
if (m_stop_at_height && index.nHeight >= m_stop_at_height) {
if (!m_shutdown()) {
LogPrintf("Error: failed to send shutdown signal after reaching stop height\n");
LogError("Failed to send shutdown signal after reaching stop height\n");
}
return kernel::Interrupted{};
}
Expand Down
6 changes: 3 additions & 3 deletions src/validation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5967,8 +5967,8 @@ SnapshotCompletionResult ChainstateManager::MaybeCompleteSnapshotValidation()
PACKAGE_NAME, snapshot_tip_height, snapshot_base_height, snapshot_base_height, PACKAGE_BUGREPORT
);

LogPrintf("[snapshot] !!! %s\n", user_error.original);
LogPrintf("[snapshot] deleting snapshot, reverting to validated chain, and stopping node\n");
LogError("[snapshot] !!! %s\n", user_error.original);
LogError("[snapshot] deleting snapshot, reverting to validated chain, and stopping node\n");

m_active_chainstate = m_ibd_chainstate.get();
m_snapshot_chainstate->m_disabled = true;
Expand Down Expand Up @@ -6320,7 +6320,7 @@ bool ChainstateManager::ValidatedSnapshotCleanup()
fs::path p_old,
fs::path p_new,
const fs::filesystem_error& err) {
LogPrintf("Error renaming path (%s) -> (%s): %s\n",
LogError("[snapshot] Error renaming path (%s) -> (%s): %s\n",
fs::PathToString(p_old), fs::PathToString(p_new), err.what());
GetNotifications().fatalError(strprintf(_(
"Rename of '%s' -> '%s' failed. "
Expand Down

0 comments on commit 538497b

Please sign in to comment.