chore: better sentry error reporting regarding stacktraces and errors

.github/workflows/oneclient_nightly.yml

@@ -95,6 +95,27 @@ jobs:
             echo "dir=target/release" >> "$GITHUB_OUTPUT"
           fi
 
+      # Upload debug info (dSYM / .dwp / .pdb produced by the release profile's
+      # split-debuginfo) to Sentry so nightly crash reports get real file/function/
+      # line stack traces instead of raw addresses. Runs on every main push, so
+      # each unique build's symbols land in Sentry (matched by debug-id). Skips
+      # silently unless SENTRY_AUTH_TOKEN secret + SENTRY_ORG/SENTRY_PROJECT vars
+      # are configured.
+      - name: Upload debug symbols to Sentry
+        shell: bash
+        env:
+          SENTRY_AUTH_TOKEN: ${{ secrets.SENTRY_AUTH_TOKEN }}
+          SENTRY_ORG: ${{ vars.SENTRY_ORG }}
+          SENTRY_PROJECT: ${{ vars.SENTRY_PROJECT }}
+          OUT_DIR: ${{ steps.outdir.outputs.dir }}
+        run: |
+          if [ -z "${SENTRY_AUTH_TOKEN:-}" ] || [ -z "${SENTRY_ORG:-}" ] || [ -z "${SENTRY_PROJECT:-}" ]; then
+            echo "Sentry credentials not configured (need SENTRY_AUTH_TOKEN secret + SENTRY_ORG/SENTRY_PROJECT vars); skipping debug-symbol upload."
+            exit 0
+          fi
+          curl -sL https://sentry.io/get-cli/ | bash
+          sentry-cli debug-files upload --include-sources "$OUT_DIR"
+
       - name: Collect installers
         id: collect
         shell: bash

.github/workflows/oneclient_release.yml

@@ -167,6 +167,25 @@ jobs:
             echo "dir=target/release" >> "$GITHUB_OUTPUT"
           fi
 
+      # Upload debug info (dSYM / .dwp / .pdb produced by the release profile's
+      # split-debuginfo) to Sentry so crash reports get real file/function/line
+      # stack traces instead of raw addresses. Skips silently unless the repo has
+      # a SENTRY_AUTH_TOKEN secret plus SENTRY_ORG / SENTRY_PROJECT variables.
+      - name: Upload debug symbols to Sentry
+        shell: bash
+        env:
+          SENTRY_AUTH_TOKEN: ${{ secrets.SENTRY_AUTH_TOKEN }}
+          SENTRY_ORG: ${{ vars.SENTRY_ORG }}
+          SENTRY_PROJECT: ${{ vars.SENTRY_PROJECT }}
+          OUT_DIR: ${{ steps.outdir.outputs.dir }}
+        run: |
+          if [ -z "${SENTRY_AUTH_TOKEN:-}" ] || [ -z "${SENTRY_ORG:-}" ] || [ -z "${SENTRY_PROJECT:-}" ]; then
+            echo "Sentry credentials not configured (need SENTRY_AUTH_TOKEN secret + SENTRY_ORG/SENTRY_PROJECT vars); skipping debug-symbol upload."
+            exit 0
+          fi
+          curl -sL https://sentry.io/get-cli/ | bash
+          sentry-cli debug-files upload --include-sources "$OUT_DIR"
+
       - name: (Windows) Locate installer
         if: matrix.platform == 'windows-latest'
         id: win_installer

Cargo.toml

@@ -137,9 +137,16 @@ freya = { git = "https://github.com/marc2332/freya.git", branch = "fix/blur" }
 opt-level = 3
 lto = "fat"
 codegen-units = 1
