Skip to content

fix(update): win32 auto-update dead-ended in "SpyDE cannot be closed" - #147

Open
CSSFrancis wants to merge 1 commit into
mainfrom
fix/windows-auto-update-handoff
Open

fix(update): win32 auto-update dead-ended in "SpyDE cannot be closed"#147
CSSFrancis wants to merge 1 commit into
mainfrom
fix/windows-auto-update-handoff

Conversation

@CSSFrancis

Copy link
Copy Markdown
Member

Reported by a user on Windows 11: the auto-update runs, the installer reaches "Installing", and then

SpyDE cannot be closed. Please close it manually and click Retry.

Retry never clears it. The only way out was uninstalling from Add/Remove Programs and installing by hand.

There are two independent defects, and fixing either one alone leaves the dead end in place.

1. Ours — we lose the handoff race

electron-updater spawns the installer first and only then asks the app to quit, so for a second or two both are alive. Meanwhile stopBackend() writes a graceful quit to the Python sidecar and arms a 1.5 s tree-kill backstop — a timer the exiting process never lives long enough to fire. The sidecar and its Dask workers are orphaned, still holding resources\python inside the install directory.

  • stopBackend({immediate: true}) tree-kills synchronously (spawnSync, no timer), so it still happens when the caller is about to end the process.
  • quitAndInstall() hands off, then kills, then force-exits rather than trusting Electron's graceful quit to win.

Order matters and is commented in the code: quitAndInstall() spawns the installer synchronously, so a throw means the handoff never started and the app has to keep running — killing the sidecar first would leave it alive but useless.

2. Theirs — electron-builder's app-running check dead-ends

The stock check asks PowerShell for every process under $INSTDIR, stops them one at a time with Stop-Process (no tree kill), re-checks with no pause, and after two rounds shows that dialog — whose Retry re-runs the identical check. Nothing the user does in that dialog can change the answer.

Upgrading does not fix it. I checked:

app-running macro
26.15.3 (latest, ours)
26.15.7 (v26) byte-identical
27.0.0-alpha.7 (next) byte-identical
upstream master FIND_PROCESS/KILL_PROCESS hardened; _CHECK_APP_RUNNING byte-identical

Master's changes (trailing-backslash normalization, quote escaping, -NoProfile -NonInteractive, matching previous install locations) are real, but none addresses a process tree outliving a single Stop-Process, and none is released. electron-builder#8131 is closed as not-planned.

electron/build/installer.nsh overrides the check through the documented customCheckAppRunning hook: kills whole process trees by pid, waits between rounds, gives the app several seconds before asking the user anything, and makes Retry re-run the kill rather than just the question.

⚠️ electron-builder picks this file up silently as the nsis.include default. Deleting it restores the broken stock behaviour with no build error. There's a warning to that effect in CLAUDE.md.

Also: Help → Report a Problem

Asked for alongside the bug report, so failures like this one arrive with their machine attached.

User-initiated only. Problems are recorded into a bounded in-memory ring as they happen; nothing is transmitted until Send is pressed. A report carries OS, app + runtime versions, GPU, managed-Python-env state, the last updater status, and the tail of the backend's output — shown in full in the dialog before sending, because a report that quietly ships a machine description is one people learn not to send.

It posts a Sentry envelope written against the ingest protocol directly rather than via @sentry/electron. The SDK's value is automatic crash capture, which "nothing without a click" rules out anyway, and skipping it keeps a native crash handler out of the notarized macOS build.

Action needed before this is useful: create the Sentry project and add its DSN as the SPYDE_SENTRY_DSN repo secret. Without it the feature still works — reports are written to <userData>/reports/ and the dialog says so, which is the offline instrument-PC path rather than a degraded one.

Verification

npm run typecheck pass
npm run test:unit 23 shell-main (9 new) + 32 shell-renderer
report_problem.spec.ts 3 new e2e, pass
update_gpu_dialogs.spec.ts, app_log.spec.ts pass (regression check)

sentryEnvelope.test.ts pins the DSN parse, the auth header, and the envelope's byte-length framing — a path with an accent in it is rejected outright if that counts characters instead.

