From 0b74604a9f069086aa491f46d0f4a3f34520d153 Mon Sep 17 00:00:00 2001 From: Jae-Heon Ji <32578710+jaeheonji@users.noreply.github.com> Date: Sun, 13 Mar 2022 20:46:03 +0900 Subject: [PATCH] feat: improve error reporting system (#1038) * feat: add draft a concept code * feat: add err_ctx to diagnostic * feat: optimize error report * chore: add user-friendly help * fix: manual merge lockfile from main --- Cargo.lock | 114 +++++++++++++++++++++++++++++++---- zellij-client/src/lib.rs | 2 + zellij-utils/Cargo.toml | 1 + zellij-utils/src/errors.rs | 120 +++++++++++++++++++------------------ 4 files changed, 170 insertions(+), 67 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9191f341f..44d90b6e9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4,11 +4,11 @@ version = 3 [[package]] name = "addr2line" -version = "0.15.2" +version = "0.17.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e7a2e47a1fbe209ee101dd6d61285226744c6c8d3c21c8dc878ba6cb9f467f3a" +checksum = "b9ecd88a8c8378ca913a680cd98f0f13ac67383d35993f86c90a70e3f137816b" dependencies = [ - "gimli 0.24.0", + "gimli 0.26.1", ] [[package]] @@ -218,16 +218,16 @@ checksum = "d468802bab17cbc0cc575e9b053f41e72aa36bfa6b7f55e3529ffa43161b97fa" [[package]] name = "backtrace" -version = "0.3.59" +version = "0.3.64" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4717cfcbfaa661a0fd48f8453951837ae7e8f81e481fbb136e3202d72805a744" +checksum = "5e121dee8023ce33ab248d9ce1493df03c3b38a659b240096fcbd7048ff9c31f" dependencies = [ "addr2line", "cc", "cfg-if 1.0.0", "libc", "miniz_oxide", - "object 0.24.0", + "object 0.27.1", "rustc-demangle", ] @@ -931,9 +931,9 @@ dependencies = [ [[package]] name = "gimli" -version = "0.24.0" +version = "0.26.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0e4075386626662786ddb0ec9081e7c7eeb1ba31951f447ca780ef9f5d568189" +checksum = "78cc372d058dcf6d5ecd98510e7fbc9e5aec4d21de70f65fea8fecebcd881bd4" [[package]] name = "gloo-timers" @@ -1088,6 +1088,12 @@ dependencies = [ "syn", ] +[[package]] +name = "is_ci" +version = "1.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "616cde7c720bb2bb5824a224687d8f77bfd38922027f01d825cd7453be5099fb" + [[package]] name = "itoa" version = "0.4.8" @@ -1270,6 +1276,36 @@ dependencies = [ "autocfg", ] +[[package]] +name = "miette" +version = "3.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cd2adcfcced5d625bf90a958a82ae5b93231f57f3df1383fee28c9b5096d35ed" +dependencies = [ + "atty", + "backtrace", + "miette-derive", + "once_cell", + "owo-colors", + "supports-color", + "supports-hyperlinks", + "supports-unicode", + "terminal_size", + "textwrap", + "thiserror", +] + +[[package]] +name = "miette-derive" +version = "3.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5c01a8b61312d367ce87956bb686731f87e4c6dd5dbc550e8f06e3c24fb1f67f" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "miniz_oxide" version = "0.4.4" @@ -1385,9 +1421,12 @@ dependencies = [ [[package]] name = "object" -version = "0.24.0" +version = "0.27.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1a5b3dd1c072ee7963717671d1ca129f1048fda25edea6b752bfc71ac8854170" +checksum = "67ac1d3f9a1d3616fd9a60c8d74296f22406a238b6a72f5cc1e6f314df4ffbf9" +dependencies = [ + "memchr", +] [[package]] name = "once_cell" @@ -1426,6 +1465,12 @@ dependencies = [ "memchr", ] +[[package]] +name = "owo-colors" +version = "3.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "20448fd678ec04e6ea15bbe0476874af65e98a01515d667aa49f1434dc44ebf4" + [[package]] name = "parking" version = "2.0.0" @@ -1882,6 +1927,12 @@ version = "1.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1ecab6c735a6bb4139c0caafd0cc3635748bbb3acf4550e8138122099251f309" +[[package]] +name = "smawk" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f67ad224767faa3c7d8b6d91985b78e70a1324408abcb1cfcc2be4c06bc06043" + [[package]] name = "socket2" version = "0.4.2" @@ -1985,6 +2036,34 @@ dependencies = [ "lev_distance", ] +[[package]] +name = "supports-color" +version = "1.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4872ced36b91d47bae8a214a683fe54e7078875b399dfa251df346c9b547d1f9" +dependencies = [ + "atty", + "is_ci", +] + +[[package]] +name = "supports-hyperlinks" +version = "1.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "590b34f7c5f01ecc9d78dba4b3f445f31df750a67621cf31626f3b7441ce6406" +dependencies = [ + "atty", +] + +[[package]] +name = "supports-unicode" +version = "1.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a8b945e45b417b125a8ec51f1b7df2f8df7920367700d1f98aedd21e5735f8b2" +dependencies = [ + "atty", +] + [[package]] name = "syn" version = "1.0.81" @@ -2078,6 +2157,11 @@ name = "textwrap" version = "0.14.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0066c8d12af8b5acd21e00547c3797fde4e8677254a7ee429176ccebbe93dd80" +dependencies = [ + "smawk", + "unicode-linebreak", + "unicode-width", +] [[package]] name = "thiserror" @@ -2214,6 +2298,15 @@ version = "0.3.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1a01404663e3db436ed2746d9fefef640d868edae3cceb81c3b8d5732fda678f" +[[package]] +name = "unicode-linebreak" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3a52dcaab0c48d931f7cc8ef826fa51690a08e1ea55117ef26f89864f532383f" +dependencies = [ + "regex", +] + [[package]] name = "unicode-normalization" version = "0.1.19" @@ -2775,6 +2868,7 @@ dependencies = [ "libc", "log", "log4rs", + "miette", "nix", "once_cell", "serde", diff --git a/zellij-client/src/lib.rs b/zellij-client/src/lib.rs index b2686cbf8..bd5db5cd1 100644 --- a/zellij-client/src/lib.rs +++ b/zellij-client/src/lib.rs @@ -194,7 +194,9 @@ pub fn start_client( std::panic::set_hook({ use zellij_utils::errors::handle_panic; let send_client_instructions = send_client_instructions.clone(); + let os_input = os_input.clone(); Box::new(move |info| { + os_input.unset_raw_mode(0); handle_panic(info, &send_client_instructions); }) }); diff --git a/zellij-utils/Cargo.toml b/zellij-utils/Cargo.toml index 3ac8f3b47..cb4042f2c 100644 --- a/zellij-utils/Cargo.toml +++ b/zellij-utils/Cargo.toml @@ -37,6 +37,7 @@ zellij-tile = { path = "../zellij-tile/", version = "0.27.0" } log = "0.4.14" log4rs = "1.0.0" unicode-width = "0.1.8" +miette = { version = "3.3.0", features = ["fancy"] } [dependencies.async-std] version = "1.3.0" diff --git a/zellij-utils/src/errors.rs b/zellij-utils/src/errors.rs index 7ac0df85c..e36fa99f1 100644 --- a/zellij-utils/src/errors.rs +++ b/zellij-utils/src/errors.rs @@ -7,6 +7,9 @@ use serde::{Deserialize, Serialize}; use std::fmt::{Display, Error, Formatter}; use std::panic::PanicInfo; +use miette::{Diagnostic, GraphicalReportHandler, GraphicalTheme, Report}; +use thiserror::Error as ThisError; + /// The maximum amount of calls an [`ErrorContext`] will keep track /// of in its stack representation. This is a per-thread maximum. const MAX_THREAD_CALL_STACK: usize = 6; @@ -15,86 +18,84 @@ pub trait ErrorInstruction { fn error(err: String) -> Self; } +#[derive(Debug, ThisError, Diagnostic)] +#[error("{0}{}", self.show_backtrace())] +#[diagnostic(help("{}", self.show_help()))] +struct Panic(String); + +impl Panic { + fn show_backtrace(&self) -> String { + if let Ok(var) = std::env::var("RUST_BACKTRACE") { + if !var.is_empty() && var != "0" { + return format!("\n{:?}", backtrace::Backtrace::new()); + } + } + "".into() + } + + fn show_help(&self) -> String { + r#"If you are seeing this message, it means that something went wrong. +Please report this error to the github issue. +(https://github.com/zellij-org/zellij/issues) + +Also, if you want to see the backtrace, you can set the `RUST_BACKTRACE` environment variable to `1`. +"#.into() + } +} + +fn fmt_report(diag: Report) -> String { + let mut out = String::new(); + GraphicalReportHandler::new_themed(GraphicalTheme::unicode()) + .render_report(&mut out, diag.as_ref()) + .unwrap(); + out +} + /// Custom panic handler/hook. Prints the [`ErrorContext`]. pub fn handle_panic(info: &PanicInfo<'_>, sender: &SenderWithContext) where T: ErrorInstruction + Clone, { - use backtrace::Backtrace; use std::{process, thread}; - let backtrace = Backtrace::new(); let thread = thread::current(); let thread = thread.name().unwrap_or("unnamed"); let msg = match info.payload().downcast_ref::<&'static str>() { Some(s) => Some(*s), None => info.payload().downcast_ref::().map(|s| &**s), - }; + } + .unwrap_or("An unexpected error occurred!"); let err_ctx = OPENCALLS.with(|ctx| *ctx.borrow()); - let backtrace = match (info.location(), msg) { - (Some(location), Some(msg)) => format!( - "{}\n\u{1b}[0;0mError: \u{1b}[0;31mthread '{}' panicked at '{}': {}:{}\n\u{1b}[0;0m{:?}", - err_ctx, - thread, - msg, - location.file(), - location.line(), - backtrace, - ), - (Some(location), None) => format!( - "{}\n\u{1b}[0;0mError: \u{1b}[0;31mthread '{}' panicked: {}:{}\n\u{1b}[0;0m{:?}", - err_ctx, - thread, - location.file(), - location.line(), - backtrace - ), - (None, Some(msg)) => format!( - "{}\n\u{1b}[0;0mError: \u{1b}[0;31mthread '{}' panicked at '{}'\n\u{1b}[0;0m{:?}", - err_ctx, thread, msg, backtrace - ), - (None, None) => format!( - "{}\n\u{1b}[0;0mError: \u{1b}[0;31mthread '{}' panicked\n\u{1b}[0;0m{:?}", - err_ctx, thread, backtrace - ), - }; + let mut report: Report = Panic(format!("\u{1b}[0;31m{}\u{1b}[0;0m", msg)).into(); - let one_line_backtrace = match (info.location(), msg) { - (Some(location), Some(msg)) => format!( - "{}\n\u{1b}[0;0mError: \u{1b}[0;31mthread '{}' panicked at '{}': {}:{}\n\u{1b}[0;0m", - err_ctx, - thread, - msg, + if let Some(location) = info.location() { + report = report.wrap_err(format!( + "At {}:{}:{}", location.file(), location.line(), - ), - (Some(location), None) => format!( - "{}\n\u{1b}[0;0mError: \u{1b}[0;31mthread '{}' panicked: {}:{}\n\u{1b}[0;0m", - err_ctx, - thread, - location.file(), - location.line(), - ), - (None, Some(msg)) => format!( - "{}\n\u{1b}[0;0mError: \u{1b}[0;31mthread '{}' panicked at '{}'\n\u{1b}[0;0m", - err_ctx, thread, msg - ), - (None, None) => format!( - "{}\n\u{1b}[0;0mError: \u{1b}[0;31mthread '{}' panicked\n\u{1b}[0;0m", - err_ctx, thread - ), - }; + location.column() + )); + } + + if !err_ctx.is_empty() { + report = report.wrap_err(format!("{}", err_ctx)); + } + + report = report.wrap_err(format!( + "Thread '\u{1b}[0;31m{}\u{1b}[0;0m' panicked.", + thread + )); if thread == "main" { // here we only show the first line because the backtrace is not readable otherwise // a better solution would be to escape raw mode before we do this, but it's not trivial // to get os_input here - println!("\u{1b}[2J{}", one_line_backtrace); + println!("\u{1b}[2J{}", fmt_report(report)); process::exit(1); } else { - let _ = sender.send(T::error(backtrace)); + let _ = sender.send(T::error(format!("{}", fmt_report(report)))); } } @@ -119,6 +120,11 @@ impl ErrorContext { } } + /// Returns `true` if the calls has all [`Empty`](ContextType::Empty) calls. + pub fn is_empty(&self) -> bool { + self.calls.iter().all(|c| c == &ContextType::Empty) + } + /// Adds a call to this [`ErrorContext`]'s call stack representation. pub fn add_call(&mut self, call: ContextType) { for ctx in &mut self.calls { @@ -146,12 +152,12 @@ impl Default for ErrorContext { impl Display for ErrorContext { fn fmt(&self, f: &mut Formatter) -> Result<(), Error> { - writeln!(f, "Originating Thread(s):")?; + writeln!(f, "Originating Thread(s)")?; for (index, ctx) in self.calls.iter().enumerate() { if *ctx == ContextType::Empty { break; } - writeln!(f, "\u{1b}[0;0m{}. {}", index + 1, ctx)?; + writeln!(f, "\t\u{1b}[0;0m{}. {}", index + 1, ctx)?; } Ok(()) }