Skip to content

fix(core): surface corrupt and permission errors in publish-state I/O#4269

Open
calebcgates wants to merge 1 commit into
electron:mainfrom
calebcgates:fix/publish-state-surface-corrupt-and-permission-errors
Open

fix(core): surface corrupt and permission errors in publish-state I/O#4269
calebcgates wants to merge 1 commit into
electron:mainfrom
calebcgates:fix/publish-state-surface-corrupt-and-permission-errors

Conversation

@calebcgates
Copy link
Copy Markdown

  • I have read the contribution documentation for this project.
  • I agree to follow the code of conduct that this project follows, as appropriate.
  • The changes are appropriately documented (if applicable).
  • The changes have sufficient test coverage (if applicable).
  • The testsuite passes successfully on my local machine (if applicable).

Summarize your changes:

packages/api/core/src/util/publish-state.ts is the persistence layer behind electron-forge publish --dry-run / --from-dry-run. The class persists make results to <outDir>/publish-dry-run/<sha256>/<hash>.forge.publish so users can resume publishing without re-running make (which can take 10+ minutes on a real Electron app).

Today, three fs-extra calls — fs.readJson in load(), fs.mkdirs + fs.writeJson in saveToDisk() — have no error handling. Three failure modes hit users with opaque errors that hide the actual cause:

  1. load() on a corrupt state file — if a previous make was killed mid-write (Ctrl-C, OOM, kill -9, NFS write that didn't fsync), readJson rejects with SyntaxError: Unexpected end of JSON input. The user sees this in the listr task UI with no path, no recovery hint.
  2. load() on a missing state file — if the user manually cleaned out/ between --dry-run and --from-dry-run, readJson rejects with ENOENT and the entire resume crashes — no graceful skip across the other state files in the same dir.
  3. saveToDisk() on a non-writable out/ — corporate-managed macOS profile, full-disk-encrypted volume locked, Windows AppData on a network share that disconnected, sandbox --readonly. fs.mkdirs rejects with EACCES/EPERM/EROFS and the user sees an opaque rejection with no hint that it's their out/ directory.

This PR:

  • Wraps load() with try/catch that translates ENOENT and SyntaxError into clear, actionable error messages (with the offending path and a recovery hint). The original error is attached as .cause via Object.assign (matching the existing target: ES2021 lib in tsconfig.base.json — no Error 2-arg constructor / ES2022 dependency). Other errors re-throw unchanged.
  • Replaces fs.mkdirs(dirname) + fs.writeJson(file, ...) in saveToDisk() with the documented fs-extra equivalent fs.outputJson(file, ...) — see outputJson.md: "Almost the same as writeJson, except that if the directory does not exist, it's created." This collapses two file-system calls into one atomic call (no race window where mkdirs succeeds but writeJson fails), and wraps it in try/catch that surfaces EACCES/EPERM/EROFS with the path included.
  • Adds packages/api/core/spec/fast/util/publish-state.spec.ts (8 tests) covering happy path + ENOENT + corrupt-JSON + EACCES + EROFS + ENOSPC re-throw for both methods. This is the first dedicated spec for publish-state.ts; it follows the existing pattern from install-dependencies.spec.ts (vi.mock(import('fs-extra'), async (importOriginal) => ...)). No new test infra introduced — the spec/fast/ workspace already globs this path via vitest.workspace.mts.

No behaviour change on the happy path. The dry-run state files are still written to and read from the same paths in the same JSON format. The only observable change is what surfaces when the file system fails.

Verification

  • yarn build — clean (the pre-existing @aws-sdk/types Credentials error in PublisherS3.ts is on main; not introduced here).
  • yarn lint:js — clean (no new warnings).
  • yarn vitest run --project fast packages/api/core — 17 files, 123 tests, all passing (including the 8 new tests).

Scope

The forge codebase has additional fs-extra calls without error handling in WebpackPlugin / VitePlugin, the template scaffolders, and the Maker base class. Those each have different trade-offs (high blast radius, shared-helper refactor, intent-by-omission swallowing) and are out of scope for this PR. This change addresses the smallest, most surgical fix to a user-reproducible failure mode in a critical publish-resume UX feature.

@calebcgates calebcgates requested a review from a team as a code owner May 20, 2026 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant