Skip to content

Commit 918efe6

Browse files
seditedsedited
committed
Make translations of fatal errors consistent
The extra `bilingual_str` argument of the fatal error notifications and `node::AbortNode()` is often unused and when used usually contains the same string as the message argument. It also seems to be confusing, since it is not consistently used for errors requiring user action. For example some assumeutxo fatal errors require the user to do something, but are not translated. So simplify the fatal error and abort node interfaces by only passing a translated string. This slightly changes the fatal errors displayed to the user.
1 parent a85e5a7 commit 918efe6

15 files changed

+54
-59
lines changed

‎src/bitcoin-chainstate.cpp‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -89,14 +89,13 @@ int main(int argc, char* argv[])
8989
{
9090
std::cout << "Warning: " << warning.original << std::endl;
9191
}
92-
void flushError(const std::string& debug_message) override
92+
void flushError(const bilingual_str& message) override
9393
{
94-
std::cerr << "Error flushing block data to disk: " << debug_message << std::endl;
94+
std::cerr << "Error flushing block data to disk: " << message.original << std::endl;
9595
}
96-
void fatalError(const std::string& debug_message, const bilingual_str& user_message) override
96+
void fatalError(const bilingual_str& message) override
9797
{
98-
std::cerr << "Error: " << debug_message << std::endl;
99-
std::cerr << (user_message.empty() ? "A fatal internal error occurred." : user_message.original) << std::endl;
98+
std::cerr << "Error: " << message.original << std::endl;
10099
}
101100
};
102101
auto notifications = std::make_unique<KernelNotifications>();

‎src/index/base.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ template <typename... Args>
3131
void BaseIndex::FatalErrorf(const char* fmt, const Args&... args)
3232
{
3333
auto message = tfm::format(fmt, args...);
34-
node::AbortNode(m_chain->context()->shutdown, m_chain->context()->exit_status, message);
34+
node::AbortNode(m_chain->context()->shutdown, m_chain->context()->exit_status, Untranslated(message));
3535
}
3636

3737
CBlockLocator GetLocator(interfaces::Chain& chain, const uint256& block_hash)

‎src/init.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1766,7 +1766,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
17661766
// Start indexes initial sync
17671767
if (!StartIndexBackgroundSync(node)) {
17681768
bilingual_str err_str = _("Failed to start indexes, shutting down..");
1769-
chainman.GetNotifications().fatalError(err_str.original, err_str);
1769+
chainman.GetNotifications().fatalError(err_str);
17701770
return;
17711771
}
17721772
// Load mempool from disk

‎src/kernel/notifications_interface.h‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,12 @@
55
#ifndef BITCOIN_KERNEL_NOTIFICATIONS_INTERFACE_H
66
#define BITCOIN_KERNEL_NOTIFICATIONS_INTERFACE_H
77

8-
#include <util/translation.h>
9-
108
#include <cstdint>
11-
#include <string>
129
#include <variant>
1310

1411
class CBlockIndex;
1512
enum class SynchronizationState;
13+
struct bilingual_str;
1614

1715
namespace kernel {
1816

@@ -48,7 +46,7 @@ class Notifications
4846
//! perform. Applications can choose to handle the flush error notification
4947
//! by logging the error, or notifying the user, or triggering an early
5048
//! shutdown as a precaution against causing more errors.
51-
virtual void flushError(const std::string& debug_message) {}
49+
virtual void flushError(const bilingual_str& message) {}
5250

5351
//! The fatal error notification is sent to notify the user when an error
5452
//! occurs in kernel code that can't be recovered from. After this
@@ -57,7 +55,7 @@ class Notifications
5755
//! handle the fatal error notification by logging the error, or notifying
5856
//! the user, or triggering an early shutdown as a precaution against
5957
//! causing more errors.
60-
virtual void fatalError(const std::string& debug_message, const bilingual_str& user_message = {}) {}
58+
virtual void fatalError(const bilingual_str& message) {}
6159
};
6260
} // namespace kernel
6361

‎src/node/abort.cpp‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,14 @@
1616

