-
Notifications
You must be signed in to change notification settings - Fork 369
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add rerun --strict
: crash if any warning or error is logged
#1812
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -64,6 +64,10 @@ struct Args { | |
#[clap(long)] | ||
profile: bool, | ||
|
||
/// Exit with a non-zero exit code if any warning or error is logged. Useful for tests. | ||
#[clap(long)] | ||
strict: bool, | ||
|
||
/// An upper limit on how much memory the Rerun Viewer should use. | ||
/// | ||
/// When this limit is used, Rerun will purge the oldest data. | ||
|
@@ -187,6 +191,11 @@ where | |
return Ok(0); | ||
} | ||
|
||
if args.strict { | ||
re_log::add_boxed_logger(Box::new(StrictLogger {})).expect("Failed to enter --strict mode"); | ||
re_log::info!("--strict mode: any warning or error will cause Rerun to panic."); | ||
} | ||
|
||
let res = if let Some(commands) = &args.commands { | ||
match commands { | ||
#[cfg(all(feature = "analytics"))] | ||
|
@@ -539,3 +548,35 @@ pub fn setup_ctrl_c_handler() -> (tokio::sync::broadcast::Receiver<()>, Arc<Atom | |
.expect("Error setting Ctrl-C handler"); | ||
(receiver, shutdown_return) | ||
} | ||
|
||
// ---------------------------------------------------------------------------- | ||
|
||
use re_log::external::log; | ||
|
||
struct StrictLogger {} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: this file here is large enough, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd keep it here since it directly reference argument names ( |
||
|
||
impl log::Log for StrictLogger { | ||
fn enabled(&self, metadata: &log::Metadata<'_>) -> bool { | ||
match metadata.level() { | ||
log::Level::Error | log::Level::Warn => true, | ||
log::Level::Info | log::Level::Debug | log::Level::Trace => false, | ||
} | ||
} | ||
|
||
fn log(&self, record: &log::Record<'_>) { | ||
let level = match record.level() { | ||
log::Level::Error => "error", | ||
log::Level::Warn => "warning", | ||
log::Level::Info | log::Level::Debug | log::Level::Trace => return, | ||
}; | ||
|
||
eprintln!("{level} logged in --strict mode: {}", record.args()); | ||
eprintln!( | ||
"{}", | ||
crate::crash_handler::callstack_from(&["log::__private_api_log"]) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. where does this string come from? Does it need to be updated if logging crate is updated? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It comes from the callstack, and yes, it will need to. The full callstack leading up to
and I want to trim away as much irrelevant stuff as possible |
||
); | ||
std::process::exit(1); | ||
} | ||
|
||
fn flush(&self) {} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is it called
start_pattern
if it can appear anywhere in the string? (was already the case)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything before it is trimmed away, so it becomes the start of the returned stacktrace