Skip to content

Commit ed1ac88

Browse files
SpriteOvOSpriteOvO
committed
Refactor ErrorHandler to allow using the built-in handler externally
1 parent 6230181 commit ed1ac88

File tree

15 files changed

+219
-187
lines changed

15 files changed

+219
-187
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ impl<F: Formatter> SinkPropAccess for BenchSink<'_, F> {
7171
unimplemented!()
7272
}
7373

74-
fn set_error_handler(&self, _handler: Option<spdlog::ErrorHandler>) {
74+
fn set_error_handler(&self, _handler: spdlog::ErrorHandler) {
7575
unimplemented!()
7676
}
7777
}

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,20 +57,21 @@ 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 => PANIC_ERR.into(),
61+
Self::Async => (move |err| {
6262
if let Error::SendToChannel(SendToChannelError::Full, _dropped_data) = err {
6363
// ignore
6464
} else {
6565
PANIC_ERR(err);
6666
}
67-
},
67+
})
68+
.into(),
6869
}
6970
}
7071
}
7172

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

7576
let logger = build_test_logger(|b| {
7677
b.error_handler(mode.error_handler())

‎spdlog/src/error.rs‎

Lines changed: 83 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,14 @@
88
use std::{
99
error::Error as StdError,
1010
fmt::{self, Display},
11-
io, result,
11+
io::{self, Write},
12+
result,
13+
sync::Arc,
1214
};
1315

14-
use atomic::Atomic;
1516
use thiserror::Error;
1617

1718
pub use crate::env_level::EnvLevelError;
18-
use crate::utils::const_assert;
1919
#[cfg(feature = "multi-thread")]
2020
use crate::{sink::Task, RecordOwned};
2121

@@ -278,11 +278,87 @@ pub struct BuildPatternError(pub(crate) spdlog_internal::pattern_parser::Error);
278278
/// The result type of this crate.
279279
pub type Result<T> = result::Result<T, Error>;
280280

281-
/// The error handler function type.
282-
pub type ErrorHandler = fn(Error);
281+
/// Represents an error handler.
282+
///
283+
/// In most cases, it can be constructed by just a `.into()`.
284+
///
285+
/// Call [`ErrorHandler::default`] to construct an empty error handler, when an
286+
/// error is triggered, a built-in fallback handler will be used which prints
287+
/// the error to `stderr`.
288+
#[derive(Clone)]
289+
pub struct ErrorHandler(Option<Arc<dyn Fn(Error) + Send + Sync>>);
290+
291+
impl ErrorHandler {
292+
/// Constructs an error handler with a custom function.
293+
#[must_use]
294+
pub fn new<F>(custom: F) -> Self
295+
where
296+
F: Fn(Error) + Send + Sync + 'static,
297+
{
298+
Self(Some(Arc::new(custom)))
299+
}
300+
301+
/// Calls the error handler with an error.
302+
pub fn call(&self, err: Error) {
303+
self.call_internal("External", err);
304+
}
305+
306+
pub(crate) fn call_internal(&self, from: impl AsRef<str>, err: Error) {
307+
if let Some(handler) = &self.0 {
308+
handler(err);
309+
} else {
310+
Self::default_impl(from, err);
311+
}
312+
}
313+
314+
fn default_impl(from: impl AsRef<str>, error: Error) {
315+
if let Error::Multiple(errs) = error {
316+
errs.into_iter()
317+
.for_each(|err| Self::default_impl(from.as_ref(), err));
318+
return;
319+
}
283320

284-
const_assert!(Atomic::<ErrorHandler>::is_lock_free());
285-
const_assert!(Atomic::<Option<ErrorHandler>>::is_lock_free());
321+
let date = chrono::Local::now()
322+
.format("%Y-%m-%d %H:%M:%S.%3f")
323+
.to_string();
324+
325+
// https://github.com/SpriteOvO/spdlog-rs/discussions/87
326+
//
327+
// Don't use `eprintln!` here, as it may fail to write and then panic.
328+
let _ = writeln!(
329+
io::stderr(),
330+
"[*** SPDLOG-RS UNHANDLED ERROR ***] [{}] [{}] {}",
331+
date,
332+
from.as_ref(),
333+
error
334+
);
335+
}
336+
}
337+
338+
impl<F> From<F> for ErrorHandler
339+
where
340+
F: Fn(Error) + Send + Sync + 'static,
341+
{
342+
fn from(handler: F) -> Self {
343+
Self::new(handler)
344+
}
345+
}
346+
347+
impl Default for ErrorHandler {
348+
/// Constructs an error handler with the built-in handler which prints
349+
/// errors to `stderr`.
350+
fn default() -> Self {
351+
Self(None)
352+
}
353+
}
354+
355+
impl fmt::Debug for ErrorHandler {
356+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
357+
f.debug_tuple("ErrorHandler")
358+
.field(&self.0.as_ref().map_or("default", |_| "custom"))
359+
.finish()
360+
}
361+
}
286362

287363
#[cfg(test)]
288364
mod tests {

‎spdlog/src/lib.rs‎

Lines changed: 1 addition & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -334,9 +334,7 @@ use std::{
334334
borrow::Cow,
335335
env::{self, VarError},
336336
ffi::OsStr,
337-
fmt,
338-
io::{self, Write},
339-
panic,
337+
fmt, panic,
340338
result::Result as StdResult,
341339
};
342340

@@ -772,29 +770,6 @@ fn flush_default_logger_at_exit() {
772770
}
773771
}
774772

775-
fn default_error_handler(from: impl AsRef<str>, error: Error) {
776-
if let Error::Multiple(errs) = error {
777-
errs.into_iter()
778-
.for_each(|err| default_error_handler(from.as_ref(), err));
779-
return;
780-
}
781-
782-
let date = chrono::Local::now()
783-
.format("%Y-%m-%d %H:%M:%S.%3f")
784-
.to_string();
785-
786-
// https://github.com/SpriteOvO/spdlog-rs/discussions/87
787-
//
788-
// Don't use `eprintln!` here, as it may fail to write and then panic.
789-
let _ = writeln!(
790-
io::stderr(),
791-
"[*** SPDLOG-RS UNHANDLED ERROR ***] [{}] [{}] {}",
792-
date,
793-
from.as_ref(),
794-
error
795-
);
796-
}
797-
798773
// Used at log macros
799774
#[doc(hidden)]
800775
pub fn __log(

‎spdlog/src/logger.rs‎

Lines changed: 27 additions & 31 deletions
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: RwLock<Option<ErrorHandler>>,
114+
error_handler: RwLock<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

@@ -379,12 +379,12 @@ impl Logger {
379379
/// ```
380380
/// use spdlog::prelude::*;
381381
///
382-
/// spdlog::default_logger().set_error_handler(Some(|err| {
382+
/// spdlog::default_logger().set_error_handler(|err| {
383383
/// panic!("An error occurred in the default logger: {}", err)
384-
/// }));
384+
/// });
385385
/// ```
386-
pub fn set_error_handler(&self, handler: Option<ErrorHandler>) {
387-
*self.error_handler.write_expect() = handler;
386+
pub fn set_error_handler<F: Into<ErrorHandler>>(&self, handler: F) {
387+
*self.error_handler.write_expect() = handler.into();
388388
}
389389

390390
/// Forks and configures a separate new logger.
@@ -479,7 +479,7 @@ impl Logger {
479479
sinks: self.sinks.clone(),
480480
flush_level_filter: Atomic::new(self.flush_level_filter()),
481481
periodic_flusher: Mutex::new(None),
482-
error_handler: RwLock::new(*self.error_handler.read_expect()),
482+
error_handler: RwLock::new(self.error_handler.read_expect().clone()),
483483
}
484484
}
485485

@@ -506,17 +506,13 @@ impl Logger {
506506
}
507507

508508
fn handle_error(&self, err: Error) {
509-
if let Some(handler) = self.error_handler.read_expect().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-
}
509+
self.error_handler.read_expect().call_internal(
510+
format!(
511+
"Logger ({})",
512+
self.name.as_ref().map_or("*no name*", String::as_str)
513+
),
514+
err,
515+
);
520516
}
521517

522518
#[must_use]
@@ -550,7 +546,7 @@ pub struct LoggerBuilder {
550546
level_filter: LevelFilter,
551547
sinks: Sinks,
552548
flush_level_filter: LevelFilter,
553-
error_handler: Option<ErrorHandler>,
549+
error_handler: ErrorHandler,
554550
}
555551

556552
impl LoggerBuilder {
@@ -624,8 +620,8 @@ impl LoggerBuilder {
624620
///
625621
/// See the documentation of [`Logger::set_error_handler`] for the
626622
/// description of this parameter.
627-
pub fn error_handler(&mut self, handler: ErrorHandler) -> &mut Self {
628-
self.error_handler = Some(handler);
623+
pub fn error_handler<F: Into<ErrorHandler>>(&mut self, handler: F) -> &mut Self {
624+
self.error_handler = handler.into();
629625
self
630626
}
631627

@@ -657,7 +653,7 @@ impl LoggerBuilder {
657653
level_filter: Atomic::new(self.level_filter),
658654
sinks: self.sinks.clone(),
659655
flush_level_filter: Atomic::new(self.flush_level_filter),
660-
error_handler: RwLock::new(self.error_handler),
656+
error_handler: RwLock::new(self.error_handler.clone()),
661657
periodic_flusher: Mutex::new(None),
662658
};
663659

‎spdlog/src/sink/async_sink/async_pool_sink.rs‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use crate::{
2-
default_error_handler, default_thread_pool,
2+
default_thread_pool,
33
formatter::{Formatter, UnreachableFormatter},
44
sink::{OverflowPolicy, Sink, SinkProp, SinkPropAccess, Sinks},
55
sync::*,
@@ -57,13 +57,13 @@ impl AsyncPoolSink {
5757
/// | Parameter | Default Value |
5858
/// |-------------------|-------------------------------------|
5959
/// | [level_filter] | `All` |
60-
/// | [error_handler] | [default error handler] |
60+
/// | [error_handler] | [`ErrorHandler::default()`] |
6161
/// | [overflow_policy] | `Block` |
6262
/// | [thread_pool] | internal shared default thread pool |
6363
///
6464
/// [level_filter]: AsyncPoolSinkBuilder::level_filter
6565
/// [error_handler]: AsyncPoolSinkBuilder::error_handler
66-
/// [default error handler]: error/index.html#default-error-handler
66+
/// [`ErrorHandler::default()`]: crate::error::ErrorHandler::default()
6767
/// [overflow_policy]: AsyncPoolSinkBuilder::overflow_policy
6868
/// [thread_pool]: AsyncPoolSinkBuilder::thread_pool
6969
#[must_use]
@@ -115,7 +115,7 @@ impl SinkPropAccess for AsyncPoolSink {
115115
}
116116
}
117117

118-
fn set_error_handler(&self, handler: Option<ErrorHandler>) {
118+
fn set_error_handler(&self, handler: ErrorHandler) {
119119
self.backend.prop.set_error_handler(handler);
120120
}
121121
}
@@ -222,8 +222,8 @@ impl AsyncPoolSinkBuilder {
222222
///
223223
/// This parameter is **optional**.
224224
#[must_use]
225-
pub fn error_handler(self, handler: ErrorHandler) -> Self {
226-
self.prop.set_error_handler(Some(handler));
225+
pub fn error_handler<F: Into<ErrorHandler>>(self, handler: F) -> Self {
226+
self.prop.set_error_handler(handler);
227227
self
228228
}
229229

@@ -267,9 +267,7 @@ impl Backend {
267267
}
268268

269269
fn handle_error(&self, err: Error) {
270-
self.prop
271-
.error_handler()
272-
.unwrap_or(|err| default_error_handler("AsyncPoolSink", err))(err);
270+
self.prop.call_error_handler_internal("AsyncPoolSink", err)
273271
}
274272
}
275273

0 commit comments

Comments
 (0)