From 3153f22b8dcaba18c952ca66bef34b138056aa1f Mon Sep 17 00:00:00 2001 From: gabime Date: Sat, 4 Jan 2025 17:09:19 +0200 Subject: [PATCH] Error handler --- CMakeLists.txt | 4 +- include/spdlog/details/default_err_handler.h | 19 +++++++ include/spdlog/details/error_handler.h | 33 ------------ include/spdlog/logger.h | 27 ++++++---- include/spdlog/sinks/async_sink.h | 4 +- src/details/default_err_handler.cpp | 27 ++++++++++ src/details/error_handler.cpp | 55 -------------------- src/logger.cpp | 20 ++++--- src/sinks/async_sink.cpp | 8 +-- tests/test_errors.cpp | 35 ++++++++----- 10 files changed, 105 insertions(+), 127 deletions(-) create mode 100644 include/spdlog/details/default_err_handler.h delete mode 100644 include/spdlog/details/error_handler.h create mode 100644 src/details/default_err_handler.cpp delete mode 100644 src/details/error_handler.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 40541047..59076775 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -156,7 +156,7 @@ set(SPDLOG_HEADERS "include/spdlog/details/mpmc_blocking_q.h" "include/spdlog/details/null_mutex.h" "include/spdlog/details/os.h" - "include/spdlog/details/error_handler.h" + "include/spdlog/details/default_err_handler.h" "include/spdlog/bin_to_hex.h" "include/spdlog/sinks/android_sink.h" "include/spdlog/sinks/base_sink.h" @@ -192,7 +192,7 @@ set(SPDLOG_SRCS "src/details/os_filesystem.cpp" "src/details/log_msg.cpp" "src/details/async_log_msg.cpp" - "src/details/error_handler.cpp" + "src/details/default_err_handler.cpp" "src/sinks/base_sink.cpp" "src/sinks/basic_file_sink.cpp" "src/sinks/rotating_file_sink.cpp" diff --git a/include/spdlog/details/default_err_handler.h b/include/spdlog/details/default_err_handler.h new file mode 100644 index 00000000..950203a2 --- /dev/null +++ b/include/spdlog/details/default_err_handler.h @@ -0,0 +1,19 @@ +// Copyright(c) 2015-present, Gabi Melman & spdlog contributors. +// Distributed under the MIT License (http://opensource.org/licenses/MIT) + +#pragma once + +#include +#include "spdlog/common.h" + +// by default, prints the error to stderr, thread safe +namespace spdlog { +namespace details { +class default_err_handler { + mutable std::mutex mutex_; +public: + void handle(const std::string& origin, const source_loc& loc, const std::string &err_msg) const; +}; + + +}} // namespace spdlog::details diff --git a/include/spdlog/details/error_handler.h b/include/spdlog/details/error_handler.h deleted file mode 100644 index 15db0f9e..00000000 --- a/include/spdlog/details/error_handler.h +++ /dev/null @@ -1,33 +0,0 @@ -// Copyright(c) 2015-present, Gabi Melman & spdlog contributors. -// Distributed under the MIT License (http://opensource.org/licenses/MIT) - -#pragma once - -#include -#include -#include "spdlog/common.h" - -// by default, prints the error to stderr. thread safe. -namespace spdlog { -namespace details { -class error_handler { -public: - explicit error_handler(std::string name); - error_handler(const error_handler &); - error_handler(error_handler &&) noexcept; - // for simplicity allow only construction of this class. - // otherwise we need to deal with mutexes and potential deadlocks. - error_handler &operator=(const error_handler &) = delete; - error_handler &operator=(error_handler &&) = delete; - void handle(const source_loc& loc, const std::string &err_msg) const; - void set_name(const std::string& name); - void set_custom_handler(err_handler handler); - -private: - mutable std::mutex mutex_; - std::string name_; - err_handler custom_handler_ = nullptr; -}; - - -}} // namespace spdlog::details diff --git a/include/spdlog/logger.h b/include/spdlog/logger.h index 82162284..babf2a71 100644 --- a/include/spdlog/logger.h +++ b/include/spdlog/logger.h @@ -3,7 +3,11 @@ #pragma once -// Thread safe logger (except for set_error_handler()) +// Thread safe logger, Except for the following methods which are not thread-safe: +// set_pattern() +// set_formatter() +// set_error_handler() +// sinks() non const version // Has name, log level, vector of std::shared sink pointers and formatter // Upon each log write the logger: // 1. Checks if its log level is enough to log the message and if yes: @@ -19,8 +23,8 @@ #include #include "common.h" +#include "details/default_err_handler.h" #include "details/log_msg.h" -#include "details/error_handler.h" #include "sinks/sink.h" namespace spdlog { @@ -29,15 +33,13 @@ class SPDLOG_API logger { public: // Empty logger explicit logger(std::string name) - : name_(name), - err_handler_(std::move(name)) {} + : name_(name) {} // Logger with range on sinks template logger(std::string name, It begin, It end) : name_(name), - sinks_(begin, end), - err_handler_(std::move(name)) {} + sinks_(begin, end) {} // Logger with single sink logger(std::string name, sink_ptr single_sink) @@ -165,7 +167,8 @@ private: std::vector sinks_; atomic_level_t level_{level::info}; atomic_level_t flush_level_{level::off}; - details::error_handler err_handler_; + err_handler custom_err_handler_; + details::default_err_handler default_err_handler_; // common implementation for after templated public api has been resolved to format string and // args @@ -178,10 +181,10 @@ private: sink_it_(details::log_msg(loc, name_, lvl, string_view_t(buf.data(), buf.size()))); } catch (const std::exception &ex) { \ - err_handler_.handle(loc, ex.what()); \ + handle_error_(loc, ex.what()); \ } \ catch (...) { \ - err_handler_.handle(loc, "Unknown exception"); \ + handle_error_(loc, "Unknown exception"); \ } } @@ -194,10 +197,10 @@ private: sink->log(msg); } catch (const std::exception &ex) { \ - err_handler_.handle(msg.source, ex.what()); \ + handle_error_(msg.source, ex.what()); \ } \ catch (...) { \ - err_handler_.handle(msg.source, "Unknown exception"); \ + handle_error_(msg.source, "Unknown exception"); \ } } } @@ -208,6 +211,8 @@ private: } void flush_(); [[nodiscard]] bool should_flush_(const details::log_msg &msg) const; + + void handle_error_(const source_loc& loc, const std::string &err_msg) const; }; } // namespace spdlog diff --git a/include/spdlog/sinks/async_sink.h b/include/spdlog/sinks/async_sink.h index 5a839d1d..1f3fd14c 100644 --- a/include/spdlog/sinks/async_sink.h +++ b/include/spdlog/sinks/async_sink.h @@ -8,7 +8,7 @@ #include #include "../details/async_log_msg.h" -#include "../details/error_handler.h" +#include "../details/default_err_handler.h" #include "sink.h" // async_sink is a sink that sends log messages to a dist_sink in a separate thread using a queue. @@ -78,7 +78,7 @@ private: config config_; std::unique_ptr q_; std::thread worker_thread_; - details::error_handler err_handler_; + details::default_err_handler err_handler_; }; } // namespace sinks diff --git a/src/details/default_err_handler.cpp b/src/details/default_err_handler.cpp new file mode 100644 index 00000000..64858055 --- /dev/null +++ b/src/details/default_err_handler.cpp @@ -0,0 +1,27 @@ +// Copyright(c) 2015-present, Gabi Melman & spdlog contributors. +// Distributed under the MIT License (http://opensource.org/licenses/MIT) + +#include "iostream" +#include "spdlog/details/default_err_handler.h" +#include "spdlog/details/os.h" + +namespace spdlog { +namespace details { + +void default_err_handler::handle(const std::string &origin, const source_loc &loc, const std::string &err_msg) const { + std::lock_guard lk{mutex_}; + const auto tm_time = os::localtime(); + char date_buf[128]; + std::strftime(date_buf, sizeof(date_buf), "%Y-%m-%d %H:%M:%S", &tm_time); + std::string msg; + if (loc.empty()) { + msg = fmt_lib::format("[*** LOGGING ERROR ***] [{}] [{}] {}\n", date_buf, origin, err_msg); + } + else { + msg = fmt_lib::format("[*** LOGGING ERROR ***] [{}({})] [{}] [{}] {}\n", loc.filename, loc.line, date_buf, origin, err_msg); + } + std::fputs(msg.c_str(), stderr); +} + +} // namespace details +} // namespace spdlog diff --git a/src/details/error_handler.cpp b/src/details/error_handler.cpp deleted file mode 100644 index 68d1fa68..00000000 --- a/src/details/error_handler.cpp +++ /dev/null @@ -1,55 +0,0 @@ -// Copyright(c) 2015-present, Gabi Melman & spdlog contributors. -// Distributed under the MIT License (http://opensource.org/licenses/MIT) - -#include "spdlog/details/error_handler.h" -#include "spdlog/details/os.h" -#include "iostream" - -namespace spdlog { -namespace details { - -error_handler::error_handler(std::string name) - : name_(std::move(name)) {} - -error_handler::error_handler(const error_handler &other) { - std::lock_guard lk{other.mutex_}; - name_ = other.name_; - custom_handler_ = other.custom_handler_; -} - -error_handler::error_handler(error_handler &&other) noexcept { - std::lock_guard lk{other.mutex_}; - name_ = std::move(other.name_); - custom_handler_ = std::move(other.custom_handler_); -} - -void error_handler::handle(const source_loc &loc, const std::string &err_msg) const { - std::lock_guard lk{mutex_}; - if (custom_handler_) { - custom_handler_(err_msg); - return; - } - const auto tm_time = os::localtime(); - char date_buf[128]; - std::strftime(date_buf, sizeof(date_buf), "%Y-%m-%d %H:%M:%S", &tm_time); - std::string msg; - if (loc.empty()) { - msg = fmt_lib::format("[*** LOGGING ERROR ***] [{}] [{}] {}\n", date_buf, name_, err_msg); - } - else { - msg = fmt_lib::format("[*** LOGGING ERROR ***] [{}({})] [{}] [{}] {}\n", loc.filename, loc.line, date_buf, name_, err_msg); - } - std::fputs(msg.c_str(), stderr); -} -void error_handler::set_name(const std::string &name) { - std::lock_guard lk{mutex_}; - name_ = name; -} - -void error_handler::set_custom_handler(err_handler handler) { - std::lock_guard lk{mutex_}; - custom_handler_ = std::move(handler); -} - -} // namespace details -} // namespace spdlog diff --git a/src/logger.cpp b/src/logger.cpp index 6ab1c151..e4428d04 100644 --- a/src/logger.cpp +++ b/src/logger.cpp @@ -2,7 +2,6 @@ // Distributed under the MIT License (http://opensource.org/licenses/MIT) #include "spdlog/logger.h" -#include #include "spdlog/pattern_formatter.h" #include "spdlog/sinks/sink.h" @@ -15,14 +14,14 @@ logger::logger(const logger &other) noexcept sinks_(other.sinks_), level_(other.level_.load(std::memory_order_relaxed)), flush_level_(other.flush_level_.load(std::memory_order_relaxed)), - err_handler_(other.err_handler_) {} + custom_err_handler_(other.custom_err_handler_) {} logger::logger(logger &&other) noexcept : name_(std::move(other.name_)), sinks_(std::move(other.sinks_)), level_(other.level_.load(std::memory_order_relaxed)), flush_level_(other.flush_level_.load(std::memory_order_relaxed)), - err_handler_(std::move(other.err_handler_)) {} + custom_err_handler_(std::move(other.custom_err_handler_)) {} void logger::set_level(level level) { level_.store(level); } @@ -62,14 +61,13 @@ std::vector &logger::sinks() { return sinks_; } // custom error handler void logger::set_error_handler(err_handler handler) { - err_handler_.set_custom_handler(std::move(handler)); + custom_err_handler_ = std::move(handler); } // create new logger with same sinks and configuration. std::shared_ptr logger::clone(std::string logger_name) { auto cloned = std::make_shared(*this); cloned->name_ = std::move(logger_name); - cloned->err_handler_.set_name(cloned->name_); return cloned; } @@ -80,10 +78,10 @@ void logger::flush_() { sink->flush(); } catch (const std::exception &ex) { \ - err_handler_.handle(source_loc{}, ex.what()); \ + handle_error_(source_loc{}, ex.what()); \ } \ catch (...) { \ - err_handler_.handle(source_loc{}, "Unknown exception"); \ + handle_error_(source_loc{}, "Unknown exception"); \ } } } @@ -93,4 +91,12 @@ bool logger::should_flush_(const details::log_msg &msg) const { return (msg.log_level >= flush_level) && (msg.log_level != level::off); } +void logger::handle_error_(const source_loc &loc, const std::string &err_msg) const { + if (custom_err_handler_) { + custom_err_handler_(err_msg); + return; + } + default_err_handler_.handle(name_, loc, err_msg); +} + } // namespace spdlog diff --git a/src/sinks/async_sink.cpp b/src/sinks/async_sink.cpp index 6c1f2f80..0ba9e0d8 100644 --- a/src/sinks/async_sink.cpp +++ b/src/sinks/async_sink.cpp @@ -15,7 +15,7 @@ namespace spdlog { namespace sinks { async_sink::async_sink(config async_config) - : config_(std::move(async_config)), err_handler_("async_sink") { + : config_(std::move(async_config)) { if (config_.queue_size == 0 || config_.queue_size > max_queue_size) { throw spdlog_ex("async_sink: invalid queue size"); } @@ -107,7 +107,7 @@ void async_sink::backend_log_(const details::log_msg &msg) { try { sink->log(msg); } catch (const std::exception &ex) { - err_handler_.handle(msg.source, std::string("async log failed: ") + ex.what()); + err_handler_.handle("async", msg.source, std::string("async log failed: ") + ex.what()); } } } @@ -118,9 +118,9 @@ void async_sink::backend_flush_() { try { sink->flush(); } catch (const std::exception &ex) { - err_handler_.handle(source_loc{}, std::string("async flush failed: ") + ex.what()); + err_handler_.handle("async", source_loc{}, std::string("async flush failed: ") + ex.what()); } catch (...) { - err_handler_.handle(source_loc{}, "Async flush failed with unknown exception"); + err_handler_.handle("async", source_loc{}, "Async flush failed with unknown exception"); } } } diff --git a/tests/test_errors.cpp b/tests/test_errors.cpp index 9a04d4a3..0c959428 100644 --- a/tests/test_errors.cpp +++ b/tests/test_errors.cpp @@ -7,51 +7,60 @@ #include "includes.h" #include "spdlog/sinks/basic_file_sink.h" -#define SIMPLE_LOG "test_logs/simple_log.txt" -#define SIMPLE_ASYNC_LOG "test_logs/simple_async_log.txt" +static std::string log_filename = "test_logs/simple_log.txt"; +static std::string log_err_msg = "Error during log"; +static std::string flush_err_msg = "Error during flush"; class failing_sink final : public spdlog::sinks::base_sink { protected: - void sink_it_(const spdlog::details::log_msg &) final { throw std::runtime_error("some error happened during log"); } - - void flush_() final { throw std::runtime_error("some error happened during flush"); } + void sink_it_(const spdlog::details::log_msg &) override { throw std::runtime_error(log_err_msg.c_str()); } + void flush_() override { throw std::runtime_error(flush_err_msg.c_str()); } }; struct custom_ex {}; using namespace spdlog::sinks; TEST_CASE("default_error_handler", "[errors]") { prepare_logdir(); - spdlog::filename_t filename = SPDLOG_FILENAME_T(SIMPLE_LOG); + spdlog::filename_t filename = SPDLOG_FILENAME_T(log_filename); auto logger = spdlog::create("test-error", filename); logger->set_pattern("%v"); logger->info(SPDLOG_FMT_RUNTIME("Test message {} {}"), 1); logger->info("Test message {}", 2); logger->flush(); using spdlog::details::os::default_eol; - REQUIRE(file_contents(SIMPLE_LOG) == spdlog::fmt_lib::format("Test message 2{}", default_eol)); - REQUIRE(count_lines(SIMPLE_LOG) == 1); + REQUIRE(file_contents(log_filename) == spdlog::fmt_lib::format("Test message 2{}", default_eol)); + REQUIRE(count_lines(log_filename) == 1); } TEST_CASE("custom_error_handler", "[errors]") { prepare_logdir(); - spdlog::filename_t filename = SPDLOG_FILENAME_T(SIMPLE_LOG); + spdlog::filename_t filename = SPDLOG_FILENAME_T(log_filename); auto logger = spdlog::create("test-error", filename); logger->flush_on(spdlog::level::info); - logger->set_error_handler([=](const std::string &) { throw custom_ex(); }); + logger->set_error_handler([=](const std::string & msg) { + REQUIRE(msg == "argument not found"); + throw custom_ex(); + }); logger->info("Good message #1"); REQUIRE_THROWS_AS(logger->info(SPDLOG_FMT_RUNTIME("Bad format msg {} {}"), "xxx"), custom_ex); logger->info("Good message #2"); - require_message_count(SIMPLE_LOG, 2); + require_message_count(log_filename, 2); } TEST_CASE("default_error_handler2", "[errors]") { auto logger = std::make_shared("failed_logger", std::make_shared()); - logger->set_error_handler([=](const std::string &) { throw custom_ex(); }); + logger->set_error_handler([=](const std::string &msg) { + REQUIRE(msg == log_err_msg); + throw custom_ex(); + }); REQUIRE_THROWS_AS(logger->info("Some message"), custom_ex); } TEST_CASE("flush_error_handler", "[errors]") { auto logger = spdlog::create("failed_logger"); - logger->set_error_handler([=](const std::string &) { throw custom_ex(); }); + logger->set_error_handler([=](const std::string &msg) { + REQUIRE(msg == flush_err_msg); + throw custom_ex(); + }); REQUIRE_THROWS_AS(logger->flush(), custom_ex); }