Skip to content

Commit

Permalink
feat: Improve custom panic hook (#2355)
Browse files Browse the repository at this point in the history
This change improves Sentry CLI's panic hook to make panic messages more user-friendly.

The panic message generated by the hook emphasizes that panics are internal errors in the Sentry CLI, and the message directs users to our bug report issue form. The message also always includes a stack backtrace, regardless of whether `RUST_BACKTRACE` is set.

The new panic message looks like the following:

```
🔥 Internal Error in Sentry CLI 🔥

Uh-oh! 😬 Sentry CLI has just crashed due to an internal error. Please open a bug report issue at https://github.com/getsentry/sentry-cli/issues/new?template=BUG_REPORT.yml. 🐞

🔬 Technical Details 🔬

thread 'main' panicked at src/commands/sourcemaps/upload.rs:422:5:
test error

Stack Backtrace:
   0: std::backtrace_rs::backtrace::libunwind::trace
             at /rustc/9fc6b43126469e3858e2fe86cafb4f0fd5068869/library/std/src/../../backtrace/src/backtrace/libunwind.rs:116:5
   1: std::backtrace_rs::backtrace::trace_unsynchronized
             at /rustc/9fc6b43126469e3858e2fe86cafb4f0fd5068869/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
[... truncated ...]
```

This change also removes the dependency on the `backtrace` crate, and renames the function which sets the panic hook from `init_backtrace` to `set_panic_hook`.
  • Loading branch information
szokeasaurusrex authored Jan 23, 2025
1 parent 8318213 commit 4278c53
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 35 deletions.
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ rust-version = "1.80"
anylog = "0.6.3"
anyhow = { version = "1.0.69", features = ["backtrace"] }
backoff = "0.4.0"
backtrace = "0.3.67"
brotli2 = "0.3.2"
bytecount = "0.6.3"
chrono = { version = "0.4.31", features = ["serde"] }
Expand Down
4 changes: 2 additions & 2 deletions src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use crate::constants::{ARCH, PLATFORM, VERSION};
use crate::utils::auth_token::{redact_token_from_string, AuthToken};
use crate::utils::logging::set_quiet_mode;
use crate::utils::logging::Logger;
use crate::utils::system::{init_backtrace, load_dotenv, print_error, QuietExit};
use crate::utils::system::{load_dotenv, print_error, set_panic_hook, QuietExit};
use crate::utils::update::run_sentrycli_update_nagger;
use crate::utils::value_parsers::auth_token_parser;

Expand Down Expand Up @@ -331,7 +331,7 @@ pub fn execute() -> Result<()> {
}

fn setup() {
init_backtrace();
set_panic_hook();

// Store the result of loading the dotenv file. We must load the dotenv file
// before setting the log level, as the log level can be set in the dotenv
Expand Down
101 changes: 70 additions & 31 deletions src/utils/system.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
use std::any::Any;
use std::backtrace::Backtrace;
use std::borrow::Cow;
use std::env;
use std::panic::{self, Location, PanicHookInfo};
#[cfg(target_os = "macos")]
use std::process;
use std::{env, thread};

use anyhow::{Error, Result};
use console::style;
Expand Down Expand Up @@ -91,36 +94,11 @@ pub fn print_error(err: &Error) {
}
}

/// Initializes the backtrace support
pub fn init_backtrace() {
std::panic::set_hook(Box::new(|info| {
let backtrace = backtrace::Backtrace::new();

let thread = std::thread::current();
let thread = thread.name().unwrap_or("unnamed");

let msg = match info.payload().downcast_ref::<&'static str>() {
Some(s) => *s,
None => match info.payload().downcast_ref::<String>() {
Some(s) => &**s,
None => "Box<Any>",
},
};

match info.location() {
Some(location) => {
eprintln!(
"thread '{}' panicked at '{}': {}:{}\n\n{:?}",
thread,
msg,
location.file(),
location.line(),
backtrace
);
}
None => eprintln!("thread '{thread}' panicked at '{msg}'{backtrace:?}"),
}
}));
/// Sets the panic hook to use our custom panic hook.
///
/// See [panic_hook] for more details on how the custom panic hook behaves.
pub fn set_panic_hook() {
panic::set_hook(Box::new(panic_hook));
}

/// Indicates that sentry-cli should quit without printing anything.
Expand Down Expand Up @@ -154,3 +132,64 @@ pub fn load_dotenv() -> DotenvResult<()> {
|_| Ok(()),
)
}

/// Custom panic hook for Sentry CLI
///
/// This custom panic hook captures a more user-friendly panic message, which indicates
/// that the panic is an internal error in the Sentry CLI, and which directs users to
/// open a bug report issue when encountering a panic.
///
/// The panic captures and prints a backtrace, regardless of whether the RUST_BACKTRACE
/// environment variable is set.
fn panic_hook(info: &PanicHookInfo) {
const PANIC_MESSAGE: &str = "Uh-oh! 😬 Sentry CLI has just crashed due to an internal error. \
Please open a bug report issue at https://github.com/getsentry/sentry-cli/issues/new?template=BUG_REPORT.yml. 🐞";

eprintln!(
"{}\n\n{}\n\n{}",
console::style("🔥 Internal Error in Sentry CLI 🔥")
.bold()
.red(),
PANIC_MESSAGE,
display_technical_details(info, &Backtrace::force_capture())
);
}

/// Generates the "technical details" section of the panic message
fn display_technical_details(info: &PanicHookInfo, backtrace: &Backtrace) -> String {
format!(
"🔬 Technical Details 🔬\n\n{} panicked at {}:\n{}\n\nStack Backtrace:\n{}",
display_thread_details(),
display_panic_location(info.location()),
display_panic_payload(info.payload()),
backtrace
)
}

/// Formats the current thread name for display in the panic message
fn display_thread_details() -> String {
match thread::current().name() {
Some(name) => format!("thread '{}'", name),
None => "unknown thread".into(),
}
}

/// Formats the panic location for display in the panic message
fn display_panic_location(location: Option<&Location>) -> String {
if let Some(location) = location {
location.to_string()
} else {
"unknown location".into()
}
}

/// Formats the panic payload for display in the panic message
fn display_panic_payload(payload: &dyn Any) -> &str {
if let Some(&payload) = payload.downcast_ref() {
payload
} else if let Some(payload) = payload.downcast_ref::<String>() {
payload.as_str()
} else {
""
}
}

0 comments on commit 4278c53

Please sign in to comment.