1717
namespace node {
1818

19-
void AbortNode(util::SignalInterrupt* shutdown, std::atomic<int>& exit_status, const std::string& debug_message, const bilingual_str& user_message)
19+
void AbortNode(util::SignalInterrupt* shutdown, std::atomic<int>& exit_status, const bilingual_str& message)
2020
{
21-
SetMiscWarning(Untranslated(debug_message));
22-
LogPrintf("*** %s\n", debug_message);
23-
InitError(user_message.empty() ? _("A fatal internal error occurred, see debug.log for details") : user_message);
21+
SetMiscWarning(message);
22+
LogError("%s\n", message.original);
23+
InitError(_("A fatal internal error occurred, see debug.log for details: ") + message);
2424
exit_status.store(EXIT_FAILURE);
2525
if (shutdown && !(*shutdown)()) {
26-
LogPrintf("Error: failed to send shutdown signal\n");
26+
LogError("Failed to send shutdown signal\n");
2727
};
2828
}
2929
} // namespace node

‎src/node/abort.h‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,17 +5,16 @@
55
#ifndef BITCOIN_NODE_ABORT_H
66
#define BITCOIN_NODE_ABORT_H
77

8-
#include <util/translation.h>
9-
108
#include <atomic>
11-
#include <string>
9+
10+
struct bilingual_str;
1211

1312
namespace util {
1413
class SignalInterrupt;
1514
} // namespace util
1615

1716
namespace node {
18-
void AbortNode(util::SignalInterrupt* shutdown, std::atomic<int>& exit_status, const std::string& debug_message, const bilingual_str& user_message = {});
17+
void AbortNode(util::SignalInterrupt* shutdown, std::atomic<int>& exit_status, const bilingual_str& message);
1918
} // namespace node
2019

2120
#endif // BITCOIN_NODE_ABORT_H

