Skip to content

Commit 1d5cf9a

Browse files
SpriteOvOSpriteOvO
committed
Refactor ErrorHandler for using the built-in handler externally
1 parent 962f521 commit 1d5cf9a

20 files changed

+205
-155
lines changed

‎spdlog/benches/spdlog-rs/compare_with_cpp_spdlog_async.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use spdlog::{
1313
formatter::{pattern, PatternFormatter},
1414
prelude::*,
1515
sink::*,
16-
ThreadPool,
16+
ErrorHandler, ThreadPool,
1717
};
1818
use test::black_box;
1919

@@ -46,21 +46,21 @@ fn bench(
4646
.thread_pool(thread_pool)
4747
.overflow_policy(policy)
4848
.sink(file_sink)
49-
.error_handler(|err| panic!("an error occurred: {err}"))
49+
.error_handler(ErrorHandler::new(|err| panic!("an error occurred: {err}")))
5050
.build()
5151
.unwrap(),
5252
);
5353

5454
let logger = Logger::builder()
5555
.sink(async_sink)
5656
.name("async_logger")
57-
.error_handler(|err| {
57+
.error_handler(ErrorHandler::new(|err| {
5858
if let Error::SendToChannel(SendToChannelError::Full, _dropped_data) = err {
5959
// ignore
6060
} else {
6161
panic!("an error occurred: {err}")
6262
}
63-
})
63+
}))
6464
.build()
6565
.unwrap();
6666

‎spdlog/benches/spdlog-rs/pattern.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ impl<F: Formatter> Sink for BenchSink<'_, F> {
6868
unimplemented!()
6969
}
7070

71-
fn set_error_handler(&self, _handler: Option<spdlog::ErrorHandler>) {
71+
fn set_error_handler(&self, _handler: spdlog::ErrorHandler) {
7272
unimplemented!()
7373
}
7474
}

‎spdlog/benches/spdlog-rs/spdlog_rs.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -57,20 +57,20 @@ impl Mode {
5757
const PANIC_ERR: fn(Error) = |err| panic!("an error occurred: {err}");
5858

5959
match self {
60-
Self::Sync => PANIC_ERR,
61-
Self::Async => move |err| {
60+
Self::Sync => ErrorHandler::new(PANIC_ERR),
61+
Self::Async => ErrorHandler::new(move |err| {
6262
if let Error::SendToChannel(SendToChannelError::Full, _dropped_data) = err {
6363
// ignore
6464
} else {
6565
PANIC_ERR(err);
6666
}
67-
},
67+
}),
6868
}
6969
}
7070
}
7171

