Error handler

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"

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 <string>
+#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

include/spdlog/details/error_handler.h

@@ -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 <string>
-#include <mutex>
-#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

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 <vector>
 
 #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 <typename It>
     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<sink_ptr> 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

include/spdlog/sinks/async_sink.h

@@ -8,7 +8,7 @@
 #include <vector>
 
 #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<queue_t> q_;
     std::thread worker_thread_;
-    details::error_handler err_handler_;
+    details::default_err_handler err_handler_;
 };
 
 }  // namespace sinks

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

src/details/error_handler.cpp

@@ -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

src/logger.cpp

@@ -2,7 +2,6 @@
 // Distributed under the MIT License (http://opensource.org/licenses/MIT)
 
 #include "spdlog/logger.h"
-#include <cstdio>
 
 #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<sink_ptr> &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> logger::clone(std::string logger_name) {
     auto cloned = std::make_shared<logger>(*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

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");
         }
     }
 }

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<std::mutex> {
 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<basic_file_sink_mt>("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<basic_file_sink_mt>("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<spdlog::logger>("failed_logger", std::make_shared<failing_sink>());
-    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<failing_sink>("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);
 }