From 65160e1126af92d8cc7aaac5d4ff2860c8e31c98 Mon Sep 17 00:00:00 2001 From: "posthog[bot]" <206114724+posthog[bot]@users.noreply.github.com> Date: Thu, 17 Sep 2026 07:10:21 +0000 Subject: [PATCH 1/3] fix(daemon): promote every error field alias into telemetry messages The SentryLayer only promoted a structured field named literally `error` into the exception value. The ~dozen `tracing::error!(%e, ...)` call sites in the daemon name the field `e`, so their underlying `AutterError` stayed in the context map and the captured event had no cause. Those issues grouped and titled by the static log string alone, which made them undiagnosable from error tracking. Promote the first non-empty of `error`, `err`, `e`, or `source`, so all alias spellings reach the exception value and distinct causes fingerprint separately. Also name the two control-socket setup failures that previously propagated bare io errors (stale socket removal and owner-only permissions), so each setup step is self-describing alongside the already named bind failure. Generated-By: PostHog Desktop Task-Id: 12288664-6d02-4945-8e0d-5064adfcdea4 --- src/daemon.rs | 16 ++++++++-- src/daemon/sentry_layer.rs | 64 ++++++++++++++++++++++++++++++++------ 2 files changed, 68 insertions(+), 12 deletions(-) diff --git a/src/daemon.rs b/src/daemon.rs index 6adb325..d061739 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -3601,7 +3601,13 @@ fn now_unix_nanos() -> u128 { 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 {}: {}", + path.display(), + e + )) + })?; } #[cfg(not(unix))] let _ = path; @@ -3613,7 +3619,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 {}: {}", + path.display(), + 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" + ); + } } From 8fd378ad2f8b37b9b8b58b382413a57a45e44658 Mon Sep 17 00:00:00 2001 From: Sagnik Ghosh Date: Fri, 9 Oct 2026 02:15:44 +0530 Subject: [PATCH 2/3] fix(daemon): name the socket by file name, not absolute path, in setup errors These errors are promoted into the exception value, so the absolute socket path (under the user's home directory) would split each cause into one issue per user and put the username into error tracking. Use the socket file name (control.sock / trace2.sock), which still tells the control and trace listeners apart. Co-Authored-By: Claude Opus 5.5 --- src/daemon.rs | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/src/daemon.rs b/src/daemon.rs index d061739..1345963 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -3598,13 +3598,24 @@ 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")) +} + fn remove_socket_if_exists(path: &Path) -> Result<(), AutterError> { #[cfg(unix)] if path.exists() { fs::remove_file(path).map_err(|e| { AutterError::Generic(format!( "failed removing stale socket {}: {}", - path.display(), + socket_label(path), e )) })?; @@ -3622,7 +3633,7 @@ fn set_socket_owner_only(path: &Path) -> Result<(), AutterError> { fs::set_permissions(path, fs::Permissions::from_mode(0o600)).map_err(|e| { AutterError::Generic(format!( "failed setting owner-only permissions on socket {}: {}", - path.display(), + socket_label(path), e )) })?; @@ -9472,6 +9483,18 @@ mod tests { ); } + #[cfg(unix)] + #[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"); + } + #[test] fn ai_replay_checkpoint_request_preserves_active_bash_agent_identity() { let agent_id = AgentId { From 5912313ecee2c251f0f2bebce763acdc07d65f7a Mon Sep 17 00:00:00 2001 From: Sagnik Ghosh Date: Fri, 9 Oct 2026 02:28:18 +0530 Subject: [PATCH 3/3] test(daemon): keep socket_label test beside the helper Avoids a merge conflict with #57, which adds a test at the same spot in the daemon tests module. Co-Authored-By: Claude Opus 5.5 --- src/daemon.rs | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/src/daemon.rs b/src/daemon.rs index 1345963..9f275fd 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -3609,6 +3609,23 @@ fn socket_label(path: &Path) -> std::borrow::Cow<'_, str> { .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() { @@ -9483,18 +9500,6 @@ mod tests { ); } - #[cfg(unix)] - #[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"); - } - #[test] fn ai_replay_checkpoint_request_preserves_active_bash_agent_identity() { let agent_id = AgentId {