+# Keep the *shipped* binary stripped and small, but still emit debug info into
+# separate files (macOS: .dSYM, Linux: .dwp, Windows: .pdb) so Sentry can
+# symbolicate crash reports. CI uploads those files with `sentry-cli debug-files
+# upload`; they are never shipped to users. Without this, release stack traces
+# are just raw addresses. Use "full" instead of "line-tables-only" if you also
+# want inlined-frame / variable info (larger debug files, slower builds).
 strip = true
 panic = "abort"
-debug = false
+debug = "line-tables-only"
+split-debuginfo = "packed"
 
 [profile.release-profiling]
 inherits = "release"

packages/oneclient_app/src/utils.rs

@@ -1,6 +1,5 @@
 use std::cmp::Ordering;
 use std::collections::BTreeMap;
-use std::sync::OnceLock;
 
 use chrono::{Datelike, NaiveDate};
 use oneclient_core::clusters::Cluster;

packages/oneclient_app/src/view/onboarding/downloading/mod.rs

@@ -8,7 +8,8 @@ use oneclient_core::notification::{
     GroupedProgressEvent, GroupedProgressSession, Notification, NotificationService,
 };
 use oneclient_core::{
-    BundleArchive, BundleFile, Cluster, ImportTarget, MigrationSource, VersionMetadata,
+    BundleArchive, BundleFile, Cluster, ImportTarget, MigrationSource, SentryExclusion,
+    VersionMetadata,
 };
 use oneclient_db::models::OverrideType;
 use tokio::sync::mpsc;
@@ -718,13 +719,28 @@ fn run_install_batch(plans: Vec<ClusterPlan>, predownload: bool, handles: Instal
             }));
 
             let fetch_now = predownload && plan.predownload;
-            if let Err(reason) = install_one(&plan, fetch_now, &notifier, &ui_tx).await {
+            if let Err(err) = install_one(&plan, fetch_now, &notifier, &ui_tx).await {
-                tracing::error!(
+                // Environmental / expected failures (out of disk, offline, no Java
-                    cluster_id = plan.cluster_id,
+                // build for this version, ...) are surfaced to the user but aren't
-                    mc_version = %plan.mc_version,
+                // crashes, so log them at warn! (breadcrumb) instead of error!
-                    "onboarding install failed: {reason}"
+                // (a reported Sentry event). See LauncherError::is_sentry_excluded.
-                );
+                if err.is_sentry_excluded() {
-                failed.push(InstallFailure { plan, reason });
+                    tracing::warn!(
+                        cluster_id = plan.cluster_id,
+                        mc_version = %plan.mc_version,
+                        "onboarding install failed (expected): {err}"
+                    );
+                } else {
+                    tracing::error!(
+                        cluster_id = plan.cluster_id,
+                        mc_version = %plan.mc_version,
+                        "onboarding install failed: {err}"
+                    );
+                }
+                failed.push(InstallFailure {
+                    plan,
+                    reason: err.to_string(),
+                });
             }
             let _ = ui_tx.send(InstallUiEvent::Progress(index + 1, total));
         }
