fix(credentials): diagnose + recover from H8 auth-profile-lock create failures (#2180)

This commit is contained in:
YellowSnnowmann
2026-05-19 14:16:54 -07:00
committed by GitHub
parent c9a77c8757
commit 1cadb986fc
3 changed files with 100 additions and 2 deletions
+25 -1
View File
@@ -593,7 +593,16 @@ impl AuthProfilesStore {
thread::sleep(Duration::from_millis(LOCK_WAIT_MS));
waited = waited.saturating_add(LOCK_WAIT_MS);
} else {
return Err(e).context("Failed to create auth profile lock");
// Sentry OPENHUMAN-TAURI-H8 collapses every
// non-AlreadyExists, non-transient `create_new`
// failure into a single fingerprint with no
// breadcrumb of which OS code actually fired.
// `annotate_lock_create_failure` embeds the
// underlying `io::ErrorKind` + `raw_os_error()` so
// future events split by root cause and we can
// widen `is_transient_fs_error` (or fix the
// underlying condition) for whichever code is hot.
return Err(annotate_lock_create_failure(e));
}
}
}
@@ -664,6 +673,21 @@ impl AuthProfilesStore {
/// means we keep waiting on a lock that was actually already gone, which
/// is the safe direction. Backed by sysinfo so we don't grow a new libc /
/// windows-sys dependency for one syscall.
/// Wrap a non-`AlreadyExists` `create_new` failure with a context line that
/// embeds the underlying `io::ErrorKind` and `raw_os_error()`. Pulled out
/// of [`AuthProfilesStore::acquire_lock`] so unit tests can drive the
/// formatting directly without depending on filesystem permissions (CI runs
/// as root and bypasses `chmod 0500`).
fn annotate_lock_create_failure(err: anyhow::Error) -> anyhow::Error {
let io = err.chain().find_map(|c| c.downcast_ref::<std::io::Error>());
let kind = io.map(|ioe| ioe.kind());
let os_code = io.and_then(|ioe| ioe.raw_os_error());
err.context(format!(
"Failed to create auth profile lock (kind={:?}, os_code={:?})",
kind, os_code
))
}
fn is_pid_alive(pid: u32) -> bool {
use sysinfo::{Pid, ProcessRefreshKind, ProcessesToUpdate, System};
let target = Pid::from_u32(pid);
@@ -442,6 +442,59 @@ fn acquire_lock_writes_pid_so_future_callers_can_recover() {
assert!(!lock_path.exists(), "guard must remove lock on drop");
}
/// Sentry OPENHUMAN-TAURI-H8: when `OpenOptions::create_new` fails with
/// anything other than `AlreadyExists`, the error surfaced to Sentry
/// must embed the underlying `io::ErrorKind` and `raw_os_error()` so we
/// can tell which OS code is firing. Drive the wrapping helper directly
/// with a synthetic `io::Error` so the test is platform-independent and
/// doesn't depend on filesystem permissions (CI runs as root and bypasses
/// `chmod`).
#[test]
fn annotate_lock_create_failure_embeds_io_kind_and_os_code() {
// Use each platform's native permission-denied code so the test exercises
// the OS error that real production failures would carry. Rust does map
// `from_raw_os_error(13)` to `PermissionDenied` on Windows too, but real
// Windows `create_new` failures surface code 5 (ERROR_ACCESS_DENIED), and
// running against the native code catches regressions in
// `annotate_lock_create_failure`'s handling of the platform-specific
// value.
#[cfg(windows)]
let raw_code = 5; // ERROR_ACCESS_DENIED
#[cfg(not(windows))]
let raw_code = 13; // EACCES
let io_err = std::io::Error::from_raw_os_error(raw_code);
let wrapped = annotate_lock_create_failure(anyhow::Error::new(io_err));
let msg = format!("{wrapped:?}");
assert!(
msg.contains("Failed to create auth profile lock"),
"stable top-level message missing: {msg}"
);
assert!(
msg.contains("kind=Some(PermissionDenied)"),
"context must include io::ErrorKind for Sentry diagnosis: {msg}"
);
assert!(
msg.contains(&format!("os_code=Some({raw_code})")),
"context must include raw OS code for Sentry diagnosis: {msg}"
);
}
/// If somehow the chained error is not an `io::Error`, the wrapper must
/// still emit the stable top-level message with explicit `None` markers so
/// the Sentry fingerprint still splits cleanly (and we know to look
/// upstream for an io::Error that got dropped).
#[test]
fn annotate_lock_create_failure_handles_missing_io_error() {
let wrapped = annotate_lock_create_failure(anyhow::anyhow!("synthetic"));
let msg = format!("{wrapped:?}");
assert!(msg.contains("Failed to create auth profile lock"), "{msg}");
assert!(msg.contains("kind=None"), "{msg}");
assert!(msg.contains("os_code=None"), "{msg}");
}
#[test]
fn auth_profile_kind_serde_roundtrip() {
let json = serde_json::to_string(&AuthProfileKind::OAuth).unwrap();
+22 -1
View File
@@ -440,6 +440,17 @@ mod tests {
);
}
#[cfg(windows)]
#[test]
fn is_transient_fs_error_classifies_windows_delete_pending() {
let io_err = std::io::Error::from_raw_os_error(303);
let err = anyhow::Error::new(io_err);
assert!(
is_transient_fs_error(&err),
"ERROR_DELETE_PENDING (303) must be transient on Windows"
);
}
/// A chained io::Error with `ErrorKind::NotFound` is not a transient
/// locking error — we should not retry it.
#[test]
@@ -617,8 +628,18 @@ pub fn is_transient_fs_error(err: &anyhow::Error) -> bool {
// 5: ERROR_ACCESS_DENIED
// 32: ERROR_SHARING_VIOLATION
// 33: ERROR_LOCK_VIOLATION
// 303: ERROR_DELETE_PENDING — the previous owner's
// `Drop::drop` issued `fs::remove_file` and Windows
// acknowledged it, but the file is still in the
// "delete pending" limbo because AV/indexer holds a
// handle. A retry-with-backoff resolves it as soon as
// the holder closes its handle. Sentry OPENHUMAN-TAURI-H8
// bails at `elapsed_ms ≈ 2` against
// `openhuman.team_get_usage` because this code was not
// previously classified as transient and `create_new`
// returned a `kind = Other` io::Error on the first try.
// 1224: ERROR_USER_MAPPED_FILE
return code == 5 || code == 32 || code == 33 || code == 1224;
return code == 5 || code == 32 || code == 33 || code == 303 || code == 1224;
}
}
#[cfg(not(windows))]