diff --git a/doc/release-notes-27632.md b/doc/release-notes-27632.md new file mode 100644 index 00000000000..d588247c858 --- /dev/null +++ b/doc/release-notes-27632.md @@ -0,0 +1,5 @@ +Updated settings +---------------- + +- Passing an invalid `-debug`, `-debugexclude`, or `-loglevel` logging configuration + option now raises an error, rather than logging an easily missed warning. (#27632) diff --git a/src/init.cpp b/src/init.cpp index 06c8d38036c..410c574235b 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -77,6 +77,7 @@ #include #include #include +#include #include #include #include @@ -951,8 +952,10 @@ bool AppInitParameterInteraction(const ArgsManager& args, bool use_syscall_sandb InitWarning(strprintf(_("Reducing -maxconnections from %d to %d, because of system limitations."), nUserMaxConnections, nMaxConnections)); // ********************************************************* Step 3: parameter-to-internal-flags - init::SetLoggingCategories(args); - init::SetLoggingLevel(args); + auto result = init::SetLoggingCategories(args); + if (!result) return InitError(util::ErrorString(result)); + result = init::SetLoggingLevel(args); + if (!result) return InitError(util::ErrorString(result)); nConnectTimeout = args.GetIntArg("-timeout", DEFAULT_CONNECT_TIMEOUT); if (nConnectTimeout <= 0) { diff --git a/src/init/common.cpp b/src/init/common.cpp index 9a52a09cea0..f3f7c696c54 100644 --- a/src/init/common.cpp +++ b/src/init/common.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -58,27 +59,28 @@ void SetLoggingOptions(const ArgsManager& args) fLogIPs = args.GetBoolArg("-logips", DEFAULT_LOGIPS); } -void SetLoggingLevel(const ArgsManager& args) +util::Result SetLoggingLevel(const ArgsManager& args) { if (args.IsArgSet("-loglevel")) { for (const std::string& level_str : args.GetArgs("-loglevel")) { if (level_str.find_first_of(':', 3) == std::string::npos) { // user passed a global log level, i.e. -loglevel= if (!LogInstance().SetLogLevel(level_str)) { - InitWarning(strprintf(_("Unsupported global logging level -loglevel=%s. Valid values: %s."), level_str, LogInstance().LogLevelsString())); + return util::Error{strprintf(_("Unsupported global logging level %s=%s. Valid values: %s."), "-loglevel", level_str, LogInstance().LogLevelsString())}; } } else { // user passed a category-specific log level, i.e. -loglevel=: const auto& toks = SplitString(level_str, ':'); if (!(toks.size() == 2 && LogInstance().SetCategoryLogLevel(toks[0], toks[1]))) { - InitWarning(strprintf(_("Unsupported category-specific logging level -loglevel=%s. Expected -loglevel=:. Valid categories: %s. Valid loglevels: %s."), level_str, LogInstance().LogCategoriesString(), LogInstance().LogLevelsString())); + return util::Error{strprintf(_("Unsupported category-specific logging level %1$s=%2$s. Expected %1$s=:. Valid categories: %3$s. Valid loglevels: %4$s."), "-loglevel", level_str, LogInstance().LogCategoriesString(), LogInstance().LogLevelsString())}; } } } } + return {}; } -void SetLoggingCategories(const ArgsManager& args) +util::Result SetLoggingCategories(const ArgsManager& args) { if (args.IsArgSet("-debug")) { // Special-case: if -debug=0/-nodebug is set, turn off debugging messages @@ -88,7 +90,7 @@ void SetLoggingCategories(const ArgsManager& args) [](std::string cat){return cat == "0" || cat == "none";})) { for (const auto& cat : categories) { if (!LogInstance().EnableCategory(cat)) { - InitWarning(strprintf(_("Unsupported logging category %s=%s."), "-debug", cat)); + return util::Error{strprintf(_("Unsupported logging category %s=%s."), "-debug", cat)}; } } } @@ -97,9 +99,10 @@ void SetLoggingCategories(const ArgsManager& args) // Now remove the logging categories which were explicitly excluded for (const std::string& cat : args.GetArgs("-debugexclude")) { if (!LogInstance().DisableCategory(cat)) { - InitWarning(strprintf(_("Unsupported logging category %s=%s."), "-debugexclude", cat)); + return util::Error{strprintf(_("Unsupported logging category %s=%s."), "-debugexclude", cat)}; } } + return {}; } bool StartLogging(const ArgsManager& args) diff --git a/src/init/common.h b/src/init/common.h index 44c3a502eea..b61a77c6d46 100644 --- a/src/init/common.h +++ b/src/init/common.h @@ -8,13 +8,15 @@ #ifndef BITCOIN_INIT_COMMON_H #define BITCOIN_INIT_COMMON_H +#include + class ArgsManager; namespace init { void AddLoggingArgs(ArgsManager& args); void SetLoggingOptions(const ArgsManager& args); -void SetLoggingCategories(const ArgsManager& args); -void SetLoggingLevel(const ArgsManager& args); +[[nodiscard]] util::Result SetLoggingCategories(const ArgsManager& args); +[[nodiscard]] util::Result SetLoggingLevel(const ArgsManager& args); bool StartLogging(const ArgsManager& args); void LogPackageVersion(); } // namespace init diff --git a/src/logging.cpp b/src/logging.cpp index 7725fefeb7d..a5cfb0d28e3 100644 --- a/src/logging.cpp +++ b/src/logging.cpp @@ -179,7 +179,7 @@ const CLogCategoryDesc LogCategories[] = {BCLog::LOCK, "lock"}, #endif {BCLog::UTIL, "util"}, - {BCLog::BLOCKSTORE, "blockstorage"}, + {BCLog::BLOCKSTORAGE, "blockstorage"}, {BCLog::TXRECONCILIATION, "txreconciliation"}, {BCLog::SCAN, "scan"}, {BCLog::ALL, "1"}, @@ -280,7 +280,7 @@ std::string LogCategoryToStr(BCLog::LogFlags category) #endif case BCLog::LogFlags::UTIL: return "util"; - case BCLog::LogFlags::BLOCKSTORE: + case BCLog::LogFlags::BLOCKSTORAGE: return "blockstorage"; case BCLog::LogFlags::TXRECONCILIATION: return "txreconciliation"; diff --git a/src/logging.h b/src/logging.h index e7c554e79f8..fc03c8eac38 100644 --- a/src/logging.h +++ b/src/logging.h @@ -65,7 +65,7 @@ namespace BCLog { LOCK = (1 << 24), #endif UTIL = (1 << 25), - BLOCKSTORE = (1 << 26), + BLOCKSTORAGE = (1 << 26), TXRECONCILIATION = (1 << 27), SCAN = (1 << 28), ALL = ~(uint32_t)0, diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 1368ae6f6d5..87cd291c7e5 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -573,7 +573,7 @@ void BlockManager::UnlinkPrunedFiles(const std::set& setFilesToPrune) const const bool removed_blockfile{fs::remove(BlockFileSeq().FileName(pos), ec)}; const bool removed_undofile{fs::remove(UndoFileSeq().FileName(pos), ec)}; if (removed_blockfile || removed_undofile) { - LogPrint(BCLog::BLOCKSTORE, "Prune: %s deleted blk/rev (%05u)\n", __func__, *it); + LogPrint(BCLog::BLOCKSTORAGE, "Prune: %s deleted blk/rev (%05u)\n", __func__, *it); } } } @@ -642,7 +642,7 @@ bool BlockManager::FindBlockPos(FlatFilePos& pos, unsigned int nAddSize, unsigne if ((int)nFile != m_last_blockfile) { if (!fKnown) { - LogPrint(BCLog::BLOCKSTORE, "Leaving block file %i: %s\n", m_last_blockfile, m_blockfile_info[m_last_blockfile].ToString()); + LogPrint(BCLog::BLOCKSTORAGE, "Leaving block file %i: %s\n", m_last_blockfile, m_blockfile_info[m_last_blockfile].ToString()); } FlushBlockFile(!fKnown, finalize_undo); m_last_blockfile = nFile; diff --git a/src/test/logging_tests.cpp b/src/test/logging_tests.cpp index beb9398c74d..2699d316da8 100644 --- a/src/test/logging_tests.cpp +++ b/src/test/logging_tests.cpp @@ -200,7 +200,9 @@ BOOST_FIXTURE_TEST_CASE(logging_Conf, LogSetup) const char* argv_test[] = {"bitcoind", "-loglevel=debug"}; std::string err; BOOST_REQUIRE(args.ParseParameters(2, argv_test, err)); - init::SetLoggingLevel(args); + + auto result = init::SetLoggingLevel(args); + BOOST_REQUIRE(result); BOOST_CHECK_EQUAL(LogInstance().LogLevel(), BCLog::Level::Debug); } @@ -212,7 +214,9 @@ BOOST_FIXTURE_TEST_CASE(logging_Conf, LogSetup) const char* argv_test[] = {"bitcoind", "-loglevel=net:trace"}; std::string err; BOOST_REQUIRE(args.ParseParameters(2, argv_test, err)); - init::SetLoggingLevel(args); + + auto result = init::SetLoggingLevel(args); + BOOST_REQUIRE(result); BOOST_CHECK_EQUAL(LogInstance().LogLevel(), BCLog::DEFAULT_LOG_LEVEL); const auto& category_levels{LogInstance().CategoryLevels()}; @@ -229,7 +233,9 @@ BOOST_FIXTURE_TEST_CASE(logging_Conf, LogSetup) const char* argv_test[] = {"bitcoind", "-loglevel=debug", "-loglevel=net:trace", "-loglevel=http:info"}; std::string err; BOOST_REQUIRE(args.ParseParameters(4, argv_test, err)); - init::SetLoggingLevel(args); + + auto result = init::SetLoggingLevel(args); + BOOST_REQUIRE(result); BOOST_CHECK_EQUAL(LogInstance().LogLevel(), BCLog::Level::Debug); const auto& category_levels{LogInstance().CategoryLevels()}; diff --git a/test/functional/feature_logging.py b/test/functional/feature_logging.py index fe4f02dfe62..b0788e2a2d1 100755 --- a/test/functional/feature_logging.py +++ b/test/functional/feature_logging.py @@ -69,6 +69,36 @@ class LoggingTest(BitcoinTestFramework): # just sanity check no crash here self.restart_node(0, [f"-debuglogfile={os.devnull}"]) + self.log.info("Test -debug and -debugexclude raise when invalid values are passed") + self.stop_node(0) + self.nodes[0].assert_start_raises_init_error( + extra_args=["-debug=abc"], + expected_msg="Error: Unsupported logging category -debug=abc.", + match=ErrorMatch.FULL_REGEX, + ) + self.nodes[0].assert_start_raises_init_error( + extra_args=["-debugexclude=abc"], + expected_msg="Error: Unsupported logging category -debugexclude=abc.", + match=ErrorMatch.FULL_REGEX, + ) + + self.log.info("Test -loglevel raises when invalid values are passed") + self.nodes[0].assert_start_raises_init_error( + extra_args=["-loglevel=abc"], + expected_msg="Error: Unsupported global logging level -loglevel=abc. Valid values: info, debug, trace.", + match=ErrorMatch.FULL_REGEX, + ) + self.nodes[0].assert_start_raises_init_error( + extra_args=["-loglevel=net:abc"], + expected_msg="Error: Unsupported category-specific logging level -loglevel=net:abc.", + match=ErrorMatch.PARTIAL_REGEX, + ) + self.nodes[0].assert_start_raises_init_error( + extra_args=["-loglevel=net:info:abc"], + expected_msg="Error: Unsupported category-specific logging level -loglevel=net:info:abc.", + match=ErrorMatch.PARTIAL_REGEX, + ) + if __name__ == '__main__': LoggingTest().main()