report_problem.spec.ts drives the real app for both outcomes: no service configured (report written, path shown, file on disk matches what was typed) and a local HTTP server standing in for Sentry (auth header, framing and payload arrive intact through Electron's net stack).

❗ The Windows half is unverified

I'm on macOS. The NSIS path needs a real packaged build on Win 11 to confirm — that's the one thing this PR can't self-check, and it's the part that actually matters to the reporter.

Splitting

The two halves share code (updater.ts records into the problem ring; the sidecar's output ring feeds reports), so they landed as one commit. Happy to separate them into two PRs if you'd rather review the reporting feature on its own.

Reported from a Windows 11 install: the auto-update runs, the installer
reaches "Installing", and then

  SpyDE cannot be closed. Please close it manually and click Retry.

Retry never clears it. The only way out was uninstalling from Add/Remove
Programs and installing by hand.

Two independent defects, and fixing either alone leaves the dead end.

OURS: the handoff is a race we lose. electron-updater spawns the installer
FIRST and only then asks the app to quit, so for a second or two both are
alive. stopBackend() writes a graceful `quit` to the Python sidecar and arms
a 1.5 s tree-kill backstop -- a timer the exiting process never lives long
enough to fire. The sidecar and its Dask workers are orphaned, still holding
resources\python inside the install directory.

  - stopBackend({immediate: true}) tree-kills synchronously (spawnSync, no
    timer) so it still happens when the caller is about to end the process.
  - quitAndInstall() hands off, THEN kills, then force-exits rather than
    trusting Electron's graceful quit to win. Order matters: quitAndInstall()
    spawns the installer synchronously, so a throw means the handoff never
    started and the app has to keep running -- killing first would leave it
    alive but useless.

THEIRS: electron-builder 26.x's stock check asks PowerShell for every process
under $INSTDIR, stops them ONE AT A TIME with Stop-Process (no tree kill),
re-checks with no pause, and after two rounds shows that box -- whose Retry
re-runs the identical check. Verified unfixed in 26.15.7 and 27.0.0-alpha.7
(byte-identical) and on upstream master, where FIND_PROCESS/KILL_PROCESS have
been hardened but _CHECK_APP_RUNNING is untouched. electron-builder#8131 is
closed as not-planned.

electron/build/installer.nsh overrides it through the documented
customCheckAppRunning hook: kills whole process TREES by pid, waits between
rounds, gives the app several seconds before asking the user anything, and
makes Retry re-run the kill. It is picked up silently as the nsis.include
default, so deleting the file restores the stock behaviour with no build
error -- hence the warning in CLAUDE.md.

NOT VERIFIED ON WINDOWS. Typecheck, unit and e2e pass, but the installer path
needs a real packaged build on Win 11 to confirm.

Also adds Help -> Report a Problem, asked for alongside the bug report so
failures like this one arrive with their machine attached. User-initiated
only: problems are recorded into a bounded in-memory ring as they happen, and
nothing is transmitted until Send is pressed. A report carries OS, app and
runtime versions, GPU, managed-Python-env state, the last updater status and
the tail of the backend's output -- shown in full in the dialog first, because
a report that quietly ships a machine description is one people learn not to
send.

It posts a Sentry envelope written against the ingest protocol directly
rather than via @sentry/electron: the SDK's value is automatic crash capture,
which "nothing without a click" rules out anyway, and skipping it keeps a
native crash handler out of the notarized macOS build. The DSN comes from the
SPYDE_SENTRY_DSN repo secret at build time; with none configured the report is
written to <userData>/reports/ and the dialog says so, which is the offline
instrument-PC path rather than a degraded one.

Tests: sentryEnvelope.test.ts pins the DSN parse, auth header and the
envelope's BYTE-length framing (a path with an accent is rejected outright if
that counts characters). report_problem.spec.ts drives the real app for both
outcomes -- no service configured (report written, path shown, file on disk
matches) and a local server standing in for Sentry (auth header, framing and
payload arrive intact through Electron's net stack).
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