zed 0.91.2

When multiple panics occur at the same time (usually because one thread
panics, and another thread joins it), multiple panic JSON objects can
get written to the same panic file. The resulting file won't be valid
JSON.

This PR addresses that problem via two changes:
* Format panic files as single-line JSON objects
* When a panic file  isn't valid JSON, try taking the first line

In the future, we could try combining all of the backtraces, but for
now, I just want to avoid a problem of not reporting a panic at all.

Release Notes:

- Fixed a problem with Zed's internal crash reporting.

Cargo.lock

@@ -8795,7 +8795,7 @@ dependencies = [
 
 [[package]]
 name = "zed"
-version = "0.91.0"
+version = "0.91.2"
 dependencies = [
  "activity_indicator",
  "ai",

crates/zed/Cargo.toml

@@ -3,7 +3,7 @@ authors = ["Nathan Sobo <nathansobo@gmail.com>"]
 description = "The fast, collaborative code editor."
 edition = "2021"
 name = "zed"
-version = "0.91.0"
+version = "0.91.2"
 publish = false
 
 [lib]

crates/zed/RELEASE_CHANNEL

@@ -1 +1 @@
-dev
+preview

crates/zed/resources/zed.entitlements

@@ -0,0 +1,28 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
+<plist version="1.0">
+<dict>
+	<key>com.apple.security.automation.apple-events</key>
+	<true/>
+	<key>com.apple.security.cs.allow-jit</key>
+	<true/>
+	<key>com.apple.security.device.audio-input</key>
+	<true/>
+	<key>com.apple.security.device.camera</key>
+	<true/>
+	<key>com.apple.security.personal-information.addressbook</key>
+	<true/>
+	<key>com.apple.security.personal-information.calendars</key>
+	<true/>
+	<key>com.apple.security.personal-information.location</key>
+	<true/>
+	<key>com.apple.security.personal-information.photos-library</key>
+	<true/>
+	<key>com.apple.security.cs.allow-dyld-environment-variables</key>
+	<true/>
+	<key>com.apple.security.cs.allow-unsigned-executable-memory</key>
+	<true/>
+	<key>com.apple.security.cs.disable-library-validation</key>
+	<true/>
+</dict>
+</plist>

crates/zed/src/main.rs

@@ -31,7 +31,6 @@ use std::{
     ffi::OsStr,
     fs::OpenOptions,
     io::Write as _,
-    ops::Not,
     os::unix::prelude::OsStrExt,
     panic,
     path::{Path, PathBuf},
@@ -373,7 +372,6 @@ struct Panic {
     os_version: Option<String>,
     architecture: String,
     panicked_on: u128,
-    identifying_backtrace: Option<Vec<String>>,
 }
 
 #[derive(Serialize)]
@@ -401,61 +399,18 @@ fn init_panic_hook(app: &App) {
             .unwrap_or_else(|| "Box<Any>".to_string());
 
         let backtrace = Backtrace::new();
-        let backtrace = backtrace
+        let mut backtrace = backtrace
             .frames()
             .iter()
-            .filter_map(|frame| {
-                let symbol = frame.symbols().first()?;
-                let path = symbol.filename()?;
-                Some((path, symbol.lineno(), format!("{:#}", symbol.name()?)))
-            })
+            .filter_map(|frame| Some(format!("{:#}", frame.symbols().first()?.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}"));
-                    }
-                }
-            }
+        // Strip out leading stack frames for rust panic-handling.
+        if let Some(ix) = backtrace
+            .iter()
+            .position(|name| name == "rust_begin_unwind")
+        {
+            backtrace.drain(0..=ix);
         }
 
         let panic_data = Panic {
@@ -477,29 +432,27 @@ 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),
+            backtrace,
         };
 
-        if let Some(panic_data_json) = serde_json::to_string_pretty(&panic_data).log_err() {
-            if is_pty {
+        if is_pty {
+            if let Some(panic_data_json) = serde_json::to_string_pretty(&panic_data).log_err() {
                 eprintln!("{}", panic_data_json);
                 return;
             }
-
-            let timestamp = chrono::Utc::now().format("%Y_%m_%d %H_%M_%S").to_string();
-            let panic_file_path = paths::LOGS_DIR.join(format!("zed-{}.panic", timestamp));
-            let panic_file = std::fs::OpenOptions::new()
-                .append(true)
-                .create(true)
-                .open(&panic_file_path)
-                .log_err();
-            if let Some(mut panic_file) = panic_file {
-                write!(&mut panic_file, "{}", panic_data_json).log_err();
-                panic_file.flush().log_err();
+        } else {
+            if let Some(panic_data_json) = serde_json::to_string(&panic_data).log_err() {
+                let timestamp = chrono::Utc::now().format("%Y_%m_%d %H_%M_%S").to_string();
+                let panic_file_path = paths::LOGS_DIR.join(format!("zed-{}.panic", timestamp));
+                let panic_file = std::fs::OpenOptions::new()
+                    .append(true)
+                    .create(true)
+                    .open(&panic_file_path)
+                    .log_err();
+                if let Some(mut panic_file) = panic_file {
+                    writeln!(&mut panic_file, "{}", panic_data_json).log_err();
+                    panic_file.flush().log_err();
+                }
             }
         }
     }));
@@ -531,23 +484,45 @@ fn upload_previous_panics(http: Arc<dyn HttpClient>, cx: &mut AppContext) {
                     }
 
                     if telemetry_settings.diagnostics {
-                        let panic_data_text = smol::fs::read_to_string(&child_path)
+                        let panic_file_content = smol::fs::read_to_string(&child_path)
                             .await
                             .context("error reading panic file")?;
 
-                        let body = serde_json::to_string(&PanicRequest {
-                            panic: serde_json::from_str(&panic_data_text)?,
-                            token: ZED_SECRET_CLIENT_TOKEN.into(),
-                        })
-                        .unwrap();
+                        let panic = serde_json::from_str(&panic_file_content)
+                            .ok()
+                            .or_else(|| {
+                                panic_file_content
+                                    .lines()
+                                    .next()
+                                    .and_then(|line| serde_json::from_str(line).ok())
+                            })
+                            .unwrap_or_else(|| {
+                                log::error!(
+                                    "failed to deserialize panic file {:?}",
+                                    panic_file_content
+                                );
+                                None
+                            });
 
-                        let request = Request::post(&panic_report_url)
-                            .redirect_policy(isahc::config::RedirectPolicy::Follow)
-                            .header("Content-Type", "application/json")
-                            .body(body.into())?;
-                        let response = http.send(request).await.context("error sending panic")?;
-                        if !response.status().is_success() {
-                            log::error!("Error uploading panic to server: {}", response.status());
+                        if let Some(panic) = panic {
+                            let body = serde_json::to_string(&PanicRequest {
+                                panic,
+                                token: ZED_SECRET_CLIENT_TOKEN.into(),
+                            })
+                            .unwrap();
+
+                            let request = Request::post(&panic_report_url)
+                                .redirect_policy(isahc::config::RedirectPolicy::Follow)
+                                .header("Content-Type", "application/json")
+                                .body(body.into())?;
+                            let response =
+                                http.send(request).await.context("error sending panic")?;
+                            if !response.status().is_success() {
+                                log::error!(
+                                    "Error uploading panic to server: {}",
+                                    response.status()
+                                );
+                            }
                         }
                     }
 

script/bundle

@@ -81,12 +81,12 @@ if [[ -n $MACOS_CERTIFICATE && -n $MACOS_CERTIFICATE_PASSWORD && -n $APPLE_NOTAR
     security import /tmp/zed-certificate.p12 -k zed.keychain -P "$MACOS_CERTIFICATE_PASSWORD" -T /usr/bin/codesign
     rm /tmp/zed-certificate.p12
     security set-key-partition-list -S apple-tool:,apple:,codesign: -s -k "$MACOS_CERTIFICATE_PASSWORD" zed.keychain
-    /usr/bin/codesign --force --deep --timestamp --options runtime --sign "Zed Industries, Inc." "${app_path}" -v
+    /usr/bin/codesign --force --deep --timestamp --options runtime --entitlements crates/zed/resources/zed.entitlements --sign "Zed Industries, Inc." "${app_path}" -v
     security default-keychain -s login.keychain
 else
     echo "One or more of the following variables are missing: MACOS_CERTIFICATE, MACOS_CERTIFICATE_PASSWORD, APPLE_NOTARIZATION_USERNAME, APPLE_NOTARIZATION_PASSWORD"
     echo "Performing an ad-hoc signature, but this bundle should not be distributed"
-    codesign --force --deep --sign - "${app_path}" -v
+    codesign --force --deep --entitlements crates/zed/resources/zed.entitlements --sign - "${app_path}" -v
 fi
 
 if [ "$target_dir" = "debug" ]; then