From 6cd9cf2ea61937665666fdd6e388625e509bb5a5 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 21 Mar 2023 16:32:53 +0100 Subject: [PATCH 1/3] Detect failure to install GUI log callback --- crates/re_log/src/lib.rs | 2 +- crates/re_log/src/multi_logger.rs | 24 ++++++++++++++++-------- crates/re_log/src/setup.rs | 2 +- crates/re_ui/examples/re_ui_example.rs | 2 +- crates/re_viewer/src/app.rs | 7 ++++++- 5 files changed, 25 insertions(+), 12 deletions(-) diff --git a/crates/re_log/src/lib.rs b/crates/re_log/src/lib.rs index 50ff693a272c..68f77b420b09 100644 --- a/crates/re_log/src/lib.rs +++ b/crates/re_log/src/lib.rs @@ -31,7 +31,7 @@ pub use log_once::{debug_once, error_once, info_once, trace_once, warn_once}; pub use { channel_logger::*, - multi_logger::{add_boxed_logger, add_logger}, + multi_logger::{add_boxed_logger, add_logger, MultiLoggerNotSetupError}, setup::*, }; diff --git a/crates/re_log/src/multi_logger.rs b/crates/re_log/src/multi_logger.rs index d11ea5a0221b..3cbe712490f7 100644 --- a/crates/re_log/src/multi_logger.rs +++ b/crates/re_log/src/multi_logger.rs @@ -6,6 +6,13 @@ static MULTI_LOGGER: MultiLogger = MultiLogger::new(); static HAS_MULTI_LOGGER: AtomicBool = AtomicBool::new(false); +/// Produced when trying to install additional loggers when [`init`] has not been called. +/// +/// This can happend for example when users of the `rerun` crate use the `spawn` method, +/// and they aren't using `re_log`. +#[derive(Clone, Copy, Debug)] +pub struct MultiLoggerNotSetupError {} + /// Install the multi-logger as the default logger. pub fn init() -> Result<(), log::SetLoggerError> { HAS_MULTI_LOGGER.store(true, SeqCst); @@ -13,17 +20,18 @@ pub fn init() -> Result<(), log::SetLoggerError> { } /// Install an additional global logger. -pub fn add_boxed_logger(logger: Box) { - add_logger(Box::leak(logger)); +pub fn add_boxed_logger(logger: Box) -> Result<(), MultiLoggerNotSetupError> { + add_logger(Box::leak(logger)) } /// Install an additional global logger. -pub fn add_logger(logger: &'static dyn log::Log) { - debug_assert!( - HAS_MULTI_LOGGER.load(SeqCst), - "You forgot to setup multi-logging" - ); - MULTI_LOGGER.loggers.write().push(logger); +pub fn add_logger(logger: &'static dyn log::Log) -> Result<(), MultiLoggerNotSetupError> { + if HAS_MULTI_LOGGER.load(SeqCst) { + MULTI_LOGGER.loggers.write().push(logger); + Ok(()) + } else { + Err(MultiLoggerNotSetupError {}) + } } /// Forward log messages to multiple [`log::log`] receivers. diff --git a/crates/re_log/src/setup.rs b/crates/re_log/src/setup.rs index 1fb270027f7e..28330c18379d 100644 --- a/crates/re_log/src/setup.rs +++ b/crates/re_log/src/setup.rs @@ -42,7 +42,7 @@ pub fn setup_native_logging() { let mut stderr_logger = env_logger::Builder::new(); stderr_logger.parse_filters(&log_filter); - crate::add_boxed_logger(Box::new(stderr_logger.build())); + crate::add_boxed_logger(Box::new(stderr_logger.build())).expect("Failed to install logger"); } #[cfg(target_arch = "wasm32")] diff --git a/crates/re_ui/examples/re_ui_example.rs b/crates/re_ui/examples/re_ui_example.rs index 40662ea06c58..df765aaa4cd1 100644 --- a/crates/re_ui/examples/re_ui_example.rs +++ b/crates/re_ui/examples/re_ui_example.rs @@ -54,7 +54,7 @@ pub struct ExampleApp { impl ExampleApp { fn new(re_ui: re_ui::ReUi) -> Self { let (logger, text_log_rx) = re_log::ChannelLogger::new(re_log::LevelFilter::Info); - re_log::add_boxed_logger(Box::new(logger)); + re_log::add_boxed_logger(Box::new(logger)).unwrap(); let tree = egui_dock::Tree::new(vec![1, 2, 3]); diff --git a/crates/re_viewer/src/app.rs b/crates/re_viewer/src/app.rs index 33f23822e3ee..e14d43d2de88 100644 --- a/crates/re_viewer/src/app.rs +++ b/crates/re_viewer/src/app.rs @@ -111,7 +111,12 @@ impl App { shutdown: std::sync::Arc, ) -> Self { let (logger, text_log_rx) = re_log::ChannelLogger::new(re_log::LevelFilter::Info); - re_log::add_boxed_logger(Box::new(logger)); + if re_log::add_boxed_logger(Box::new(logger)).is_err() { + // This can happen when `rerun` crate users call `spawn`. TODO(emilk): make `spawn` spawn a new process. + re_log::debug!( + "re_log not initialized - we won't see any log messages as GUI notifications" + ); + } let state: AppState = storage .and_then(|storage| eframe::get_value(storage, eframe::APP_KEY)) From bb2e272b25a0486fc073f44092d1e82800dcedca Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 21 Mar 2023 16:36:29 +0100 Subject: [PATCH 2/3] typo --- crates/re_log/src/multi_logger.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/re_log/src/multi_logger.rs b/crates/re_log/src/multi_logger.rs index 3cbe712490f7..400545b69824 100644 --- a/crates/re_log/src/multi_logger.rs +++ b/crates/re_log/src/multi_logger.rs @@ -8,7 +8,7 @@ static HAS_MULTI_LOGGER: AtomicBool = AtomicBool::new(false); /// Produced when trying to install additional loggers when [`init`] has not been called. /// -/// This can happend for example when users of the `rerun` crate use the `spawn` method, +/// This can happen for example when users of the `rerun` crate use the `spawn` method, /// and they aren't using `re_log`. #[derive(Clone, Copy, Debug)] pub struct MultiLoggerNotSetupError {} From 8a317e99ea9753383780dba236a4fd94eaa9e9a1 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 21 Mar 2023 16:47:57 +0100 Subject: [PATCH 3/3] Fix compilation on wasm --- crates/re_log/src/setup.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/re_log/src/setup.rs b/crates/re_log/src/setup.rs index 28330c18379d..d24d4e5d1597 100644 --- a/crates/re_log/src/setup.rs +++ b/crates/re_log/src/setup.rs @@ -51,5 +51,6 @@ pub fn setup_web_logging() { log::set_max_level(log::LevelFilter::Debug); crate::add_boxed_logger(Box::new(crate::web_logger::WebLogger::new( log::LevelFilter::Debug, - ))); + ))) + .expect("Failed to install logger"); }