‎src/node/blockstorage.cpp‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -404,7 +404,7 @@ bool BlockManager::LoadBlockIndex(const std::optional<uint256>& snapshot_blockha
404404
if (snapshot_blockhash) {
405405
const std::optional<AssumeutxoData> maybe_au_data = GetParams().AssumeutxoForBlockhash(*snapshot_blockhash);
406406
if (!maybe_au_data) {
407-
m_opts.notifications.fatalError(strprintf("Assumeutxo data not found for the given blockhash '%s'.", snapshot_blockhash->ToString()));
407+
m_opts.notifications.fatalError(strprintf(_("Assumeutxo data not found for the given blockhash '%s'."), snapshot_blockhash->ToString()));
408408
return false;
409409
}
410410
const AssumeutxoData& au_data = *Assert(maybe_au_data);
@@ -741,7 +741,7 @@ bool BlockManager::FlushUndoFile(int block_file, bool finalize)
741741
{
742742
FlatFilePos undo_pos_old(block_file, m_blockfile_info[block_file].nUndoSize);
743743
if (!UndoFileSeq().Flush(undo_pos_old, finalize)) {
744-
m_opts.notifications.flushError("Flushing undo file to disk failed. This is likely the result of an I/O error.");
744+
m_opts.notifications.flushError(_("Flushing undo file to disk failed. This is likely the result of an I/O error."));
745745
return false;
746746
}
747747
return true;
@@ -763,7 +763,7 @@ bool BlockManager::FlushBlockFile(int blockfile_num, bool fFinalize, bool finali
763763

764764
FlatFilePos block_pos_old(blockfile_num, m_blockfile_info[blockfile_num].nSize);
765765
if (!BlockFileSeq().Flush(block_pos_old, fFinalize)) {
766-
m_opts.notifications.flushError("Flushing block file to disk failed. This is likely the result of an I/O error.");
766+
m_opts.notifications.flushError(_("Flushing block file to disk failed. This is likely the result of an I/O error."));
767767
success = false;
768768
}
769769
// we do not always flush the undo file, as the chain tip may be lagging behind the incoming blocks,
@@ -935,7 +935,7 @@ bool BlockManager::FindBlockPos(FlatFilePos& pos, unsigned int nAddSize, unsigne
935935
bool out_of_space;
936936
size_t bytes_allocated = BlockFileSeq().Allocate(pos, nAddSize, out_of_space);
937937
if (out_of_space) {
938-
m_opts.notifications.fatalError("Disk space is too low!", _("Disk space is too low!"));
938+
m_opts.notifications.fatalError(_("Disk space is too low!"));
939939
return false;
940940
}
941941
if (bytes_allocated != 0 && IsPruneMode()) {
@@ -960,7 +960,7 @@ bool BlockManager::FindUndoPos(BlockValidationState& state, int nFile, FlatFileP
960960
bool out_of_space;
961961
size_t bytes_allocated = UndoFileSeq().Allocate(pos, nAddSize, out_of_space);
962962
if (out_of_space) {
963-
return FatalError(m_opts.notifications, state, "Disk space is too low!", _("Disk space is too low!"));
963+
return FatalError(m_opts.notifications, state, _("Disk space is too low!"));
964964
}
965965
if (bytes_allocated != 0 && IsPruneMode()) {
966966
m_check_for_pruning = true;
@@ -1008,7 +1008,7 @@ bool BlockManager::WriteUndoDataForBlock(const CBlockUndo& blockundo, BlockValid
10081008
return false;
10091009
}
10101010
if (!UndoWriteToDisk(blockundo, _pos, block.pprev->GetBlockHash())) {
1011-
return FatalError(m_opts.notifications, state, "Failed to write undo data");
1011+
return FatalError(m_opts.notifications, state, _("Failed to write undo data."));
10121012
}
10131013
// rev files are written in block height order, whereas blk files are written as blocks come in (often out of order)
10141014
// we want to flush the rev (undo) file once we've written the last block, which is indicated by the last height
@@ -1149,7 +1149,7 @@ FlatFilePos BlockManager::SaveBlockToDisk(const CBlock& block, int nHeight, cons
11491149
}
11501150
if (!position_known) {
11511151
if (!WriteBlockToDisk(block, blockPos)) {
1152-
m_opts.notifications.fatalError("Failed to write block");
1152+
m_opts.notifications.fatalError(_("Failed to write block."));
11531153
return FlatFilePos();
11541154
}
11551155
}
@@ -1233,7 +1233,7 @@ void ImportBlocks(ChainstateManager& chainman, std::vector<fs::path> vImportFile
12331233
for (Chainstate* chainstate : WITH_LOCK(::cs_main, return chainman.GetAll())) {
12341234
BlockValidationState state;
12351235
if (!chainstate->ActivateBestChain(state, nullptr)) {
1236-
chainman.GetNotifications().fatalError(strprintf("Failed to connect best block (%s)", state.ToString()));
1236+
chainman.GetNotifications().fatalError(strprintf(_("Failed to connect best block (%s)."), state.ToString()));
12371237
return;
12381238
}
12391239
}

‎src/node/kernel_notifications.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -84,15 +84,15 @@ void KernelNotifications::warning(const bilingual_str& warning)
8484
DoWarning(warning);
8585
}
8686

87-
void KernelNotifications::flushError(const std::string& debug_message)
87+
void KernelNotifications::flushError(const bilingual_str& message)
8888
{
89-
AbortNode(&m_shutdown, m_exit_status, debug_message);
89+
AbortNode(&m_shutdown, m_exit_status, message);
9090
}
9191

92-
void KernelNotifications::fatalError(const std::string& debug_message, const bilingual_str& user_message)
92+
void KernelNotifications::fatalError(const bilingual_str& message)
9393
{
9494
node::AbortNode(m_shutdown_on_fatal_error ? &m_shutdown : nullptr,
95-
m_exit_status, debug_message, user_message);
95+
m_exit_status, message);
9696
}
9797

9898
void ReadNotificationArgs(const ArgsManager& args, KernelNotifications& notifications)

‎src/node/kernel_notifications.h‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99

1010
#include <atomic>
1111
#include <cstdint>
12-
#include <string>
1312

1413
class ArgsManager;
1514
class CBlockIndex;
@@ -37,9 +36,9 @@ class KernelNotifications : public kernel::Notifications
3736

3837
void warning(const bilingual_str& warning) override;
3938

40-
void flushError(const std::string& debug_message) override;
39+
void flushError(const bilingual_str& message) override;
4140

42-
void fatalError(const std::string& debug_message, const bilingual_str& user_message = {}) override;
41+
void fatalError(const bilingual_str& message) override;
4342

4443
//! Block height after which blockTip notification will return Interrupted{}, if >0.
4544
int m_stop_at_height{DEFAULT_STOPATHEIGHT};

‎src/validation.cpp‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2024,10 +2024,10 @@ bool CheckInputScripts(const CTransaction& tx, TxValidationState& state,
20242024
return true;
20252025
}
20262026

2027-
bool FatalError(Notifications& notifications, BlockValidationState& state, const std::string& strMessage, const bilingual_str& userMessage)
2027+
bool FatalError(Notifications& notifications, BlockValidationState& state, const bilingual_str& message)
20282028
{
2029-
notifications.fatalError(strMessage, userMessage);
2030-
return state.Error(strMessage);
2029+
notifications.fatalError(message);
2030+
return state.Error(message.original);
20312031
}
20322032

20332033
/**
@@ -2249,7 +2249,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
22492249
// We don't write down blocks to disk if they may have been
22502250
// corrupted, so this should be impossible unless we're having hardware
22512251
// problems.
2252-
return FatalError(m_chainman.GetNotifications(), state, "Corrupt block found indicating potential hardware failure; shutting down");
2252+
return FatalError(m_chainman.GetNotifications(), state, _("Corrupt block found indicating potential hardware failure."));
22532253
}
22542254
LogError("%s: Consensus::CheckBlock: %s\n", __func__, state.ToString());
22552255
return false;
@@ -2675,7 +2675,7 @@ bool Chainstate::FlushStateToDisk(
26752675
if (fDoFullFlush || fPeriodicWrite) {
26762676
// Ensure we can write block index
26772677
if (!CheckDiskSpace(m_blockman.m_opts.blocks_dir)) {
2678-
return FatalError(m_chainman.GetNotifications(), state, "Disk space is too low!", _("Disk space is too low!"));
2678+
return FatalError(m_chainman.GetNotifications(), state, _("Disk space is too low!"));
26792679
}
26802680
{
26812681
LOG_TIME_MILLIS_WITH_CATEGORY("write block and undo data to disk", BCLog::BENCH);
@@ -2693,7 +2693,7 @@ bool Chainstate::FlushStateToDisk(
26932693
LOG_TIME_MILLIS_WITH_CATEGORY("write block index to disk", BCLog::BENCH);
26942694

26952695
if (!m_blockman.WriteBlockIndexDB()) {
2696-
return FatalError(m_chainman.GetNotifications(), state, "Failed to write to block index database");
2696+
return FatalError(m_chainman.GetNotifications(), state, _("Failed to write to block index database."));
26972697
}
26982698
}
26992699
// Finally remove any pruned files
@@ -2715,11 +2715,11 @@ bool Chainstate::FlushStateToDisk(
27152715
// an overestimation, as most will delete an existing entry or
27162716
// overwrite one. Still, use a conservative safety factor of 2.
27172717
if (!CheckDiskSpace(m_chainman.m_options.datadir, 48 * 2 * 2 * CoinsTip().GetCacheSize())) {
2718-
return FatalError(m_chainman.GetNotifications(), state, "Disk space is too low!", _("Disk space is too low!"));
2718+
return FatalError(m_chainman.GetNotifications(), state, _("Disk space is too low!"));
27192719
}
27202720
// Flush the chainstate (which may refer to block index entries).
27212721
if (!CoinsTip().Flush())
2722-
return FatalError(m_chainman.GetNotifications(), state, "Failed to write to coin database");
2722+
return FatalError(m_chainman.GetNotifications(), state, _("Failed to write to coin database."));
27232723
m_last_flush = nNow;
27242724
full_flush_completed = true;
27252725
TRACE5(utxocache, flush,
@@ -2735,7 +2735,7 @@ bool Chainstate::FlushStateToDisk(
27352735
m_chainman.m_options.signals->ChainStateFlushed(this->GetRole(), m_chain.GetLocator());
27362736
}
27372737
} catch (const std::runtime_error& e) {
2738-
return FatalError(m_chainman.GetNotifications(), state, std::string("System error while flushing: ") + e.what());
2738+
return FatalError(m_chainman.GetNotifications(), state, strprintf(_("System error while flushing: %s"), e.what()));
27392739
}
27402740
return true;
27412741
}
@@ -2971,7 +2971,7 @@ bool Chainstate::ConnectTip(BlockValidationState& state, CBlockIndex* pindexNew,
29712971
if (!pblock) {
29722972
std::shared_ptr<CBlock> pblockNew = std::make_shared<CBlock>();
29732973
if (!m_blockman.ReadBlockFromDisk(*pblockNew, *pindexNew)) {
2974-
return FatalError(m_chainman.GetNotifications(), state, "Failed to read block");
2974+
return FatalError(m_chainman.GetNotifications(), state, _("Failed to read block."));
29752975
}
29762976
pthisBlock = pblockNew;
29772977
} else {
@@ -3158,7 +3158,7 @@ bool Chainstate::ActivateBestChainStep(BlockValidationState& state, CBlockIndex*
31583158
// If we're unable to disconnect a block during normal operation,
31593159
// then that is a failure of our local system -- we should abort
31603160
// rather than stay on a less work chain.
3161-
FatalError(m_chainman.GetNotifications(), state, "Failed to disconnect block; see debug.log for details");
3161+
FatalError(m_chainman.GetNotifications(), state, _("Failed to disconnect block."));
31623162
return false;
31633163
}
31643164
fBlocksDisconnected = true;
@@ -4299,7 +4299,7 @@ bool ChainstateManager::AcceptBlock(const std::shared_ptr<const CBlock>& pblock,
42994299
}
43004300
ReceivedBlockTransactions(block, pindex, blockPos);
43014301
} catch (const std::runtime_error& e) {
4302-
return FatalError(GetNotifications(), state, std::string("System error: ") + e.what());
4302+
return FatalError(GetNotifications(), state, strprintf(_("System error while saving block to disk: %s"), e.what()));
43034303
}
43044304

43054305
// TODO: FlushStateToDisk() handles flushing of both block and chainstate
@@ -4983,7 +4983,7 @@ void ChainstateManager::LoadExternalBlockFile(
49834983
}
49844984
}
49854985
} catch (const std::runtime_error& e) {
4986-
GetNotifications().fatalError(std::string("System error: ") + e.what());
4986+
GetNotifications().fatalError(strprintf(_("System error while loading external block file: %s"), e.what()));
49874987
}
49884988
LogPrintf("Loaded %i blocks from external file in %dms\n", nLoaded, Ticks<std::chrono::milliseconds>(SteadyClock::now() - start));
49894989
}
@@ -5450,8 +5450,8 @@ bool ChainstateManager::ActivateSnapshot(
54505450
snapshot_chainstate.reset();
54515451
bool removed = DeleteCoinsDBFromDisk(*snapshot_datadir, /*is_snapshot=*/true);
54525452
if (!removed) {
5453-
GetNotifications().fatalError(strprintf("Failed to remove snapshot chainstate dir (%s). "
5454-
"Manually remove it before restarting.\n", fs::PathToString(*snapshot_datadir)));
5453+
GetNotifications().fatalError(strprintf(_("Failed to remove snapshot chainstate dir (%s). "
5454+
"Manually remove it before restarting.\n"), fs::PathToString(*snapshot_datadir)));
54555455
}
54565456
}
54575457
return false;
@@ -5805,7 +5805,7 @@ SnapshotCompletionResult ChainstateManager::MaybeCompleteSnapshotValidation()
58055805
user_error = strprintf(Untranslated("%s\n%s"), user_error, util::ErrorString(rename_result));
58065806
}
58075807

5808-
GetNotifications().fatalError(user_error.original, user_error);
5808+
GetNotifications().fatalError(user_error);
58095809
};
58105810

58115811
if (index_new.GetBlockHash() != snapshot_blockhash) {
@@ -6146,9 +6146,9 @@ bool ChainstateManager::ValidatedSnapshotCleanup()
61466146
const fs::filesystem_error& err) {
61476147
LogPrintf("Error renaming path (%s) -> (%s): %s\n",
61486148
fs::PathToString(p_old), fs::PathToString(p_new), err.what());
6149-
GetNotifications().fatalError(strprintf(
6149+
GetNotifications().fatalError(strprintf(_(
61506150
"Rename of '%s' -> '%s' failed. "
6151-
"Cannot clean up the background chainstate leveldb directory.",
6151+
"Cannot clean up the background chainstate leveldb directory."),
61526152
fs::PathToString(p_old), fs::PathToString(p_new)));
61536153
};
61546154

0 commit comments

Comments
 (0)