Skip to content

feat(cli): Symbolicate remote apple crash reports #1923

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jjbayer merged 1 commit into from
Apr 13, 2026
+36 −30
Merged

feat(cli): Symbolicate remote apple crash reports #1923

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 36 additions & 30 deletions crates/symbolicli/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,51 +259,52 @@ enum Payload {
impl Payload {
fn parse<P: AsRef<Path> + fmt::Debug>(path: &P) -> Result<Option<Self>> {
match File::open(path) {
Ok(mut file) => {
let mut magic = [0; 4];
file.read_exact(&mut magic)?;
file.rewind()?;

if &magic == b"MDMP" || &magic == b"PMDM" {
Ok(Some(Payload::Minidump(file)))
} else if &magic == b"Inci" {
Ok(Some(Payload::AppleCrashReport(file)))
} else {
let event =
serde_json::from_reader(file).context("failed to parse event json file")?;
Ok(Some(Payload::Event(event)))
}
}
Ok(file) => Self::parse_file(file).map(Option::Some),
Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(None),
Err(e) => Err(e).context(format!("Could not open event file at {path:?}")),
}
}

fn parse_file(mut file: File) -> Result<Self> {
let mut magic = [0; 4];
file.read_exact(&mut magic)?;
file.rewind()?;

if &magic == b"MDMP" || &magic == b"PMDM" {
Ok(Payload::Minidump(file))
} else if &magic == b"Inci" || &magic == b"----" {
Copy link
Copy Markdown

@sentry sentry bot Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The code classifies files starting with b"----" as Apple crash reports, but the downstream parser likely doesn't support this format, which will cause a runtime crash.
Severity: HIGH

Suggested Fix

Remove the || &magic == b"----" check. Alternatively, if this format is valid, add test cases with fixtures that start with b"----" to verify the parser can handle them. Also, add documentation explaining what this format represents.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: crates/symbolicli/src/main.rs#L275

Potential issue: The code adds a check for `b"----"` to identify Apple crash reports.
When a file with these magic bytes is encountered, it's passed to
`AppleCrashReport::from_reader()`. However, there is no evidence that the
`apple_crash_report_parser` crate supports this format. This will likely cause the
parser to fail, resulting in an unhandled error that crashes the `symbolicli` tool with
the message "failed to parse apple crash report". This can be triggered in online mode
if a Sentry event has an `event.applecrashreport` attachment starting with `----`.

Did we get this right? 👍 / 👎 to inform future reviews.

Ok(Payload::AppleCrashReport(file))
} else {
let event = serde_json::from_reader(file).context("failed to parse event json file")?;
Ok(Payload::Event(event))
}
}

async fn get_remote(client: &reqwest::Client, key: EventKey<'_>) -> Result<Self> {
tracing::info!("trying to resolve event remotely");
let event = remote::download_event(client, key).await?;
tracing::info!("event json file downloaded");

match remote::get_attached_minidump(client, key).await? {
Some(minidump_url) => {
tracing::info!("minidump attachment found");
let minidump_path = remote::download_minidump(client, minidump_url).await?;
tracing::info!(path = ?minidump_path, "minidump file downloaded");
match remote::get_attached_crash(client, key).await? {
Some(url) => {
tracing::info!("crash attachment found");
let file = remote::download_crash_file(client, url).await?;
tracing::info!(path = ?file, "crash file downloaded");

Ok(Payload::Minidump(minidump_path))
Payload::parse_file(file)
}

None => {
tracing::info!("no minidump attachment found");
tracing::info!("no crash attachment found");
Ok(Self::Event(event))
}
}
}
}

mod remote {
use std::fs::File;
use std::io::Write;
use std::{fs::File, io::Seek};

use anyhow::{Context, Result, bail};
use reqwest::{StatusCode, Url};
Expand All @@ -325,7 +326,7 @@ mod remote {
id: String,
}

pub async fn get_attached_minidump(
pub async fn get_attached_crash(
client: &reqwest::Client,
key: EventKey<'_>,
) -> Result<Option<Url>> {
Expand Down Expand Up @@ -366,7 +367,10 @@ mod remote {

let Some(minidump_id) = attachments
.iter()
.find(|attachment| attachment.r#type == "event.minidump")
.find(|attachment| {
attachment.r#type == "event.minidump"
|| attachment.r#type == "event.applecrashreport"
})
.map(|attachment| &attachment.id)
else {
return Ok(None);
Expand All @@ -378,16 +382,16 @@ mod remote {
Ok(Some(download_url))
}

pub async fn download_minidump(client: &reqwest::Client, download_url: Url) -> Result<File> {
tracing::info!(url = %download_url, "downloading minidump file");
pub async fn download_crash_file(client: &reqwest::Client, download_url: Url) -> Result<File> {
tracing::info!(url = %download_url, "downloading crash file");

let response = client
.get(download_url)
.send()
.await
.context("Failed to send request")?;

let minidump = if response.status().is_success() {
let crash_file = if response.status().is_success() {
response
.bytes()
.await
Expand All @@ -404,8 +408,10 @@ mod remote {

let mut temp_file = tempfile::tempfile().unwrap();
temp_file
.write_all(&minidump)
.context("Failed to write minidump to disk")?;
.write_all(&crash_file)
.context("Failed to write crash file to disk")?;

temp_file.rewind()?;

Ok(temp_file)
}
Expand Down
Loading