From 0e382b8afa0bf5b41a625ce74e1cb6912743ec39 Mon Sep 17 00:00:00 2001 From: Yoshua Wuyts Date: Thu, 23 Aug 2018 21:42:58 +0200 Subject: [PATCH 1/6] Including original panic-cause in report Okay so this PR is a bit of a dilemma. The API we need to get the original panic message isn't stable and requires a feature flag. We can conditionally enable the feature flag for nightly users but that doesn't quite deal with the problem of what to do when someone is using `human-panic` on stable. As far as I can see there's no other API to get that information? We could always just gate it all away behind the `nightly` feature and just ommit a static "not supported" or something when running on stable. The tests pass (provided you pass `--features nightly` for now). Wanted to open this PR to discuss how to proceed :) --- src/lib.rs | 7 ++++++- src/report.rs | 3 +++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index bc70e69..b7531ea 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -42,6 +42,7 @@ #![cfg_attr(feature = "nightly", deny(missing_docs))] #![cfg_attr(feature = "nightly", feature(external_doc))] +#![cfg_attr(feature = "nightly", feature(panic_info_message))] extern crate backtrace; #[macro_use] @@ -179,6 +180,10 @@ pub fn print_msg>( /// Utility function which will handle dumping information to disk pub fn handle_dump(meta: &Metadata, panic_info: &PanicInfo) -> Option { let mut expl = String::new(); + let cause = match panic_info.message() { + Some(m) => format!("{}", m), + None => "Unknown".into(), + }; let payload = panic_info.payload().downcast_ref::<&str>(); if let Some(payload) = payload { @@ -194,7 +199,7 @@ pub fn handle_dump(meta: &Metadata, panic_info: &PanicInfo) -> Option { None => expl.push_str("Panic location unknown.\n"), } - let report = Report::new(&meta.name, &meta.version, Method::Panic, expl); + let report = Report::new(&meta.name, &meta.version, Method::Panic, expl, cause); match report.persist() { Ok(f) => Some(f), diff --git a/src/report.rs b/src/report.rs index e6b1726..4e28e46 100644 --- a/src/report.rs +++ b/src/report.rs @@ -23,6 +23,7 @@ pub struct Report { operating_system: Cow<'static, str>, crate_version: String, explanation: String, + cause: String, method: Method, backtrace: String, } @@ -34,6 +35,7 @@ impl Report { version: &str, method: Method, explanation: String, + cause: String, ) -> Self { let operating_system = if cfg!(windows) { "windows".into() @@ -50,6 +52,7 @@ impl Report { operating_system, method, explanation, + cause, backtrace, } } From 4c74d57b0c5e751b330e86d604f508661d008839 Mon Sep 17 00:00:00 2001 From: Katharina Fey Date: Thu, 20 Sep 2018 14:47:55 +0200 Subject: [PATCH 2/6] Fixing the build on stable through a not-so-nice `cfg` attr --- src/lib.rs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index b7531ea..6de7769 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,4 +1,5 @@ -//! Panic messages for humans +//! Panic message +//! #[cfg(feature = "nightly")]s for humans //! //! Handles panics by calling //! [`std::panic::set_hook`](https://doc.rust-lang.org/std/panic/fn.set_hook.html) @@ -9,7 +10,8 @@ //! pretty great at safety, it's not unheard of to access the wrong index in a //! vector or have an assert fail somewhere. //! -//! When an error eventually occurs, you probably will want to know about it. So +//! When an error eventually occurs, you proba +//! #[cfg(feature = "nightly")]bly will want to know about it. So //! instead of just providing an error message on the command line, we can create a //! call to action for people to submit a report. //! @@ -74,7 +76,8 @@ pub struct Metadata { /// `human-panic` initialisation macro /// -/// You can either call this macro with no arguments `setup_panic!()` or +/// You can either call this macro with no arguments `setup_pan +/// #[cfg(feature = "nightly")]ic!()` or /// with a Metadata struct, if you don't want the error message to display /// the values used in your `Cargo.toml` file. /// @@ -102,6 +105,7 @@ macro_rules! setup_panic { let file_path = handle_dump(&$meta, info); print_msg(file_path, &$meta) + #[cfg(feature = "nightly")] .expect("human-panic: printing error message to console failed"); })); }; @@ -121,12 +125,14 @@ macro_rules! setup_panic { let file_path = handle_dump(&meta, info); print_msg(file_path, &meta) + #[cfg(feature = "nightly")] .expect("human-panic: printing error message to console failed"); })); }; } -/// Utility function that prints a message to our human users + +#[cfg(feature = "nightly")]/// Utility function that prints a message to our human users pub fn print_msg>( file_path: Option

, meta: &Metadata, @@ -180,11 +186,15 @@ pub fn print_msg>( /// Utility function which will handle dumping information to disk pub fn handle_dump(meta: &Metadata, panic_info: &PanicInfo) -> Option { let mut expl = String::new(); + #[cfg(feature = "nightly")] let cause = match panic_info.message() { Some(m) => format!("{}", m), None => "Unknown".into(), }; + #[cfg(not(feature = "nightly"))] + let cause = String::from("Not supported by application"); + let payload = panic_info.payload().downcast_ref::<&str>(); if let Some(payload) = payload { expl.push_str(&format!("Cause: {}. ", &payload)); From ffddaf72b4916145914aafa84e646271364312d9 Mon Sep 17 00:00:00 2001 From: Katharina Fey Date: Thu, 4 Oct 2018 14:04:09 +0200 Subject: [PATCH 3/6] Fixing broken doc comments --- src/lib.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 6de7769..5f0993b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -10,8 +10,7 @@ //! pretty great at safety, it's not unheard of to access the wrong index in a //! vector or have an assert fail somewhere. //! -//! When an error eventually occurs, you proba -//! #[cfg(feature = "nightly")]bly will want to know about it. So +//! When an error eventually occurs, you probably will want to know about it. So //! instead of just providing an error message on the command line, we can create a //! call to action for people to submit a report. //! @@ -76,8 +75,7 @@ pub struct Metadata { /// `human-panic` initialisation macro /// -/// You can either call this macro with no arguments `setup_pan -/// #[cfg(feature = "nightly")]ic!()` or +/// You can either call this macro with no arguments `setup_panic!()` or /// with a Metadata struct, if you don't want the error message to display /// the values used in your `Cargo.toml` file. /// @@ -132,7 +130,8 @@ macro_rules! setup_panic { } -#[cfg(feature = "nightly")]/// Utility function that prints a message to our human users +/// Utility function that prints a message to our human users +#[cfg(feature = "nightly")] pub fn print_msg>( file_path: Option

, meta: &Metadata, From e6dd1a1111e9aa16a1ec9d980ced2cb0a9d3b299 Mon Sep 17 00:00:00 2001 From: Katharina Fey Date: Thu, 4 Oct 2018 14:05:03 +0200 Subject: [PATCH 4/6] Outputting a more helpful message when no original-panic could be determined --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 5f0993b..fbc8fcb 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -192,7 +192,7 @@ pub fn handle_dump(meta: &Metadata, panic_info: &PanicInfo) -> Option { }; #[cfg(not(feature = "nightly"))] - let cause = String::from("Not supported by application"); + let cause = String::from("Error cause could not be determined by the application."); let payload = panic_info.payload().downcast_ref::<&str>(); if let Some(payload) = payload { From fb9ae172d498a0b8794dba4d6e1cbad3d9d8ec29 Mon Sep 17 00:00:00 2001 From: Katharina Fey Date: Thu, 4 Oct 2018 14:30:46 +0200 Subject: [PATCH 5/6] Fixing weird artefacts in random locations --- src/lib.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index fbc8fcb..a970fa8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,5 +1,4 @@ -//! Panic message -//! #[cfg(feature = "nightly")]s for humans +//! Panic messages for humans //! //! Handles panics by calling //! [`std::panic::set_hook`](https://doc.rust-lang.org/std/panic/fn.set_hook.html) @@ -103,7 +102,6 @@ macro_rules! setup_panic { let file_path = handle_dump(&$meta, info); print_msg(file_path, &$meta) - #[cfg(feature = "nightly")] .expect("human-panic: printing error message to console failed"); })); }; @@ -123,7 +121,6 @@ macro_rules! setup_panic { let file_path = handle_dump(&meta, info); print_msg(file_path, &meta) - #[cfg(feature = "nightly")] .expect("human-panic: printing error message to console failed"); })); }; @@ -131,7 +128,6 @@ macro_rules! setup_panic { /// Utility function that prints a message to our human users -#[cfg(feature = "nightly")] pub fn print_msg>( file_path: Option

, meta: &Metadata, From 3143f751415323797c3be115cf35c09165c34122 Mon Sep 17 00:00:00 2001 From: Katharina Fey Date: Thu, 4 Oct 2018 19:26:25 +0200 Subject: [PATCH 6/6] Making rustfmt happy --- src/lib.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index a970fa8..90c7f53 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -126,7 +126,6 @@ macro_rules! setup_panic { }; } - /// Utility function that prints a message to our human users pub fn print_msg>( file_path: Option

, @@ -188,7 +187,8 @@ pub fn handle_dump(meta: &Metadata, panic_info: &PanicInfo) -> Option { }; #[cfg(not(feature = "nightly"))] - let cause = String::from("Error cause could not be determined by the application."); + let cause = + String::from("Error cause could not be determined by the application."); let payload = panic_info.payload().downcast_ref::<&str>(); if let Some(payload) = payload { @@ -204,7 +204,8 @@ pub fn handle_dump(meta: &Metadata, panic_info: &PanicInfo) -> Option { None => expl.push_str("Panic location unknown.\n"), } - let report = Report::new(&meta.name, &meta.version, Method::Panic, expl, cause); + let report = + Report::new(&meta.name, &meta.version, Method::Panic, expl, cause); match report.persist() { Ok(f) => Some(f),