Repository navigation
fix(desktop): retry store writes on Windows EPERM - #51474
waterundman wants to merge 1 commit into
Conversation
|
The following comment was made by an LLM, it may be inaccurate: |
3cd7a80 to
fe1908b
Compare
|
One thing worth flagging for reviewers: |
|
Two things that might be keeping this out of the queue, plus one scope question.
Scope question: the mutation-path half of this race is the same class as #45154 (desktop scoped-store cleanup can race later writes), which is assigned to you — and #47497 covered the CLI half before it closed. Would you rather have this as a standalone fix for the store mutation path, or folded into #45154's cleanup work? Happy either way; I just don't want to land a second fix for the same race. For the record on timing: I know older PRs here are auto-closed at one month without 2 positive reactions. Not fishing for reactions — just noting that if the shape is wrong, closing it is cheaper for everyone than letting it time out. |
|
This branch has been upgraded to V2. We're no longer taking PRs for V1. If you think this is still relevant, please port it over and open it for V2. — from 𝕺𝖕𝖊𝖓𝕮𝖔𝖉𝖊 |
Issue for this PR
Closes #51470
Type of change
What does this PR do?
store-set,store-deleteandstore-clearcalledelectron-storedirectly, so when the finalrenamehit a transient Windows sharing violation (EPERM/EBUSY/EACCES) the error was thrown straight back into the renderer and the write was silently lost — the store kept its old contents and the change disappeared on the next reload. Log line in #51470.The three handlers now go through
mutateStore(store-write.ts):conf/atomicallyalready retry therenameitself for 1s, but that is a hot sync loop that blocks the main process; the extra attempts are awaited with a timer in between, so the event loop gets a chance to run between them.writeFileSync. That is not atomic, but a tiny JSON file that survives a crash mid-write is the better trade against dropping the user's change and keeping the stale file. If even that fails, it logs and resolves instead of rejecting into the renderer.removeStoreFileIfEmptyafter delete/clear now runs through the same queue instead of reading/deleting the file while a write is renaming onto it.Errors that are not transient still propagate to the renderer exactly as before.
How did you verify your code works?
bun testinpackages/desktop: addedstore-write.test.tswith 5 tests (retry until it lands, in-place fallback, non-transient errors rejected, per-file serialization, unrelated stores not blocked) — all pass; the two pre-existing failures in that package (draft-store,wsl/servers) fail identically on a clean checkout here because of this machine's environment (node:sqlitemissing, Electron binary not installed).bun run typecheckinpackages/desktoppasses.bun run lint: no findings on the changed files.Screenshots / recordings
Not a UI change.
Checklist