Skip to content

Commit c122318

Browse files
committed
Merge bitcoin/bitcoin#29672: validation: Make translations of fatal errors consistent
824f472 node: Use log levels in noui_ThreadSafeMessageBox (TheCharlatan) ddc7872 node: Make translations of fatal errors consistent (TheCharlatan) Pull request description: 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. ACKs for top commit: stickies-v: re-ACK 824f472 maflcko: ACK 824f472 🔎 achow101: ACK 824f472 hebasto: re-ACK 824f472. Tree-SHA512: 2868ee7b045fe7f3ac582ce5039141b398480b7627734976201dafaaef7544b8461635a7292fee4a7f32ff1bfc26f9bd4d0c292dca424ba42fb7fc4483d7ce8d
2 parents 2795e89 + 824f472 commit c122318

15 files changed

+56
-61
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
@@ -1757,7 +1757,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
17571757
// Start indexes initial sync
17581758
if (!StartIndexBackgroundSync(node)) {
17591759
bilingual_str err_str = _("Failed to start indexes, shutting down..");
1760-
chainman.GetNotifications().fatalError(err_str.original, err_str);
1760+
chainman.GetNotifications().fatalError(err_str);
17611761
return;
17621762
}
17631763
// 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: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,13 @@
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+
InitError(_("A fatal internal error occurred, see debug.log for details: ") + message);
2423
exit_status.store(EXIT_FAILURE);
2524
if (shutdown && !(*shutdown)()) {
26-
LogPrintf("Error: failed to send shutdown signal\n");
25+
LogError("Failed to send shutdown signal\n");
2726
};
2827
}
2928
} // 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/noui.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,20 +28,21 @@ bool noui_ThreadSafeMessageBox(const bilingual_str& message, const std::string&
2828
switch (style) {
2929
case CClientUIInterface::MSG_ERROR:
3030
strCaption = "Error: ";
31+
if (!fSecure) LogError("%s\n", message.original);
3132
break;
3233
case CClientUIInterface::MSG_WARNING:
3334
strCaption = "Warning: ";
35+
if (!fSecure) LogWarning("%s\n", message.original);
3436
break;
3537
case CClientUIInterface::MSG_INFORMATION:
3638
strCaption = "Information: ";
39+
if (!fSecure) LogInfo("%s\n", message.original);
3740
break;
3841
default:
3942
strCaption = caption + ": "; // Use supplied caption (can be empty)
43+
if (!fSecure) LogInfo("%s%s\n", strCaption, message.original);
4044
}
4145

42-
if (!fSecure) {
43-
LogPrintf("%s%s\n", strCaption, message.original);
44-
}
4546
tfm::format(std::cerr, "%s%s\n", strCaption, message.original);
4647
return false;
4748
}

0 commit comments

Comments
 (0)