@@ -739,8 +755,8 @@ async fn install_one(
     predownload: bool,
     notifier: &NotificationService,
     ui_tx: &mpsc::UnboundedSender<InstallUiEvent>,
-) -> Result<(), String> {
+) -> oneclient_core::LauncherResult<()> {
-    let state = oneclient_core::LauncherState::get().map_err(|err| err.to_string())?;
+    let state = oneclient_core::LauncherState::get()?;
 
     if !plan.overrides.is_empty() {
         let _ = ui_tx.send(InstallUiEvent::Activity(
@@ -748,8 +764,7 @@ async fn install_one(
         ));
     }
     oneclient_core::set_bundle_package_overrides(plan.cluster_id, &plan.overrides, &state.services)
-        .await
+        .await?;
-        .map_err(|err| err.to_string())?;
 
     if !predownload {
         return Ok(());
@@ -791,8 +806,8 @@ async fn install_one(
 
     session.finish();
 
-    prepared.map_err(|err| err.to_string())?;
+    prepared?;
-    bundles_result.map_err(|err| err.to_string())?;
+    bundles_result?;
     Ok(())
 }
 

packages/oneclient_core/src/auth/msa.rs

@@ -787,7 +787,7 @@ async fn parse_json_response<T: for<'de> Deserialize<'de>>(
         .map_err(|source| MinecraftAuthError::RequestError { step, source })?;
 
     if !status.is_success() {
-        tracing::error!("[auth] step={step:?} status={status} body={text}");
+        tracing::error!(sentry = false, ?step, %status, body = %text, "MSA endpoint returned a non-success status");
     }
 
     serde_json::from_str(&text).map_err(|source| {

packages/oneclient_core/src/error.rs

@@ -6,7 +6,7 @@ pub enum LauncherError {
     JsonError(#[from] serde_json::Error),
 
     #[error("Database execution failed: {0}")]
-    DatabaseError(#[from] sqlx::Error),
+    SqlError(#[from] sqlx::Error),
 
     #[error(transparent)]
     IoError(#[from] polyio::IOError),
@@ -81,4 +81,75 @@ impl LauncherError {
     }
 }
 
+pub trait SentryExclusion {
+    /// Whether this error is expected/environmental noise that should be kept out
+    /// of Sentry rather than reported as a crash.
+    fn is_sentry_excluded(&self) -> bool {
+        false
+    }
+}
+
+impl SentryExclusion for LauncherError {
+    fn is_sentry_excluded(&self) -> bool {
+        match self {
+            LauncherError::StdIoError(e) => e.is_sentry_excluded(),
+            LauncherError::IoError(e) => e.is_sentry_excluded(),
+            LauncherError::RequestError(e) => e.is_sentry_excluded(),
+            LauncherError::JavaError(e) => e.is_sentry_excluded(),
+            _ => false,
+        }
+    }
+}
+
+impl SentryExclusion for std::io::Error {
+    fn is_sentry_excluded(&self) -> bool {
+        use std::io::ErrorKind;
+
+        // Out of disk space. `ErrorKind::StorageFull` is still unstable, so match
+        // the raw OS codes instead. Codes are platform-gated so a Unix errno can't
+        // collide with a Windows code (112 is the unrelated EHOSTDOWN on Linux).
+        if let Some(code) = self.raw_os_error() {
+            #[cfg(unix)]
+            if code == 28 {
+                // ENOSPC
+                return true;
+            }
+            #[cfg(windows)]
+            if code == 112 || code == 39 {
+                // ERROR_DISK_FULL / ERROR_HANDLE_DISK_FULL
+                return true;
+            }
+            let _ = code;
+        }
+
+        // Lost/refused network connections during downloads.
+        matches!(
+            self.kind(),
+            ErrorKind::ConnectionRefused
+                | ErrorKind::ConnectionReset
+                | ErrorKind::ConnectionAborted
+                | ErrorKind::NotConnected
+                | ErrorKind::TimedOut
+        )
+    }
+}
+
+impl SentryExclusion for reqwest::Error {
+    fn is_sentry_excluded(&self) -> bool {
+        // Connectivity problems (offline, connection refused, timed out) are the
+        // user's network, not a launcher bug.
+        self.is_timeout() || self.is_connect()
+    }
+}
+
+impl SentryExclusion for polyio::IOError {
+    fn is_sentry_excluded(&self) -> bool {
+        match self {
+            polyio::IOError::IOError(source)
+            | polyio::IOError::PathIOError { source, .. } => source.is_sentry_excluded(),
+            _ => false,
+        }
+    }
+}
+
 pub type LauncherResult<T> = Result<T, LauncherError>;

packages/oneclient_core/src/http/mod.rs

@@ -45,3 +45,15 @@ pub enum RequestError {
     #[error("Invalid HTTP header value: {0}")]
     InvalidHeaderValue(#[from] reqwest::header::InvalidHeaderValue),
 }
+
+impl crate::error::SentryExclusion for RequestError {
+    fn is_sentry_excluded(&self) -> bool {
+        // Only connectivity / disk failures are noise; malformed requests, bad
+        // URLs, and unexpected response shapes are bugs worth reporting.
+        match self {
+            RequestError::ReqwestError(source) => source.is_sentry_excluded(),
+            RequestError::IOError(source) => source.is_sentry_excluded(),
+            _ => false,
+        }
+    }
+}

packages/oneclient_core/src/java/error.rs

@@ -41,3 +41,18 @@ pub enum JavaError {
 	#[error("failed to extract archive '{archive}'")]
 	ArchiveExtractFailed { archive: String },
 }
+
+impl crate::error::SentryExclusion for JavaError {
+	fn is_sentry_excluded(&self) -> bool {
+		match self {
+			// Expected outcomes: no build exists for the requested version, or the
+			// user cancelled setup.
+			JavaError::PackageNotFound { .. } | JavaError::Cancelled => true,
+			// Environmental IO (e.g. out of disk while extracting the runtime).
+			JavaError::RuntimeCheckError { source, .. }
+			| JavaError::ArchiveExtract { source, .. } => source.is_sentry_excluded(),
+			JavaError::PolyIOError(source) => source.is_sentry_excluded(),
+			_ => false,
+		}
+	}
+}

packages/oneclient_core/src/lib.rs

@@ -54,7 +54,7 @@ pub use clusters::{
     Cluster, ClusterError, ClusterManager, ClusterStage, ClusterUpdate, CreateClusterOptions,
     ensure_from_bundles, ensure_from_versions, estimate_cluster_download,
 };
-pub use error::{LauncherError, LauncherResult};
+pub use error::{LauncherError, LauncherResult, SentryExclusion};
 pub use game::{GameError, LaunchedGame, get_loader_versions, launch_cluster};
 pub use images::ImageCacheStore;
 pub use logs::{

packages/oneclient_core/src/reporting.rs

@@ -1,76 +1,122 @@
 use std::panic;
+use std::sync::Arc;
 use std::time::Duration;
 
+use sentry::protocol::{Context, Event};
 use sentry::{ClientInitGuard, ClientOptions};
 
 use crate::constants::SENTRY_DSN;
 
 const PANIC_FLUSH_TIMEOUT: Duration = Duration::from_secs(2);
+const TRACING_FIELDS_CONTEXT: &str = "Rust Tracing Fields";
 
 const ENVIRONMENT: &str = if cfg!(debug_assertions) {
-	"development"
+    "development"
 } else {
-	"production"
+    "production"
 };
 
+/// `before_send` hook: drop any event a log call explicitly opted out of by
+/// setting the `sentry` field to `false`, e.g.
+///
+/// ```ignore
+/// tracing::error!(sentry = false, "handled MSA failure, don't report as a crash");
+/// ```
+fn drop_opted_out_events(event: Event<'static>) -> Option<Event<'static>> {
+    let opted_out = matches!(
+        event.contexts.get(TRACING_FIELDS_CONTEXT),
+        Some(Context::Other(fields))
+            if fields.get("sentry").and_then(serde_json::Value::as_bool) == Some(false)
+    );
+
+    if opted_out { None } else { Some(event) }
+}
+
 pub fn init(enabled: bool) -> Option<ClientInitGuard> {
-	if !enabled {
+    if !enabled {
-		tracing::debug!("crash reporting disabled by settings");
+        tracing::debug!("crash reporting disabled by settings");
-		return None;
+        return None;
-	}
+    }
 
-	if cfg!(debug_assertions) && option_env!("ONECLIENT_SENTRY_DSN").is_none() {
+    if cfg!(debug_assertions) && option_env!("ONECLIENT_SENTRY_DSN").is_none() {
-		tracing::debug!("crash reporting skipped: debug build without an explicit DSN");
+        tracing::debug!("crash reporting skipped: debug build without an explicit DSN");
-		return None;
+        return None;
-	}
+    }
 
-	let guard = sentry::init((
+    let guard = sentry::init((
-		SENTRY_DSN,
+        SENTRY_DSN,
-		ClientOptions {
+        ClientOptions {
-			release: Some(format!("oneclient@{}", env!("CARGO_PKG_VERSION")).into()),
+            release: Some(format!("oneclient@{}", env!("CARGO_PKG_VERSION")).into()),
-			environment: Some(ENVIRONMENT.into()),
+            environment: Some(ENVIRONMENT.into()),
-			attach_stacktrace: true,
+            attach_stacktrace: true,
-			send_default_pii: false,
+            send_default_pii: false,
-			..Default::default()
+            before_send: Some(Arc::new(drop_opted_out_events)),
-		},
+            ..Default::default()
-	));
+        },
+    ));
 
-	if !guard.is_enabled() {
+    if !guard.is_enabled() {
-		tracing::warn!("sentry client failed to start; crash reports will not be sent");
+        tracing::warn!("sentry client failed to start; crash reports will not be sent");
-		return None;
+        return None;
-	}
+    }
 
-	install_panic_flush();
+    install_panic_flush();
 
-	tracing::info!(environment = ENVIRONMENT, "crash reporting enabled");
+    tracing::info!(environment = ENVIRONMENT, "crash reporting enabled");
-	Some(guard)
+    Some(guard)
 }
 
 fn install_panic_flush() {
-	let capture = panic::take_hook();
+    let capture = panic::take_hook();
 
-	panic::set_hook(Box::new(move |info| {
+    panic::set_hook(Box::new(move |info| {
-		capture(info);
+        capture(info);
 
-		if let Some(client) = sentry::Hub::current().client() {
+        if let Some(client) = sentry::Hub::current().client() {
-			client.flush(Some(PANIC_FLUSH_TIMEOUT));
+            client.flush(Some(PANIC_FLUSH_TIMEOUT));
-		}
+        }
-	}));
+    }));
 }
 
 #[cfg(test)]
 mod tests {
-	use super::*;
+    use super::*;
 
-	#[test]
+    #[test]
-	fn bundled_dsn_parses() {
+    fn bundled_dsn_parses() {
-		SENTRY_DSN
+        SENTRY_DSN
-			.parse::<sentry::types::Dsn>()
+            .parse::<sentry::types::Dsn>()
-			.expect("bundled sentry DSN should be valid");
+            .expect("bundled sentry DSN should be valid");
-	}
+    }
 
-	#[test]
+    #[test]
-	fn opting_out_skips_the_client() {
+    fn opting_out_skips_the_client() {
-		assert!(init(false).is_none());
+        assert!(init(false).is_none());
-	}
+    }
+
+    /// Build an event the way `sentry-tracing` would, given the value of a
+    /// `sentry` field on the log call (or `None` for no such field).
+    fn event_with_sentry_field(value: Option<bool>) -> Event<'static> {
+        let mut event = Event::default();
+        if let Some(value) = value {
+            let mut fields = std::collections::BTreeMap::new();
+            fields.insert("sentry".to_owned(), serde_json::Value::Bool(value));
+            event
+                .contexts
+                .insert(TRACING_FIELDS_CONTEXT.to_owned(), Context::Other(fields));
+        }
+        event
+    }
+
+    #[test]
+    fn drops_events_flagged_sentry_false() {
+        assert!(drop_opted_out_events(event_with_sentry_field(Some(false))).is_none());
+    }
+
+    #[test]
+    fn keeps_events_without_opt_out() {
+        assert!(drop_opted_out_events(event_with_sentry_field(Some(true))).is_some());
+        assert!(drop_opted_out_events(event_with_sentry_field(None)).is_some());
+        assert!(drop_opted_out_events(Event::default()).is_some());
+    }
 }