Improve panic reports (#2560)
* Add an `identifying_backtrace` field that only contains symbols in
*our* own codebase, which can be used for better deduplication.
* In the main backtrace, include file and line numbers for all symbols
in our codebase
* Exclude any stack frames within the panic handling/hooking system
itself, so that the top line of the backtrace is where the panic
originated in our codebase.
This should improve our panic deduplication, and also make panic reports
a bit more readable.
example:
```
{
"thread": "main",
"payload": "wtf",
"location_data": {
"file": "crates/zed/src/zed.rs",
"line": 459
},
"backtrace": [
"zed::open_log_file::{{closure}}::{{closure}}::{{closure}}",
" crates/zed/src/zed.rs:459",
"gpui::app::AppContext::spawn_internal::{{closure}}",
" crates/gpui/src/app.rs:2073",
"gpui::executor::any_local_future::{{closure}}",
" crates/gpui/src/executor.rs:1026",
"<core::pin::Pin<P> as core::future::future::Future>::poll",
"<async_task::runnable::spawn_local::Checked<F> as core::future::future::Future>::poll",
"async_task::raw::RawTask<F,T,S>::run",
"async_task::runnable::Runnable::run",
"<gpui::platform::mac::dispatcher::Dispatcher as gpui::platform::Dispatcher>::run_on_main_thread::trampoline",
" crates/gpui/src/platform/mac/dispatcher.rs:40",
"<() as objc::message::MessageArguments>::invoke",
"objc::message::platform::send_unverified",
"objc::message::send_message",
"<gpui::platform::mac::platform::MacForegroundPlatform as gpui::platform::ForegroundPlatform>::run",
" crates/gpui/src/platform/mac/platform.rs:366",
"gpui::app::App::run",
" crates/gpui/src/app.rs:251",
"Zed::main",
" crates/zed/src/main.rs:118",
"core::ops::function::FnOnce::call_once",
"std::sys_common::backtrace::__rust_begin_short_backtrace",
"std::rt::lang_start::{{closure}}",
"core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once",
"std::rt::lang_start"
],
"release_channel": "dev",
"os_name": "macOS",
"os_version": "12.6.1",
"architecture": "aarch64",
"panicked_on": 1685734744050,
"identifying_backtrace": [
"zed::open_log_file::{{closure}}::{{closure}}::{{closure}}",
"gpui::app::AppContext::spawn_internal::{{closure}}",
"gpui::executor::any_local_future::{{closure}}",
"<gpui::platform::mac::dispatcher::Dispatcher as gpui::platform::Dispatcher>::run_on_main_thread::trampoline",
"<gpui::platform::mac::platform::MacForegroundPlatform as gpui::platform::ForegroundPlatform>::run",
"gpui::app::App::run",
"Zed::main"
]
}
```
Release Notes:
N/A
This commit is contained in:
+70
-15
@@ -32,6 +32,7 @@ use std::{
|
||||
ffi::OsStr,
|
||||
fs::OpenOptions,
|
||||
io::Write as _,
|
||||
ops::Not,
|
||||
os::unix::prelude::OsStrExt,
|
||||
panic,
|
||||
path::{Path, PathBuf},
|
||||
@@ -371,13 +372,12 @@ struct Panic {
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
location_data: Option<LocationData>,
|
||||
backtrace: Vec<String>,
|
||||
// TODO
|
||||
// stripped_backtrace: String,
|
||||
release_channel: String,
|
||||
os_name: String,
|
||||
os_version: Option<String>,
|
||||
architecture: String,
|
||||
panicked_on: u128,
|
||||
identifying_backtrace: Option<Vec<String>>,
|
||||
}
|
||||
|
||||
#[derive(Serialize)]
|
||||
@@ -396,18 +396,73 @@ fn init_panic_hook(app: &App) {
|
||||
let app_version = ZED_APP_VERSION
|
||||
.or_else(|| platform.app_version().ok())
|
||||
.map_or("dev".to_string(), |v| v.to_string());
|
||||
let backtrace = Backtrace::new();
|
||||
|
||||
let thread = thread::current();
|
||||
let thread = thread.name().unwrap_or("<unnamed>");
|
||||
|
||||
let payload = match info.payload().downcast_ref::<&'static str>() {
|
||||
Some(s) => *s,
|
||||
None => match info.payload().downcast_ref::<String>() {
|
||||
Some(s) => &**s,
|
||||
None => "Box<Any>",
|
||||
},
|
||||
};
|
||||
let payload = info.payload();
|
||||
let payload = None
|
||||
.or_else(|| payload.downcast_ref::<&str>().map(|s| s.to_string()))
|
||||
.or_else(|| payload.downcast_ref::<String>().map(|s| s.clone()))
|
||||
.unwrap_or_else(|| "Box<Any>".to_string());
|
||||
|
||||
let backtrace = Backtrace::new();
|
||||
let backtrace = backtrace
|
||||
.frames()
|
||||
.iter()
|
||||
.filter_map(|frame| {
|
||||
let symbol = frame.symbols().first()?;
|
||||
let path = symbol.filename()?;
|
||||
Some((path, symbol.lineno(), format!("{:#}", symbol.name()?)))
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
let this_file_path = Path::new(file!());
|
||||
|
||||
// Find the first frame in the backtrace for this panic hook itself. Exclude
|
||||
// that frame and all frames before it.
|
||||
let mut start_frame_ix = 0;
|
||||
let mut codebase_root_path = None;
|
||||
for (ix, (path, _, _)) in backtrace.iter().enumerate() {
|
||||
if path.ends_with(this_file_path) {
|
||||
start_frame_ix = ix + 1;
|
||||
codebase_root_path = path.ancestors().nth(this_file_path.components().count());
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
// Exclude any subsequent frames inside of rust's panic handling system.
|
||||
while let Some((path, _, _)) = backtrace.get(start_frame_ix) {
|
||||
if path.starts_with("/rustc") {
|
||||
start_frame_ix += 1;
|
||||
} else {
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
// Build two backtraces:
|
||||
// * one for display, which includes symbol names for all frames, and files
|
||||
// and line numbers for symbols in this codebase
|
||||
// * one for identification and de-duplication, which only includes symbol
|
||||
// names for symbols in this codebase.
|
||||
let mut display_backtrace = Vec::new();
|
||||
let mut identifying_backtrace = Vec::new();
|
||||
for (path, line, symbol) in &backtrace[start_frame_ix..] {
|
||||
display_backtrace.push(symbol.clone());
|
||||
|
||||
if let Some(codebase_root_path) = &codebase_root_path {
|
||||
if let Ok(suffix) = path.strip_prefix(&codebase_root_path) {
|
||||
identifying_backtrace.push(symbol.clone());
|
||||
|
||||
let display_path = suffix.to_string_lossy();
|
||||
if let Some(line) = line {
|
||||
display_backtrace.push(format!(" {display_path}:{line}"));
|
||||
} else {
|
||||
display_backtrace.push(format!(" {display_path}"));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
let panic_data = Panic {
|
||||
thread: thread.into(),
|
||||
@@ -416,11 +471,6 @@ fn init_panic_hook(app: &App) {
|
||||
file: location.file().into(),
|
||||
line: location.line(),
|
||||
}),
|
||||
backtrace: format!("{:?}", backtrace)
|
||||
.split("\n")
|
||||
.map(|line| line.to_string())
|
||||
.collect(),
|
||||
// modified_backtrace: None,
|
||||
release_channel: RELEASE_CHANNEL.dev_name().into(),
|
||||
os_name: platform.os_name().into(),
|
||||
os_version: platform
|
||||
@@ -432,6 +482,11 @@ fn init_panic_hook(app: &App) {
|
||||
.duration_since(UNIX_EPOCH)
|
||||
.unwrap()
|
||||
.as_millis(),
|
||||
backtrace: display_backtrace,
|
||||
identifying_backtrace: identifying_backtrace
|
||||
.is_empty()
|
||||
.not()
|
||||
.then_some(identifying_backtrace),
|
||||
};
|
||||
|
||||
if let Some(panic_data_json) = serde_json::to_string_pretty(&panic_data).log_err() {
|
||||
|
||||
Reference in New Issue
Block a user