desktop: classify expected sidecar shutdowns - #1776
Merged
Merged
Conversation
Contributor
Cloudflare previewTorn down — the PR is closed. |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
executor-marketing | c092bd4 | Commit Preview URL Branch Preview URL |
Aug 28 2026, 02:54 AM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
executor-cloud | c092bd4 | Aug 28 2026, 02:56 AM |
@executor-js/cli
@executor-js/config
@executor-js/execution
@executor-js/sdk
@executor-js/codemode-core
@executor-js/runtime-quickjs
@executor-js/plugin-file-secrets
@executor-js/plugin-graphql
@executor-js/plugin-keychain
@executor-js/plugin-mcp
@executor-js/plugin-onepassword
@executor-js/plugin-openapi
executor
commit: |
The reconnect path reported a stale chunk through the global reportError hook, which files it as an unhandled crash and drops the surface and action. Route it through the renderer's handled-error reporter instead.
RhysSullivan
force-pushed
the
fix/desktop-sidecar-shutdown-classification
branch
from
August 28, 2026 02:52
e10b725 to
c092bd4
Compare
RhysSullivan
marked this pull request as ready for review
August 28, 2026 03:12
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
1 participant
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
When the local server (the desktop sidecar) is interrupted from outside the app — a group-wide Ctrl-C aimed at Electron, which reaches the child because it shares Electron's process group — the app treated it as a crash. Only a stop the app initiated itself counted as expected, so an ordinary shutdown filed a crash report and the window told the user one had been sent.
Two follow-on symptoms come from the same moment:
Failed to fetch dynamically imported module). Nothing listened forvite:preloadError, so the rejection went unhandled and the route was simply dead, with no explanation and a reported TypeError.loadURLrejects withERR_FAILEDwhen its window is destroyed mid-navigation. That is a teardown race we cause, not a failure to reach the server, but it surfaced as an error.Fix
classifySidecarExitsplits a post-boot exit into managed-stop / external-shutdown / crash. A shutdown signal (SIGINT/SIGTERM/SIGHUP, or the matching 129/130/143 exit codes) or a quit already in progress still shows the disconnected screen — the server really is gone — but reports nothing and does not claim a report was sent. Every other exit is reported exactly as before.vite:preloadErrorhandler probes/api/healthto tell a dead server from a stale chunk: reconnect surface and no report when the server is gone, one session-guarded reload when it answers, reported if that reload does not help.loadURLaborts during teardown are classified and logged instead of becoming unhandled rejections.Testing
No e2e scenario: neither behaviour is observable through the harness. The exit classification only changes what is reported and what the crash screen claims, and both are gated on a release-only build define that dev and e2e builds never have;
vite:preloadErroris dispatched only by Vite's production preload helper, and the targets whose server can be killed mid-run serve dev bundles where it never fires. So verification here is unit-level.The tests cover the classifiers and the wiring around them, since a correct verdict acted on wrongly is the same bug:
startSidecar, with only the process spawn and Electron's environment replaced. An outside interrupt is checked to leave the crash channel untouched and to tell the window nothing was sent; a genuine death still reports; a stop we asked for says nothing at all.The one link still uncovered is the single line in
index.tsthat passes the notice to the crash screen; that module boots the whole app at import, so covering it needs a full Electron harness for one assignment.apps/desktop and packages/app suites, typecheck, lint and format pass. No signing, updater, or process-group changes here.