diff --git a/src/daemon.rs b/src/daemon.rs index 6adb325..9f275fd 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -3598,10 +3598,44 @@ fn now_unix_nanos() -> u128 { .as_nanos() } +/// Low-cardinality socket label for error messages: the file name only (e.g. +/// `control.sock`), never the absolute path. These messages become exception +/// values, so a home-directory path would split one cause into an issue per +/// user and leak the username into error tracking. +#[cfg(unix)] +fn socket_label(path: &Path) -> std::borrow::Cow<'_, str> { + path.file_name() + .map(|name| name.to_string_lossy()) + .unwrap_or(std::borrow::Cow::Borrowed("socket")) +} + +#[cfg(all(test, unix))] +mod socket_label_tests { + use super::socket_label; + use std::path::Path; + + #[test] + fn socket_label_omits_the_absolute_path() { + assert_eq!( + socket_label(Path::new( + "/Users/someone/.autter/internal/daemon/control.sock" + )), + "control.sock" + ); + assert_eq!(socket_label(Path::new("/")), "socket"); + } +} + fn remove_socket_if_exists(path: &Path) -> Result<(), AutterError> { #[cfg(unix)] if path.exists() { - fs::remove_file(path)?; + fs::remove_file(path).map_err(|e| { + AutterError::Generic(format!( + "failed removing stale socket {}: {}", + socket_label(path), + e + )) + })?; } #[cfg(not(unix))] let _ = path; @@ -3613,7 +3647,13 @@ fn set_socket_owner_only(path: &Path) -> Result<(), AutterError> { #[cfg(unix)] { use std::os::unix::fs::PermissionsExt; - fs::set_permissions(path, fs::Permissions::from_mode(0o600))?; + fs::set_permissions(path, fs::Permissions::from_mode(0o600)).map_err(|e| { + AutterError::Generic(format!( + "failed setting owner-only permissions on socket {}: {}", + socket_label(path), + e + )) + })?; } #[cfg(not(unix))] { diff --git a/src/daemon/sentry_layer.rs b/src/daemon/sentry_layer.rs index c79918a..4c2c677 100644 --- a/src/daemon/sentry_layer.rs +++ b/src/daemon/sentry_layer.rs @@ -67,20 +67,29 @@ impl Visit for MessageVisitor { } } -/// Combine the static tracing message with a structured `error` field (as -/// emitted by `%error`) so the resulting exception value reflects the real -/// underlying cause. Returns the message unchanged when there is no non-empty -/// `error` field to promote. +/// Field names that carry an underlying error's text, in priority order. Call +/// sites name this field inconsistently (`%error`, `error = %e`, `%e`, `%err`), +/// so the promotion has to recognise every alias rather than only `error`. +const PROMOTED_ERROR_FIELDS: [&str; 4] = ["error", "err", "e", "source"]; + +/// Combine the static tracing message with the underlying error's text so the +/// resulting exception value reflects the real cause. The error is read from the +/// first non-empty of the [`PROMOTED_ERROR_FIELDS`] aliases. Returns the message +/// unchanged when none of them are present. fn message_with_promoted_error( message: &str, fields: &serde_json::Map, ) -> String { - match fields.get("error").and_then(|v| v.as_str()) { - Some(error) if !error.is_empty() && !message.is_empty() => { - format!("{}: {}", message, error) - } - Some(error) if !error.is_empty() => error.to_string(), - _ => message.to_string(), + let promoted = PROMOTED_ERROR_FIELDS.iter().find_map(|name| { + fields + .get(*name) + .and_then(|v| v.as_str()) + .filter(|error| !error.is_empty()) + }); + match promoted { + Some(error) if !message.is_empty() => format!("{}: {}", message, error), + Some(error) => error.to_string(), + None => message.to_string(), } } @@ -177,4 +186,39 @@ mod tests { let f = fields(&[("error", json!("standalone cause"))]); assert_eq!(message_with_promoted_error("", &f), "standalone cause"); } + + #[test] + fn promotes_e_field_from_percent_e_call_sites() { + let f = fields(&[( + "e", + json!("failed binding control socket: Address already in use"), + )]); + assert_eq!( + message_with_promoted_error("control listener exited with error", &f), + "control listener exited with error: failed binding control socket: Address already in use" + ); + } + + #[test] + fn promotes_err_and_source_aliases() { + let err = fields(&[("err", json!("update check failed cause"))]); + assert_eq!( + message_with_promoted_error("update check failed", &err), + "update check failed: update check failed cause" + ); + let source = fields(&[("source", json!("root cause"))]); + assert_eq!( + message_with_promoted_error("wrapper failed", &source), + "wrapper failed: root cause" + ); + } + + #[test] + fn skips_empty_alias_for_a_later_populated_one() { + let f = fields(&[("error", json!("")), ("e", json!("real cause"))]); + assert_eq!( + message_with_promoted_error("something happened", &f), + "something happened: real cause" + ); + } }