From eb8964e1d18b187402bbaa57602c7c40f08f6f19 Mon Sep 17 00:00:00 2001 From: Lucien Greathouse Date: Thu, 12 Mar 2020 15:42:56 -0700 Subject: [PATCH] Improve panic messaging and process behavior. - Replaced main() to use custom panic hook - Updated log formatting - Switched to panic=abort, since we don't need to unwind now. - Process will now abort if any thread panics. --- CHANGELOG.md | 4 ++ Cargo.lock | 1 + Cargo.toml | 7 +++ src/bin.rs | 120 ++++++++++++++++++++++++++++++--------------------- 4 files changed, 84 insertions(+), 48 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 82e495bc..3a77d67b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ * Added `--watch` argument to `rojo build`. ([#284](https://github.com/rojo-rbx/rojo/pull/284)) * Added dark theme support to plugin. ([#241](https://github.com/rojo-rbx/rojo/issues/241)) * Simplified filesystem access code dramatically. +* Improved error reporting and logging across the board. + * Log messages have a less noisy prefix. + * Any thread panicking now causes Rojo to abort instead of existing as a zombie. + * Errors now have a list of causes, helping make many errors more clear. ## [0.6.0 Alpha 2](https://github.com/rojo-rbx/rojo/releases/tag/v0.6.0-alpha.2) (March 6, 2020) * Fixed `rojo upload` command always uploading models. diff --git a/Cargo.lock b/Cargo.lock index 9cf91fc1..133c55bb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1665,6 +1665,7 @@ dependencies = [ name = "rojo" version = "0.6.0-alpha.2" dependencies = [ + "backtrace 0.3.45 (registry+https://github.com/rust-lang/crates.io-index)", "criterion 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)", "crossbeam-channel 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)", "csv 1.1.3 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/Cargo.toml b/Cargo.toml index e3da831a..36df1fbb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,6 +15,12 @@ exclude = [ "/test-projects/**", ] +[profile.dev] +panic = "abort" + +[profile.release] +panic = "abort" + [features] default = [] @@ -56,6 +62,7 @@ harness = false [dependencies] memofs = { version = "0.1.0", path = "memofs" } +backtrace = "0.3" crossbeam-channel = "0.4.0" csv = "1.1.1" diff --git a/src/bin.rs b/src/bin.rs index 1b745ded..3d3189e1 100644 --- a/src/bin.rs +++ b/src/bin.rs @@ -1,49 +1,10 @@ -use std::{error::Error, panic, process}; +use std::{env, error::Error, panic, process}; -use log::error; +use backtrace::Backtrace; use structopt::StructOpt; use librojo::cli::{self, Options, Subcommand}; -fn main() { - let options = Options::from_args(); - - { - let log_filter = match options.verbosity { - 0 => "warn", - 1 => "warn,librojo=info", - 2 => "warn,librojo=trace", - _ => "trace", - }; - - let log_env = env_logger::Env::default().default_filter_or(log_filter); - - env_logger::Builder::from_env(log_env) - .format_timestamp(None) - .init(); - } - - let panic_result = panic::catch_unwind(|| { - if let Err(err) = run(options.subcommand) { - log::error!("{}", err); - process::exit(1); - } - }); - - if let Err(error) = panic_result { - let message = match error.downcast_ref::<&str>() { - Some(message) => message.to_string(), - None => match error.downcast_ref::() { - Some(message) => message.clone(), - None => "".to_string(), - }, - }; - - show_crash_message(&message); - process::exit(1); - } -} - fn run(subcommand: Subcommand) -> Result<(), Box> { match subcommand { Subcommand::Init(init_options) => cli::init(init_options)?, @@ -55,11 +16,74 @@ fn run(subcommand: Subcommand) -> Result<(), Box> { Ok(()) } -fn show_crash_message(message: &str) { - error!("Rojo crashed!"); - error!("This is a bug in Rojo."); - error!(""); - error!("Please consider filing a bug: https://github.com/rojo-rbx/rojo/issues"); - error!(""); - error!("Details: {}", message); +fn main() { + panic::set_hook(Box::new(|panic_info| { + // PanicInfo's payload is usually a &'static str or String. + // See: https://doc.rust-lang.org/beta/std/panic/struct.PanicInfo.html#method.payload + let message = match panic_info.payload().downcast_ref::<&str>() { + Some(message) => message.to_string(), + None => match panic_info.payload().downcast_ref::() { + Some(message) => message.clone(), + None => "".to_string(), + }, + }; + + log::error!("Rojo crashed!"); + log::error!("This is probably a Rojo bug."); + log::error!(""); + log::error!("Please consider filing an issue: https://github.com/rojo-rbx/rojo/issues"); + log::error!(""); + log::error!("Details: {}", message); + + if let Some(location) = panic_info.location() { + log::error!("in file {} on line {}", location.file(), location.line()); + } + + // When using the backtrace crate, we need to check the RUST_BACKTRACE + // environment variable ourselves. Once we switch to the (currently + // unstable) std::backtrace module, we won't need to do this anymore. + let should_backtrace = env::var("RUST_BACKTRACE") + .map(|var| var == "1") + .unwrap_or(false); + + if should_backtrace { + eprintln!("{:?}", Backtrace::new()); + } else { + eprintln!( + "note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace." + ); + } + + process::exit(1); + })); + + let options = Options::from_args(); + + let log_filter = match options.verbosity { + 0 => "warn", + 1 => "warn,librojo=info", + 2 => "warn,librojo=trace", + _ => "trace", + }; + + let log_env = env_logger::Env::default().default_filter_or(log_filter); + + env_logger::Builder::from_env(log_env) + .format_module_path(false) + .format_timestamp(None) + // Indent following lines equal to the log level label, like `[ERROR] ` + .format_indent(Some(8)) + .init(); + + if let Err(err) = run(options.subcommand) { + log::error!("{}", err); + + let mut current_err: &dyn Error = &*err; + while let Some(source) = current_err.source() { + log::error!(" caused by {}", err); + current_err = &*source; + } + + process::exit(1); + } }