fix(desktop): guard window-state store writes against disk-write failures#3304
Draft
posthog[bot] wants to merge 1 commit into
Draft
fix(desktop): guard window-state store writes against disk-write failures#3304posthog[bot] wants to merge 1 commit into
posthog[bot] wants to merge 1 commit into
Conversation
…ures Window-state persistence goes through electron-store's synchronous atomic `writeFileSync`, which can throw on transient filesystem errors (e.g. a full disk). These writes ran inside a `setTimeout` callback and event handlers with no try/catch, so a throw became an uncaught exception in the Electron main process, crashing it and spamming error tracking. Route the window-state writes through a shared `setWindowStateKey` helper that catches and logs via the scoped logger, and use it in `saveWindowState` and the `utils/store.ts` setters. Persistence is non-critical, so a failed write should never take down the main process. Generated-By: PostHog Code Task-Id: b3d5abbb-2f47-4171-bdf8-a7464bf6c6e1
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
|
React Doctor found no issues in the changed files. 🎉 Reviewed by React Doctor for commit |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A full disk on one user's machine crashed the Electron main process. When the app saves window position/size,
saveWindowStatecalls into electron-store, which does a synchronous atomicwriteFileSync. That write threwENOSPC: no space left on device, and because it runs inside thescheduleSaveWindowStatesetTimeoutcallback (and window event handlers) with no try/catch, the throw became an uncaught exception in the main process and got reported to error tracking. The same unguarded pattern existed in theutils/store.tssetters (saveZoomLevel,saveFullScreenState,setRestoreFullScreenOnNextLaunch).Why: Window-state persistence is non-critical — a failed write should never be able to take down the main process or spam error tracking. The disk-full condition is on the user's side and can't be fixed in-app, but a transient FS error shouldn't crash the app.
Changes
setWindowStateKeyhelper inutils/store.tsthat wrapswindowStateStore.set(...)in try/catch and logs failures via the scoped logger instead of letting them propagate.saveWindowState(window.ts) and theutils/store.tssetters through this helper.How did you test this?
pnpm --filter code typecheck— passes.pnpm biome checkon both changed files — no issues.Created with PostHog Code from an inbox report