7272
fn bench_any(bencher: &mut Bencher, mode: Mode, sink: Arc<dyn Sink>) {
73-
sink.set_error_handler(Some(|err| panic!("an error occurred: {err}")));
73+
sink.set_error_handler(ErrorHandler::new(|err| panic!("an error occurred: {err}")));
7474

7575
let logger = build_test_logger(|b| {
7676
b.error_handler(mode.error_handler())

‎spdlog/examples/05_sink.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use spin::{Mutex, RwLock};
1212
struct CollectVecSink {
1313
level_filter: Atomic<LevelFilter>,
1414
formatter: RwLock<Box<dyn Formatter>>,
15-
error_handler: Atomic<Option<ErrorHandler>>,
15+
error_handler: Atomic<ErrorHandler>,
1616
collected: Mutex<Vec<String>>,
1717
}
1818

@@ -21,7 +21,7 @@ impl CollectVecSink {
2121
Self {
2222
level_filter: Atomic::new(LevelFilter::All),
2323
formatter: RwLock::new(Box::new(FullFormatter::new())),
24-
error_handler: Atomic::new(None),
24+
error_handler: Atomic::new(ErrorHandler::default()),
2525
collected: Mutex::new(Vec::new()),
2626
}
2727
}
@@ -58,7 +58,7 @@ impl Sink for CollectVecSink {
5858
*self.formatter.write() = formatter;
5959
}
6060

61-
fn set_error_handler(&self, handler: Option<ErrorHandler>) {
61+
fn set_error_handler(&self, handler: ErrorHandler) {
6262
self.error_handler.store(handler, Ordering::Relaxed);
6363
}
6464
}

‎spdlog/src/error.rs

+79-10
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,4 @@
11
//! Provides error types.
2-
//!
3-
//! # Default error handler
4-
//!
5-
//! If a logger or sink does not have an error handler set up, a default error
6-
//! handler will be used, which will print the error to `stderr`.
7-
82
use std::{
93
error::Error as StdError,
104
fmt::{self, Display},
@@ -65,7 +59,7 @@ use crate::{sink::Task, RecordOwned};
6559
/// # { unimplemented!() }
6660
/// fn set_formatter(&self, formatter: Box<dyn Formatter>) /* ... */
6761
/// # { unimplemented!() }
68-
/// fn set_error_handler(&self, handler: Option<spdlog::ErrorHandler>) /* ... */
62+
/// fn set_error_handler(&self, handler: spdlog::ErrorHandler) /* ... */
6963
/// # { unimplemented!() }
7064
/// }
7165
/// ```
@@ -340,11 +334,86 @@ pub struct BuildPatternError(pub(crate) spdlog_internal::pattern_parser::Error);
340334
/// The result type of this crate.
341335
pub type Result<T> = result::Result<T, Error>;
342336

343-
/// The error handler function type.
344-
pub type ErrorHandler = fn(Error);
337+
/// Represents an error handler.
338+
///
339+
/// Call [`ErrorHandler::new`] to construct an error handler with a custom
340+
/// function.
341+
///
342+
/// Call [`ErrorHandler::default`] to construct an empty error handler, when an
343+
/// error is triggered, a built-in fallback handler will be used which prints
344+
/// the error to `stderr`.
345+
#[derive(Copy, Clone, Debug)]
346+
pub struct ErrorHandler(Option<fn(Error)>);
345347

346348
const_assert!(Atomic::<ErrorHandler>::is_lock_free());
347-
const_assert!(Atomic::<Option<ErrorHandler>>::is_lock_free());
349+
350+
impl ErrorHandler {
351+
/// Constructs an error handler with a custom function.
352+
#[must_use]
353+
pub fn new(custom: fn(Error)) -> Self {
354+
Self(Some(custom))
355+
}
356+
357+
/// Sets the error handler.
358+
///
359+
/// Passes `None` to use the built-in fallback handler, which prints errors
360+
/// to `stderr`.
361+
pub fn set(&mut self, handler: Option<fn(Error)>) {
362+
self.0 = handler;
363+
}
364+
365+
/// Calls the error handler with an error.
366+
pub fn call(&self, err: Error) {
367+
self.call_internal("External", err);
368+
}
369+
370+
pub(crate) fn call_internal(&self, from: impl AsRef<str>, err: Error) {
371+
if let Some(handler) = self.0 {
372+
handler(err);
373+
} else {
374+
Self::default_impl(from, err);
375+
}
376+
}
377+
378+
fn default_impl(from: impl AsRef<str>, error: Error) {
379+
if let Error::Multiple(errs) = error {
380+
errs.into_iter()
381+
.for_each(|err| Self::default_impl(from.as_ref(), err));
382+
return;
383+
}
384+
385+
let date = chrono::Local::now()
386+
.format("%Y-%m-%d %H:%M:%S.%3f")
387+
.to_string();
388+
389+
eprintln!(
390+
"[*** SPDLOG-RS UNHANDLED ERROR ***] [{}] [{}] {}",
391+
date,
392+
from.as_ref(),
393+
error
394+
);
395+
}
396+
}
397+
398+
impl Default for ErrorHandler {
399+
/// Constructs an error handler with the built-in handler which prints
400+
/// errors to `stderr`.
401+
fn default() -> Self {
402+
Self(None)
403+
}
404+
}
405+
406+
// FIXME: Doesn't work as expected at the moment
407+
// https://rust-lang.zulipchat.com/#narrow/channel/219381-t-libs/topic/impl.20From.3Cno-capture-closure.3E.20for.20fn.3F
408+
//
409+
// impl<F> From<F> for ErrorHandler
410+
// where
411+
// F: Into<fn(Error)>,
412+
// {
413+
// fn from(handler: F) -> Self {
414+
// Self::new(handler.into())
415+
// }
416+
// }
348417

349418
#[cfg(test)]
350419
mod tests {

‎spdlog/src/lib.rs

-19
Original file line numberDiff line numberDiff line change
@@ -758,25 +758,6 @@ fn flush_default_logger_at_exit() {
758758
}
759759
}
760760

761-
fn default_error_handler(from: impl AsRef<str>, error: Error) {
762-
if let Error::Multiple(errs) = error {
763-
errs.into_iter()
764-
.for_each(|err| default_error_handler(from.as_ref(), err));
765-
return;
766-
}
767-
768-
let date = chrono::Local::now()
769-
.format("%Y-%m-%d %H:%M:%S.%3f")
770-
.to_string();
771-
772-
eprintln!(
773-
"[*** SPDLOG-RS UNHANDLED ERROR ***] [{}] [{}] {}",
774-
date,
775-
from.as_ref(),
776-
error
777-
);
778-
}
779-
780761
// Used at log macros
781762
#[doc(hidden)]
782763
pub fn __log(

‎spdlog/src/logger.rs

+31-29
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ pub struct Logger {
111111
level_filter: Atomic<LevelFilter>,
112112
sinks: Sinks,
113113
flush_level_filter: Atomic<LevelFilter>,
114-
error_handler: SpinRwLock<Option<ErrorHandler>>,
114+
error_handler: SpinRwLock<ErrorHandler>,
115115
periodic_flusher: Mutex<Option<(Duration, PeriodicWorker)>>,
116116
}
117117

@@ -148,30 +148,30 @@ impl Debug for Logger {
148148
impl Logger {
149149
/// Gets a [`LoggerBuilder`] with default parameters:
150150
///
151-
/// | Parameter | Default Value |
152-
/// |----------------------|-------------------------|
153-
/// | [name] | `None` |
154-
/// | [sinks] | `[]` |
155-
/// | [level_filter] | `MoreSevereEqual(Info)` |
156-
/// | [flush_level_filter] | `Off` |
157-
/// | [flush_period] | `None` |
158-
/// | [error_handler] | [default error handler] |
151+
/// | Parameter | Default Value |
152+
/// |----------------------|-----------------------------|
153+
/// | [name] | `None` |
154+
/// | [sinks] | `[]` |
155+
/// | [level_filter] | `MoreSevereEqual(Info)` |
156+
/// | [flush_level_filter] | `Off` |
157+
/// | [flush_period] | `None` |
158+
/// | [error_handler] | [`ErrorHandler::default()`] |
159159
///
160160
/// [name]: LoggerBuilder::name
161161
/// [sinks]: LoggerBuilder::sink
162162
/// [level_filter]: LoggerBuilder::level_filter
163163
/// [flush_level_filter]: LoggerBuilder::flush_level_filter
164164
/// [flush_period]: Logger::set_flush_period
165165
/// [error_handler]: LoggerBuilder::error_handler
166-
/// [default error handler]: error/index.html#default-error-handler
166+
/// [`ErrorHandler::default()`]: crate::error::ErrorHandler::default()
167167
#[must_use]
168168
pub fn builder() -> LoggerBuilder {
169169
LoggerBuilder {
170170
name: None,
171171
level_filter: LevelFilter::MoreSevereEqual(Level::Info),
172172
sinks: vec![],
173173
flush_level_filter: LevelFilter::Off,
174-
error_handler: None,
174+
error_handler: ErrorHandler::default(),
175175
}
176176
}
177177

@@ -377,14 +377,17 @@ impl Logger {
377377
/// # Examples
378378
///
379379
/// ```
380-
/// use spdlog::prelude::*;
380+
/// use spdlog::{prelude::*, ErrorHandler};
381381
///
382-
/// spdlog::default_logger().set_error_handler(Some(|err| {
382+
/// spdlog::default_logger().set_error_handler(ErrorHandler::new(|err| {
383383
/// panic!("An error occurred in the default logger: {}", err)
384384
/// }));
385385
/// ```
386-
pub fn set_error_handler(&self, handler: Option<ErrorHandler>) {
387-
*self.error_handler.write() = handler;
386+
pub fn set_error_handler<H>(&self, handler: H)
387+
where
388+
H: Into<ErrorHandler>,
389+
{
390+
*self.error_handler.write() = handler.into();
388391
}
389392

390393
/// Forks and configures a separate new logger.
@@ -506,17 +509,13 @@ impl Logger {
506509
}
507510

508511
fn handle_error(&self, err: Error) {
509-
if let Some(handler) = self.error_handler.read().as_ref() {
510-
handler(err)
511-
} else {
512-
crate::default_error_handler(
513-
format!(
514-
"Logger ({})",
515-
self.name.as_ref().map_or("*no name*", String::as_str)
516-
),
517-
err,
518-
);
519-
}
512+
self.error_handler.read().call_internal(
513+
format!(
514+
"Logger ({})",
515+
self.name.as_ref().map_or("*no name*", String::as_str)
516+
),
517+
err,
518+
)
520519
}
521520

522521
#[must_use]
@@ -550,7 +549,7 @@ pub struct LoggerBuilder {
550549
level_filter: LevelFilter,
551550
sinks: Sinks,
552551
flush_level_filter: LevelFilter,
553-
error_handler: Option<ErrorHandler>,
552+
error_handler: ErrorHandler,
554553
}
555554

556555
impl LoggerBuilder {
@@ -624,8 +623,11 @@ impl LoggerBuilder {
624623
///
625624
/// See the documentation of [`Logger::set_error_handler`] for the
626625
/// description of this parameter.
627-
pub fn error_handler(&mut self, handler: ErrorHandler) -> &mut Self {
628-
self.error_handler = Some(handler);
626+
pub fn error_handler<H>(&mut self, handler: H) -> &mut Self
627+
where
628+
H: Into<ErrorHandler>,
629+
{
630+
self.error_handler = handler.into();
629631
self
630632
}
631633

0 commit comments

Comments
 (0)