fix(desktop): survive losing the auth-token lock mid-write
Problem
DESKTOP-11M plus roughly 20 sibling groups over 90 days (releases 1.18.3, 1.19.0, 1.20.2, 1.22.0) are all one event in the desktop main process:
Error: ENOENT: no such file or directory, stat '<home>/.superset/auth-token.enc.lock'
reported level=fatal, handled=no, mechanism=generic, with no stack. It splits into a new group per machine because the message embeds the lock entry's absolute path, which contains the account name — so the family is self-multiplying and every new group puts an account name in an issue title.
User-visible harm: when this fires, the auth write that was in flight succeeded on disk but is reported to the user as failed. handleAuthCallback catches the rejection, puts the CSRF state back, and shows "Superset could not save your sign-in to …"; the persistToken / signOut mutations fail the same way in the renderer. The user is told their sign-in did not persist when it did, and is pointed at a file path to go inspect.
The app itself does not quit: main/index.ts:276 registers an uncaughtException listener, and @sentry/electron's handler only shows its error box when fewer than three listeners exist. So apart from the wrong outcome, the crash is invisible — except that it marks the session crashed in release health. Rare (~20 events / 90 days), but it lands on the auth path and the message it produces actively misinforms.
Root cause
Not what the ENOENT looks like. Two things had to be ruled out first, both against the installed proper-lockfile@4.1.2 with this call site's exact options:
- It is not the acquire-time staleness race. The library already absorbs a lock entry that vanishes between
mkdir'sEEXISTand thestatthat checks its age —lib/lockfile.js:56-62retries withstale: 0. Reproduced directly: acquisition resolves, no error escapes. A persistent version of that same failure surfaces asELOCKED, never asENOENT. - It is not an unhandled rejection.
saveToken/clearToken/saveOrganizationIdsall go throughserializeAuthWrite, whose promise is observed twice (by the caller and by the queue chain), and every caller awaits it —persistToken,persistOrganizationIds,signOut, andhandleAuthCallback, which already wraps the write intry/catch. No handler on the write path could have caught this one.
What actually happens: after lock() succeeds, proper-lockfile keeps the entry fresh on a 5s timer (stale / 2). If a write is still in flight when that tick lands and another writer has meanwhile reclaimed the entry — our refresh fell behind, so our lock looked stale to them — the refresh fs.stat returns ENOENT and the library calls onCompromised. We never supplied one, so the default (err) => { throw err } throws from inside its own setTimeout callback. Nothing can catch that, and it explains the missing stack: async fs errors carry no JS frames. The captured level: 'fatal' / mechanism: { handled: false, type: 'generic' } matches @sentry/electron's onUncaughtExceptionIntegration verbatim, which is how we know it arrived as an uncaught exception rather than a rejection.
[!NOTE]
Corrected during triage review, before merge. This paragraph originally claimed that two main processes on one machine are reachable because the app never calls requestSingleInstanceLock(). That is wrong. main/index.ts:344 does take a single-instance lock and calls app.exit(0) when it loses, so a second copy of the same build cannot race this entry; and dev/worktree setups point SUPERSET_HOME_DIR at their own directory, so those do not contend either.
What the event actually establishes is narrower, and the fix does not depend on the difference: the lock entry was gone when our own refresh timer stat'd it. Anything that removes ~/.superset/auth-token.enc.lock while a write is in flight produces exactly this ENOENT, and the event does not record what did. The library throwing from inside its own timer is a defect in how we call it regardless of what removed the entry.
proper-lockfile is imported nowhere else in the repo, so this lock has exactly one writer in our code.
The same event has a second consequence. Losing the lock marks it released, so the finally { await release() } rejects with ERELEASED — and a rejection thrown from a finally replaces the result of the try. That is the step that turns a token that was written into a sign-in reported as failed.
Fix
Two lines in withAuthLock, both about the same lost-lock event:
- Supply
onCompromised, so the lock library reports the loss to us instead of throwing it into a timer. The write is atomic (temp file +rename), so it stays valid whoever else is writing; there is nothing to abort, and the operation goes on to report its own result. - Make the
finallyrelease best-effort. Releasing is cleanup — it fails when the lock was already taken from us — and it says nothing about whether the write landed, so it must not override the operation's success or failure.
On the two shapes proposed in triage. Neither is available, and the evidence above is why. Shape (i), making acquisition tolerate the vanished-entry race so the retries absorb it, is already true in the library — that path resolves today, so implementing it would be a no-op that leaves the fatal event exactly where it is. Shape (ii), handling the rejection on the write path, cannot apply: this is not a rejection but a synchronous throw from the lock library's own refresh timer, unreachable from any try/catch we could add, and the Sentry mechanism confirms it arrived as an uncaught exception. What both shapes were reaching for is the third option — the throw exists only because we never gave the library somewhere to report a compromised lock — so that is what this changes, at the same call site, with the same blast radius. I took the release fix with it rather than separately because it is caused by the identical event and without it the crash is gone while the behaviour is still wrong: the user still gets told a sign-in failed that in fact persisted.
What this deliberately does not do: it does not swallow failures. Acquisition failures (lock() rejects before the try) and write failures (operation() rejects inside it) both still propagate to the caller unchanged. A token that was not persisted can never come back looking persisted. Timing parameters, encryption, file format, permissions repair, and the quarantine path are untouched, and there is no new Sentry capture — the two warnings use console.warn, matching the rest of the file, and reach Sentry as breadcrumbs via the existing console integration.
Verification
Negative test, written first (does not report a saved token when the write lock never frees up): a lock entry with a fresh mtime never looks stale, so acquisition runs out of retries. saveToken must still reject, must not emit token-saved, and must leave no token on disk. It passes on unmodified main, and — the point of it — it fails against a naive swallow-everything version of this change (try { … } catch { return undefined as Result }), together with the two negative tests the file already had:
15 pass
3 fail
(fail) auth token storage > does not report a saved token when the write lock never frees up
Expected promise that rejects
Received promise that resolved: Promise { <resolved> }
(fail) auth token storage > does not report sign-out when the credential cannot be cleared
Reproduction of the reported event, before and after. Mirrors withAuthLock with the same options, a write still in flight at the 5s refresh tick, and another writer that reclaimed and released the entry:
== before the fix ==
[FATAL uncaught exception -> Sentry level=fatal handled=no] ENOENT: no such file or directory, stat '…/auth-token.enc.lock'
stack frames: 0
caller saw: FAILURE (ERELEASED: Lock is already released) -- "Superset could not save your sign-in"
token on disk: YES (the write did land)
== after the fix ==
[auth] Lost the auth token lock while writing ECOMPROMISED
[auth] Failed to release the auth token lock ERELEASED
caller saw: SUCCESS (token written)
token on disk: YES (the write did land)
Suite — bun test src/lib/trpc/routers/auth/, the file that holds the signed-in session:
before: 17 pass, 0 fail, 38 expect() calls [664.00ms]
after: 18 pass, 0 fail, 41 expect() calls [960.00ms]
Lint — bunx biome check on both changed files:
Checked 2 files in 24ms. No fixes applied.
Types — cd apps/desktop && bun run typecheck:
$ tsc --noEmit
TYPECHECK_EXIT=0
Adjacent, named and left alone
app.on("open-url", async …) (main/index.ts:175) and app.on("second-instance", async …) (main/index.ts:350) both float the promise their async handler returns. Neither can reject today — processDeepLink awaits handleAuthCallback, which already catches — so they are unguarded but not currently reachable, and they are not this bug. Left as-is.
Refs DESKTOP-11M
https://claude.ai/code/session_01PXMCuUAHrKLAhRWtPF1o7V
Summary by cubic
Prevent desktop auth writes from surfacing as failures when the lock is reclaimed mid-write. Previously, proper-lockfile’s refresh could throw ENOENT as an uncaught exception and release()’s ERELEASED would overwrite a successful write; now we handle compromised locks and make release best-effort.
- Review notes
- In
withAuthLock, passonCompromisedtoproper-lockfileto log and continue; wraprelease()in a.catch()so cleanup failures never replace the operation result. - Behavior change: successful token writes now report success even if the lock was reclaimed; acquisition and write errors still propagate; only new console warnings are emitted. Eliminates the Sentry fatal and the user-facing “could not save your sign-in” false negative.
- Adds a test to ensure we still reject when the lock never becomes available.
- In
Refs DESKTOP-11M
Summary by CodeRabbit
- Bug Fixes
- Improved authentication token storage reliability when persistent locking is unavailable.
- Prevented lock-related errors from interrupting authentication operations.
- Ensured lock-release failures are handled gracefully without replacing the original operation result.
- Added coverage for failed token saves, confirming no token file or save event is produced.