diff --git a/apps/desktop/src/main/app-quit.ts b/apps/desktop/src/main/app-quit.ts new file mode 100644 index 000000000..616e1bdb1 --- /dev/null +++ b/apps/desktop/src/main/app-quit.ts @@ -0,0 +1,23 @@ +/** + * Shared "is the app on its way out?" flag for the main process. + * + * Teardown makes several failures expected rather than exceptional: a sidecar + * child dying, a navigation aborting mid-load. Both the sidecar and window code + * need the same answer, so the flag lives here instead of being threaded + * through either. Listeners are registered at import time (Electron accepts + * `app.on` before ready) so no caller can forget to arm it. + */ + +import { app } from "electron"; + +let quitting = false; + +const markQuitting = () => { + quitting = true; +}; + +app.on("before-quit", markQuitting); +app.on("will-quit", markQuitting); +app.on("quit", markQuitting); + +export const isAppQuitting = (): boolean => quitting; diff --git a/apps/desktop/src/main/crash-screen.test.ts b/apps/desktop/src/main/crash-screen.test.ts new file mode 100644 index 000000000..3c0e6bd8b --- /dev/null +++ b/apps/desktop/src/main/crash-screen.test.ts @@ -0,0 +1,26 @@ +/** + * The screen is shown for an outside interrupt as well as a real crash — the + * server is gone either way — so the one sentence that must not appear when + * nothing was sent upstream is pinned here. Telling a user a report was filed + * when none was is the visible half of the reported problem. + */ + +import { describe, expect, it } from "@effect/vitest"; +import { sidecarCrashHtml } from "./crash-screen"; + +const REPORT_CLAIM = "A crash report was sent automatically"; + +describe("sidecarCrashHtml", () => { + it("claims nothing was sent when nothing was", () => { + const html = sidecarCrashHtml({ reported: false }); + + expect(html).not.toContain(REPORT_CLAIM); + // The screen still has to explain the server is gone and offer a way back. + expect(html).toContain("The local Executor server stopped unexpectedly"); + expect(html).toContain("Restart server"); + }); + + it("says a report was sent when one was", () => { + expect(sidecarCrashHtml({ reported: true })).toContain(REPORT_CLAIM); + }); +}); diff --git a/apps/desktop/src/main/index.ts b/apps/desktop/src/main/index.ts index ba64a02e7..b43c0f4d8 100644 --- a/apps/desktop/src/main/index.ts +++ b/apps/desktop/src/main/index.ts @@ -35,6 +35,8 @@ import { reportAProblem, } from "./diagnostics"; import { sidecarCrashHtml, startupWindowHtml } from "./crash-screen"; +import { isExpectedNavigationAbort } from "./navigation-errors"; +import { isAppQuitting } from "./app-quit"; import { replaceSupervisedDaemonForDesktop } from "./supervised-connection"; import { bundledExecutorPath, @@ -148,6 +150,37 @@ const webUrlForConnection = (conn: SidecarConnection): string => { const htmlDataUrl = (html: string): string => `data:text/html;charset=utf-8,${encodeURIComponent(html)}`; +/** + * True when a `loadURL` rejection is just Chromium cancelling a navigation + * whose window went away — the window closing mid-load is a race we cause, not + * a failure to reach the server. + */ +const isTeardownNavigationError = (error: unknown, window: BrowserWindow): boolean => + isExpectedNavigationAbort({ + // oxlint-disable-next-line executor/no-instanceof-error, executor/no-unknown-error-message -- boundary: Electron rejects loadURL with a plain Node Error carrying the net error name + message: error instanceof Error ? error.message : String(error), + windowDestroyed: window.isDestroyed(), + appQuitting: isAppQuitting(), + }); + +/** + * Fire-and-forget navigation. The caller has no way to react to a failure, so + * an uncaught rejection here would only reach the main-process unhandled + * rejection handler; log it instead, quietly when it is teardown noise. + */ +const loadWindowUrl = async (window: BrowserWindow, url: string): Promise => { + // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: Electron navigation rejects; there is no caller left to surface it to + try { + await window.loadURL(url); + } catch (error) { + if (isTeardownNavigationError(error, window)) { + log.info("Navigation aborted while the window was closing"); + return; + } + log.error("Failed to load window URL", error); + } +}; + // The supervised daemon (and the desktop sidecar) own this data dir — the same // path the CLI's `executor web`/daemon uses, so desktop and CLI share state. const DESKTOP_DATA_DIR = join(homedir(), ".executor"); @@ -355,7 +388,7 @@ const armSupervisedMonitor = () => { supervisedDaemonDown = false; connection = live; installBearerAuthHeader(live.baseUrl, live.authToken); - if (window) void window.loadURL(webUrlForConnection(live)); + if (window) void loadWindowUrl(window, webUrlForConnection(live)); } })(); }, 10_000); @@ -540,10 +573,13 @@ const showStartupWindow = async (): Promise => { } }; -const showCrashScreen = (window: BrowserWindow | null = liveMainWindow()): void => { +const showCrashScreen = ( + window: BrowserWindow | null = liveMainWindow(), + options: { readonly reported: boolean } = { reported: errorReportingEnabled }, +): void => { if (!window) return; - const html = sidecarCrashHtml({ reported: errorReportingEnabled }); - void window.loadURL(htmlDataUrl(html)); + const html = sidecarCrashHtml({ reported: options.reported && errorReportingEnabled }); + void loadWindowUrl(window, htmlDataUrl(html)); }; const createWindow = async (conn: SidecarConnection) => { @@ -560,6 +596,12 @@ const createWindow = async (conn: SidecarConnection) => { await window.loadURL(webUrlForConnection(conn)); if (!window.isDestroyed() && !window.isVisible()) window.show(); } catch (error) { + // The window was closed (or the app is quitting) while this navigation was + // in flight. Nothing failed — there is just nowhere left to navigate. + if (isTeardownNavigationError(error, window)) { + log.info("Executor web UI load aborted while the window was closing"); + return; + } log.error("Failed to load Executor web UI", error); if (!existingWindow) destroyWindow(window); // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: caller decides whether to fall back or surface startup failure @@ -1158,8 +1200,11 @@ const boot = async () => { // A sidecar that dies under a live window would leave the web UI failing // every request with no explanation. Swap in the crash screen — its // buttons drive the regular preload bridge (restart / export diagnostics). - onUnexpectedSidecarExit(() => { - showCrashScreen(); + onUnexpectedSidecarExit((notice) => { + // An expected shutdown (an interrupt aimed at the app) still leaves the web + // UI dead, so the screen is the same — it just must not claim a crash + // report was sent when none was. + showCrashScreen(liveMainWindow(), { reported: notice.reported }); // A crashing sidecar may be a broken release — quietly stage any // available update so the install prompt appears on its own (same // self-heal as the fatal startup path). diff --git a/apps/desktop/src/main/navigation-errors.test.ts b/apps/desktop/src/main/navigation-errors.test.ts new file mode 100644 index 000000000..eb6d16695 --- /dev/null +++ b/apps/desktop/src/main/navigation-errors.test.ts @@ -0,0 +1,40 @@ +import { describe, expect, it } from "@effect/vitest"; +import { isExpectedNavigationAbort } from "./navigation-errors"; + +const abort = (overrides: Partial[0]> = {}) => + isExpectedNavigationAbort({ + message: "ERR_FAILED (-2) loading 'http://127.0.0.1:4789/?_token=redacted'", + windowDestroyed: false, + appQuitting: false, + ...overrides, + }); + +describe("isExpectedNavigationAbort", () => { + it("treats ERR_FAILED as expected when the window was destroyed mid-load", () => { + expect(abort({ windowDestroyed: true })).toBe(true); + }); + + it("treats ERR_CONNECTION_RESET as expected while the app is quitting", () => { + expect( + abort({ + message: "ERR_CONNECTION_RESET (-101) loading 'http://127.0.0.1:4789/'", + appQuitting: true, + }), + ).toBe(true); + }); + + it("keeps ERR_FAILED unexpected under a live window", () => { + // A load that fails while the window is alive and the app is running is a + // genuine "cannot reach the local server" failure and must still surface. + expect(abort()).toBe(false); + }); + + it("keeps an unrelated navigation failure unexpected even during teardown", () => { + expect( + abort({ + message: "ERR_CERT_AUTHORITY_INVALID (-202) loading 'https://example.invalid/'", + windowDestroyed: true, + }), + ).toBe(false); + }); +}); diff --git a/apps/desktop/src/main/navigation-errors.ts b/apps/desktop/src/main/navigation-errors.ts new file mode 100644 index 000000000..cc77f5068 --- /dev/null +++ b/apps/desktop/src/main/navigation-errors.ts @@ -0,0 +1,21 @@ +/** + * Classification of `BrowserWindow.loadURL` rejections. + * + * Chromium cancels an in-flight navigation when the window it belongs to is + * destroyed, and reports the cancellation as a generic net error (ERR_FAILED + * / ERR_ABORTED / ERR_CONNECTION_RESET). That is a shutdown race with no + * diagnostic value. The same net errors under a live window mean the local + * server genuinely could not be loaded, which must still surface — so this is + * a branch, never a blanket ignore. + */ + +const TEARDOWN_NET_ERRORS = /ERR_FAILED|ERR_ABORTED|ERR_CONNECTION_RESET|ERR_CONNECTION_CLOSED/; + +export interface NavigationAbortInput { + readonly message: string; + readonly windowDestroyed: boolean; + readonly appQuitting: boolean; +} + +export const isExpectedNavigationAbort = (input: NavigationAbortInput): boolean => + (input.windowDestroyed || input.appQuitting) && TEARDOWN_NET_ERRORS.test(input.message); diff --git a/apps/desktop/src/main/sidecar-exit-wiring.test.ts b/apps/desktop/src/main/sidecar-exit-wiring.test.ts new file mode 100644 index 000000000..c51e9a7dd --- /dev/null +++ b/apps/desktop/src/main/sidecar-exit-wiring.test.ts @@ -0,0 +1,163 @@ +/** + * The classifier is pure and tested next door; this covers what `startSidecar` + * DOES with each verdict — which is where the reported symptom lived. A child + * interrupted from outside must leave the crash channel untouched and tell the + * window no report was sent, while a genuine post-boot death must still be + * reported. Both halves are driven through the real `startSidecar`, with only + * the process spawn and Electron's environment replaced. + */ + +import { describe, expect, it } from "@effect/vitest"; +// oxlint-disable-next-line executor/no-vitest-import -- boundary: vi.mock must come from vitest itself for mock hoisting to resolve +import { vi } from "vitest"; +import { EventEmitter } from "node:events"; + +let appQuitting = false; +const crashReports: string[] = []; + +vi.mock("electron", () => ({ + app: { isPackaged: true, getVersion: () => "1.6.0", on: () => {} }, +})); + +vi.mock("electron-log/main.js", () => { + const scope = () => ({ info: () => {}, error: () => {}, warn: () => {}, debug: () => {} }); + return { default: { scope }, scope }; +}); + +vi.mock("./local-auth", () => ({ loadOrMintLocalAuthToken: () => "test-token" })); + +vi.mock("./settings", () => ({ getServerSettings: () => ({ port: 4789 }) })); + +vi.mock("./diagnostics", () => ({ + reportSidecarCrash: (message: string) => { + crashReports.push(message); + }, + sidecarCrashReportingEnv: () => ({}), +})); + +vi.mock("./supervised-daemon", () => ({ resolveSupervisedDaemonAttach: () => null })); + +vi.mock("./app-quit", () => ({ isAppQuitting: () => appQuitting })); + +class FakeChild extends EventEmitter { + readonly stdout = new EventEmitter(); + readonly stderr = new EventEmitter(); + readonly pid = 4242; + exitCode: number | null = null; + killed = false; + kill() { + this.killed = true; + return true; + } +} + +// The packaged branch of `resolveSidecarCommand` reads Electron's resources +// path; nothing is executed, since the spawn itself is replaced below. +(process as { resourcesPath?: string }).resourcesPath ??= "/tmp/executor-test-resources"; + +let spawned: FakeChild | null = null; + +vi.mock("node:child_process", () => ({ + spawn: () => { + spawned = new FakeChild(); + return spawned; + }, +})); + +const { startSidecar, onUnexpectedSidecarExit, stopSidecar } = await import("./sidecar"); + +/** Boot a sidecar to the point `startSidecar` resolves, then hand back the child. */ +const bootedSidecar = async (): Promise => { + const started = startSidecar(); + // The child announces readiness with the structured stdout sentinel. + await Promise.resolve(); + spawned?.stdout.emit("data", Buffer.from("EXECUTOR_READY:4789\n")); + await started; + // oxlint-disable-next-line executor/no-non-null-assertion -- the spawn mock always records a child + return spawned!; +}; + +interface Observed { + readonly notices: { readonly reported: boolean }[]; + readonly reports: string[]; +} + +/** Boot, drive one post-boot exit, and report what the app did about it. */ +const exitAfterBoot = async ( + exit: { readonly code: number | null; readonly signal: NodeJS.Signals | null }, + options: { readonly quitting?: boolean; readonly stopFirst?: boolean } = {}, +): Promise => { + appQuitting = false; + crashReports.length = 0; + const notices: { readonly reported: boolean }[] = []; + onUnexpectedSidecarExit((notice) => notices.push(notice)); + + const child = await bootedSidecar(); + child.stderr.emit("data", Buffer.from("some trailing output\n")); + if (options.stopFirst) { + const stopping = stopSidecar(child as never); + child.emit("exit", exit.code, exit.signal); + await stopping; + } else { + appQuitting = options.quitting ?? false; + child.emit("exit", exit.code, exit.signal); + } + await Promise.resolve(); + return { notices, reports: [...crashReports] }; +}; + +describe("startSidecar post-boot exit handling", () => { + it("does not report an outside interrupt, and tells the window nothing was sent", async () => { + // A group-wide Ctrl-C reaches the child because it shares Electron's + // process group; Node surfaces it as 128+SIGINT. The server really is gone, + // so the window must still be told — but no crash report exists to claim. + const { notices, reports } = await exitAfterBoot({ code: 130, signal: null }); + + expect(reports).toEqual([]); + expect(notices).toEqual([{ reported: false }]); + }); + + it("does not report a SIGTERM the app did not send", async () => { + const { notices, reports } = await exitAfterBoot({ code: null, signal: "SIGTERM" }); + + expect(reports).toEqual([]); + expect(notices).toEqual([{ reported: false }]); + }); + + it("still reports a genuine post-boot death and says a report was sent", async () => { + const { notices, reports } = await exitAfterBoot({ code: 1, signal: null }); + + expect(reports).toHaveLength(1); + expect(reports[0]).toContain("code=1"); + expect(notices).toEqual([{ reported: true }]); + }); + + it("still reports an abnormal death signal", async () => { + const { notices, reports } = await exitAfterBoot({ code: null, signal: "SIGSEGV" }); + + expect(reports).toHaveLength(1); + expect(notices).toEqual([{ reported: true }]); + }); + + it("says nothing at all when the app itself stopped the sidecar", async () => { + const { notices, reports } = await exitAfterBoot( + { code: null, signal: "SIGTERM" }, + { stopFirst: true }, + ); + + expect(reports).toEqual([]); + expect(notices).toEqual([]); + }); + + it("says nothing at all when the app is quitting", async () => { + // Quit tears the tree down; the window is going away, so surfacing a + // disconnected screen at that moment would be its own bug. + const { notices, reports } = await exitAfterBoot( + { code: 130, signal: null }, + { quitting: true }, + ); + + expect(reports).toEqual([]); + expect(notices).toEqual([]); + }); +}); diff --git a/apps/desktop/src/main/sidecar-exit.test.ts b/apps/desktop/src/main/sidecar-exit.test.ts new file mode 100644 index 000000000..0fdb7417a --- /dev/null +++ b/apps/desktop/src/main/sidecar-exit.test.ts @@ -0,0 +1,70 @@ +import { describe, expect, it } from "@effect/vitest"; +import { classifySidecarExit, type SidecarExitInput } from "./sidecar-exit"; + +const classify = (overrides: Partial = {}) => + classifySidecarExit({ + code: null, + signal: null, + stoppedByUs: false, + appQuitting: false, + ...overrides, + }); + +describe("classifySidecarExit", () => { + it("classifies the SIGTERM stopSidecar sends as a managed stop", () => { + expect(classify({ stoppedByUs: true, signal: "SIGTERM" })).toEqual({ + kind: "managed-stop", + reason: "stopped-by-app", + }); + }); + + it("classifies any exit while the app is quitting as a managed stop", () => { + // Quit tears the whole process tree down; whichever signal wins the race, + // the sidecar going away is what we asked for. + expect(classify({ appQuitting: true, code: 130 })).toEqual({ + kind: "managed-stop", + reason: "app-quitting", + }); + expect(classify({ appQuitting: true, code: 1 })).toEqual({ + kind: "managed-stop", + reason: "app-quitting", + }); + }); + + it("classifies exit code 130 as an external shutdown rather than a crash", () => { + // 128+SIGINT. Nothing in the app sends SIGINT, so it came from outside + // (a group interrupt aimed at Electron) — an expected shutdown, not a crash. + expect(classify({ code: 130 })).toEqual({ + kind: "external-shutdown", + reason: "code 130", + }); + }); + + it("classifies a SIGINT signal exit as an external shutdown", () => { + expect(classify({ code: null, signal: "SIGINT" })).toEqual({ + kind: "external-shutdown", + reason: "SIGINT", + }); + }); + + it("classifies an unsolicited SIGTERM (code 143) as an external shutdown", () => { + expect(classify({ code: 143 })).toEqual({ + kind: "external-shutdown", + reason: "code 143", + }); + expect(classify({ signal: "SIGTERM" })).toEqual({ + kind: "external-shutdown", + reason: "SIGTERM", + }); + }); + + it("still classifies an ordinary non-zero exit as a crash", () => { + expect(classify({ code: 1 })).toEqual({ kind: "crash" }); + expect(classify({ code: 7 })).toEqual({ kind: "crash" }); + }); + + it("still classifies an abnormal death signal as a crash", () => { + expect(classify({ signal: "SIGSEGV" })).toEqual({ kind: "crash" }); + expect(classify({ code: 134 })).toEqual({ kind: "crash" }); + }); +}); diff --git a/apps/desktop/src/main/sidecar-exit.ts b/apps/desktop/src/main/sidecar-exit.ts new file mode 100644 index 000000000..758e251c0 --- /dev/null +++ b/apps/desktop/src/main/sidecar-exit.ts @@ -0,0 +1,50 @@ +/** + * Classification of a sidecar child exit that happened AFTER a successful boot. + * + * Pure so it can be unit-tested without Electron. Three outcomes, because the + * three deserve different treatment: + * + * managed-stop — we asked for it (stopSidecar, or the app is quitting). + * Log it; the UI is going away anyway. + * external-shutdown — something outside the app interrupted the child + * (a group SIGINT aimed at Electron is the common one, + * reported by Node as code 130 = 128+SIGINT). The server + * really is gone, so the window must say so, but this is + * a shutdown and not a crash — nothing to report upstream. + * crash — anything else. Reported with the stderr tail. + * + * The distinction matters both ways: 130 exits were drowning the crash channel, + * and a genuine post-boot death has to stay visible in it. + */ + +/** Signals that mean "stop", as opposed to "you are broken". */ +const SHUTDOWN_SIGNALS: ReadonlySet = new Set(["SIGINT", "SIGTERM", "SIGHUP"]); + +/** POSIX 128+N exit codes for those same signals. */ +const SHUTDOWN_EXIT_CODES: ReadonlySet = new Set([129, 130, 143]); + +export interface SidecarExitInput { + readonly code: number | null; + readonly signal: string | null; + /** True when `stopSidecar` signalled this child (quit, restart, update). */ + readonly stoppedByUs: boolean; + /** True once Electron has begun quitting. */ + readonly appQuitting: boolean; +} + +export type SidecarExitClassification = + | { readonly kind: "managed-stop"; readonly reason: "stopped-by-app" | "app-quitting" } + | { readonly kind: "external-shutdown"; readonly reason: string } + | { readonly kind: "crash" }; + +export const classifySidecarExit = (input: SidecarExitInput): SidecarExitClassification => { + if (input.stoppedByUs) return { kind: "managed-stop", reason: "stopped-by-app" }; + if (input.appQuitting) return { kind: "managed-stop", reason: "app-quitting" }; + if (input.signal !== null && SHUTDOWN_SIGNALS.has(input.signal)) { + return { kind: "external-shutdown", reason: input.signal }; + } + if (input.code !== null && SHUTDOWN_EXIT_CODES.has(input.code)) { + return { kind: "external-shutdown", reason: `code ${input.code}` }; + } + return { kind: "crash" }; +}; diff --git a/apps/desktop/src/main/sidecar.ts b/apps/desktop/src/main/sidecar.ts index b0f5bb7e7..fc9d80ec9 100644 --- a/apps/desktop/src/main/sidecar.ts +++ b/apps/desktop/src/main/sidecar.ts @@ -25,6 +25,8 @@ import { loadOrMintLocalAuthToken } from "./local-auth"; import { getServerSettings } from "./settings"; import { reportSidecarCrash, sidecarCrashReportingEnv } from "./diagnostics"; import { resolveSupervisedDaemonAttach } from "./supervised-daemon"; +import { classifySidecarExit } from "./sidecar-exit"; +import { isAppQuitting } from "./app-quit"; import { SERVER_SETTINGS_USERNAME, type DesktopServerSettings } from "../shared/server-settings"; // Sidecar output is echoed to the terminal (visible when Electron is run @@ -44,8 +46,12 @@ const expectedExits = new WeakSet(); // Main/index.ts subscribes to swap the dead web UI for the in-window crash // screen. A callback (not an import) keeps this module free of window // concerns. -let unexpectedExitListener: (() => void) | null = null; -export const onUnexpectedSidecarExit = (listener: () => void) => { +export interface SidecarExitNotice { + /** False for an expected shutdown, where nothing was sent upstream. */ + readonly reported: boolean; +} +let unexpectedExitListener: ((notice: SidecarExitNotice) => void) | null = null; +export const onUnexpectedSidecarExit = (listener: (notice: SidecarExitNotice) => void) => { unexpectedExitListener = listener; }; @@ -341,17 +347,34 @@ export async function startSidecar(options: StartOptions = {}): Promise { if (resolved) { - // Post-boot exit: expected when we stopped it ourselves (quit, - // restart, update); anything else is a sidecar crash under a live - // window — log it and report upstream with the stderr tail. - if (expectedExits.has(child)) { - sidecarLog.info(`exited (code=${code} signal=${signal})`); + // Post-boot exit. Expected when we stopped it ourselves (quit, restart, + // update) and when something outside the app interrupted it — the child + // shares Electron's process group, so a group-wide SIGINT reaches it + // directly and surfaces as code 130. Only the rest is a crash under a + // live window, reported upstream with the stderr tail. + const classification = classifySidecarExit({ + code, + signal, + stoppedByUs: expectedExits.has(child), + appQuitting: isAppQuitting(), + }); + if (classification.kind === "managed-stop") { + sidecarLog.info(`exited (code=${code} signal=${signal}) — ${classification.reason}`); + return; + } + if (classification.kind === "external-shutdown") { + // The server really is gone, so the window still has to say so; it + // just wasn't a crash, so nothing is reported. + sidecarLog.info( + `Sidecar shut down by ${classification.reason} (code=${code} signal=${signal})`, + ); + unexpectedExitListener?.({ reported: false }); return; } const message = `Sidecar exited unexpectedly (code=${code} signal=${signal})`; sidecarLog.error(message); reportSidecarCrash(message, stderrBuffer); - unexpectedExitListener?.(); + unexpectedExitListener?.({ reported: true }); return; } if (rejected) return; diff --git a/bun.lock b/bun.lock index 4aebc2673..6ada53f84 100644 --- a/bun.lock +++ b/bun.lock @@ -448,6 +448,7 @@ "react-dom": "catalog:", }, "devDependencies": { + "@effect/vitest": "catalog:", "@tailwindcss/vite": "catalog:", "@tanstack/router-plugin": "^1.167.12", "@tanstack/virtual-file-routes": "^1.162.0", @@ -457,6 +458,7 @@ "react-grab": "^0.1.31", "typescript": "catalog:", "vite": "catalog:", + "vitest": "catalog:", }, }, "packages/core/analytics": { diff --git a/packages/app/package.json b/packages/app/package.json index 6c1efde0f..9af106700 100644 --- a/packages/app/package.json +++ b/packages/app/package.json @@ -34,6 +34,7 @@ "react-dom": "catalog:" }, "devDependencies": { + "@effect/vitest": "catalog:", "@tailwindcss/vite": "catalog:", "@tanstack/router-plugin": "^1.167.12", "@tanstack/virtual-file-routes": "^1.162.0", @@ -42,6 +43,7 @@ "@vitejs/plugin-react": "catalog:", "react-grab": "^0.1.31", "typescript": "catalog:", - "vite": "catalog:" + "vite": "catalog:", + "vitest": "catalog:" } } diff --git a/packages/app/src/entry-client.tsx b/packages/app/src/entry-client.tsx index 29feed9e2..1c17a8bc9 100644 --- a/packages/app/src/entry-client.tsx +++ b/packages/app/src/entry-client.tsx @@ -4,9 +4,14 @@ import { RouterProvider } from "@tanstack/react-router"; import { bootstrapLocalAuthToken } from "@executor-js/react/api/local-auth"; import { getRouter } from "./router"; import { initDesktopCrashReporting } from "./crash-reporting"; +import { installServerDisconnectedRecovery } from "./server-disconnected"; import "@executor-js/react/globals.css"; initDesktopCrashReporting(); +// A route chunk that fails to load usually means the server serving this bundle +// went away (desktop: the sidecar exited). Recover instead of leaving a dead +// route behind an unhandled TypeError. +installServerDisconnectedRecovery(); if ("executor" in window && navigator.platform.includes("Mac")) { document.documentElement.classList.add("executor-desktop-macos"); diff --git a/packages/app/src/preload-error.test.ts b/packages/app/src/preload-error.test.ts new file mode 100644 index 000000000..1746a6d67 --- /dev/null +++ b/packages/app/src/preload-error.test.ts @@ -0,0 +1,132 @@ +import { describe, expect, it } from "@effect/vitest"; +import { + PRELOAD_ERROR_EVENT, + installPreloadErrorHandler, + respondToPreloadError, + type PreloadErrorEnvironment, +} from "./preload-error"; + +interface Recorder { + readonly env: PreloadErrorEnvironment; + readonly calls: { + probes: number; + disconnected: number; + reloads: number; + reported: unknown[]; + flagWrites: number; + }; +} + +const recorder = (options: { + readonly serverUp: readonly boolean[]; + readonly reloadFlag?: boolean; + readonly target?: EventTarget; +}): Recorder => { + const calls = { + probes: 0, + disconnected: 0, + reloads: 0, + reported: [] as unknown[], + flagWrites: 0, + }; + let flag = options.reloadFlag ?? false; + return { + calls, + env: { + target: options.target ?? new EventTarget(), + probeServer: async () => { + const answer = options.serverUp[calls.probes] ?? options.serverUp.at(-1) ?? false; + calls.probes += 1; + return answer; + }, + showDisconnected: () => { + calls.disconnected += 1; + }, + reload: () => { + calls.reloads += 1; + }, + report: (error) => { + calls.reported.push(error); + }, + readReloadFlag: () => flag, + writeReloadFlag: () => { + flag = true; + calls.flagWrites += 1; + }, + delay: async () => {}, + }, + }; +}; + +// oxlint-disable-next-line executor/no-error-constructor -- test boundary: reproduces the exact TypeError the browser hands to vite:preloadError +const chunkError = new TypeError( + "Failed to fetch dynamically imported module: http://127.0.0.1:4789/assets/route-abc123.js", +); + +describe("respondToPreloadError", () => { + it("shows the reconnect surface and never reports when the server is unreachable", async () => { + // The sidecar died under a live window: the chunk fetch failing is a + // symptom of that one event, already covered by the sidecar-exit path. + const { env, calls } = recorder({ serverUp: [false, true] }); + + const outcome = await respondToPreloadError(env, chunkError); + + expect(outcome).toBe("reconnected"); + expect(calls.disconnected).toBe(1); + expect(calls.reported).toEqual([]); + expect(calls.reloads).toBe(1); + expect(calls.flagWrites).toBe(0); + }); + + it("reloads exactly once when the server answers and the chunk is genuinely stale", async () => { + const { env, calls } = recorder({ serverUp: [true] }); + + const outcome = await respondToPreloadError(env, chunkError); + + expect(outcome).toBe("reloaded"); + expect(calls.reloads).toBe(1); + expect(calls.flagWrites).toBe(1); + expect(calls.disconnected).toBe(0); + expect(calls.reported).toEqual([]); + }); + + it("reports instead of looping when a reload already happened this session", async () => { + const { env, calls } = recorder({ serverUp: [true], reloadFlag: true }); + + const outcome = await respondToPreloadError(env, chunkError); + + expect(outcome).toBe("reported"); + expect(calls.reloads).toBe(0); + expect(calls.reported).toEqual([chunkError]); + }); +}); + +describe("installPreloadErrorHandler", () => { + it("prevents the default rejection so a dead chunk is not an unhandled TypeError", () => { + const target = new EventTarget(); + const { env, calls } = recorder({ serverUp: [false, true], target }); + installPreloadErrorHandler(env); + + const event = new CustomEvent(PRELOAD_ERROR_EVENT, { + cancelable: true, + detail: { payload: chunkError }, + }); + target.dispatchEvent(event); + + expect(event.defaultPrevented).toBe(true); + expect(calls.probes).toBe(1); + }); + + it("stops responding after dispose", () => { + const target = new EventTarget(); + const { env, calls } = recorder({ serverUp: [false, true], target }); + const dispose = installPreloadErrorHandler(env); + dispose(); + + target.dispatchEvent( + new CustomEvent(PRELOAD_ERROR_EVENT, { cancelable: true, detail: { payload: chunkError } }), + ); + + expect(calls.probes).toBe(0); + }); +}); diff --git a/packages/app/src/preload-error.ts b/packages/app/src/preload-error.ts new file mode 100644 index 000000000..78bbac7d2 --- /dev/null +++ b/packages/app/src/preload-error.ts @@ -0,0 +1,90 @@ +/** + * Handling for `vite:preloadError` — a route's lazy chunk failing to load. + * + * Two very different causes produce the same `TypeError: Failed to fetch + * dynamically imported module`: + * + * 1. The server serving this bundle went away (in the desktop app, the + * sidecar exited under a live window). Every subsequent chunk fails too. + * Nothing is broken in the app — it is the same event the main process + * already surfaces as "server disconnected", so this must show the + * reconnect surface and report nothing. + * 2. The server is up but the chunk hash is stale, because a deploy replaced + * the bundle under a long-lived tab. One reload fixes it. + * + * A health probe tells them apart. Without this handler the rejection is + * unhandled, so the user gets a dead route and the crash channel gets a + * TypeError for what is usually an ordinary shutdown. + */ + +export const PRELOAD_ERROR_EVENT = "vite:preloadError"; + +/** How long to wait between health probes while the server is unreachable. */ +const RECONNECT_POLL_MS = 1_000; + +export interface PreloadErrorEnvironment { + readonly target: EventTarget; + /** Resolves true when the server answers its health endpoint. */ + readonly probeServer: () => Promise; + /** Render the "lost connection, reconnecting" surface. */ + readonly showDisconnected: () => void; + readonly reload: () => void; + /** Report a chunk failure that a reload could not resolve. */ + readonly report: (error: unknown) => void; + /** Has this session already spent its one automatic reload? */ + readonly readReloadFlag: () => boolean; + readonly writeReloadFlag: () => void; + readonly delay: (ms: number) => Promise; +} + +export type PreloadErrorOutcome = + /** Server was down; waited for it and reloaded once it answered. */ + | "reconnected" + /** Server was up, so the chunk was stale; reloaded once. */ + | "reloaded" + /** A reload already happened this session and did not help; reported. */ + | "reported"; + +export const respondToPreloadError = async ( + env: PreloadErrorEnvironment, + error: unknown, +): Promise => { + if (await env.probeServer()) { + // The origin answers, so this is a stale chunk rather than a dead server. + // Exactly one reload per session, so a chunk that stays missing cannot + // turn into a reload loop — it gets reported instead. + if (env.readReloadFlag()) { + env.report(error); + return "reported"; + } + env.writeReloadFlag(); + env.reload(); + return "reloaded"; + } + + env.showDisconnected(); + for (;;) { + await env.delay(RECONNECT_POLL_MS); + if (await env.probeServer()) { + env.reload(); + return "reconnected"; + } + } +}; + +/** Vite dispatches the failure as `CustomEvent<{ payload: Error }>`. */ +const preloadErrorPayload = (event: Event): unknown => + (event as CustomEvent<{ readonly payload?: unknown }>).detail?.payload; + +/** Registers the handler; returns a disposer. */ +export const installPreloadErrorHandler = (env: PreloadErrorEnvironment): (() => void) => { + const handler = (event: Event) => { + // Always claim the event: Vite rethrows an unclaimed preload error into the + // page as an unhandled rejection, which is the crash report we are here to + // replace with a real recovery path. + event.preventDefault(); + void respondToPreloadError(env, preloadErrorPayload(event)); + }; + env.target.addEventListener(PRELOAD_ERROR_EVENT, handler); + return () => env.target.removeEventListener(PRELOAD_ERROR_EVENT, handler); +}; diff --git a/packages/app/src/server-disconnected.test.ts b/packages/app/src/server-disconnected.test.ts new file mode 100644 index 000000000..ad3d7c84b --- /dev/null +++ b/packages/app/src/server-disconnected.test.ts @@ -0,0 +1,184 @@ +/** + * The recovery policy is tested against a fake environment in + * preload-error.test.ts; this covers the environment the browser actually gets, + * because a policy fed the wrong inputs recovers from nothing. The failure this + * guards against is silent: a probe that answers backwards, or a reload guard + * that never latches, turns "reload once" into a reload loop with every + * behavioural test still green. + */ + +import { afterEach, describe, expect, it } from "@effect/vitest"; +import { browserEnvironment } from "./server-disconnected"; + +const BROWSER_GLOBALS = ["window", "document", "fetch", "reportError"] as const; +const originalGlobals = new Map( + BROWSER_GLOBALS.map((key) => [key, (globalThis as Record)[key]]), +); + +afterEach(() => { + // The fakes below are installed on the real global object; leaving them there + // would silently hand the next test a browser that does not exist. + for (const [key, value] of originalGlobals) { + if (value === undefined) delete (globalThis as Record)[key]; + else (globalThis as Record)[key] = value; + } +}); + +class FakeElement { + id = ""; + textContent = ""; + readonly style = { cssText: "" }; + readonly children: FakeElement[] = []; + + append(...nodes: FakeElement[]): void { + this.children.push(...nodes); + } + + byId(id: string): FakeElement | null { + if (this.id === id) return this; + for (const child of this.children) { + const found = child.byId(id); + if (found) return found; + } + return null; + } + + get text(): string { + return [this.textContent, ...this.children.map((child) => child.text)].join(" ").trim(); + } +} + +interface FetchCall { + readonly input: unknown; + readonly init: RequestInit | undefined; +} + +const environment = (options: { + readonly respond?: () => Promise<{ ok: boolean; body: string }>; + readonly storage?: "working" | "blocked"; +}) => { + const body = new FakeElement(); + const fetches: FetchCall[] = []; + const reloads = { count: 0 }; + const reported: unknown[] = []; + const store = new Map(); + + const sessionStorage = { + getItem: (key: string) => store.get(key) ?? null, + setItem: (key: string, value: string) => store.set(key, value), + }; + + const fakeWindow = { + addEventListener: () => {}, + removeEventListener: () => {}, + location: { + reload: () => { + reloads.count += 1; + }, + }, + }; + if (options.storage === "blocked") { + Object.defineProperty(fakeWindow, "sessionStorage", { + get: () => { + // oxlint-disable-next-line executor/no-error-constructor, executor/no-try-catch-or-throw -- boundary: reproduces the SecurityError a browser raises when storage is blocked + throw new Error("SecurityError: storage is disabled"); + }, + }); + } else { + Object.defineProperty(fakeWindow, "sessionStorage", { get: () => sessionStorage }); + } + + const globals = globalThis as Record; + globals.window = fakeWindow; + globals.document = { + body, + createElement: () => new FakeElement(), + getElementById: (id: string) => body.byId(id), + }; + globals.fetch = async (input: unknown, init?: RequestInit) => { + fetches.push({ input, init }); + const answer = await (options.respond?.() ?? Promise.resolve({ ok: true, body: "ok" })); + return { ok: answer.ok, text: async () => answer.body }; + }; + globals.reportError = (error: unknown) => reported.push(error); + + return { env: browserEnvironment(), body, fetches, reloads, reported, store }; +}; + +describe("browserEnvironment", () => { + it("probes the unauthenticated health endpoint and believes only a healthy answer", async () => { + const healthy = environment({ respond: async () => ({ ok: true, body: "ok\n" }) }); + expect(await healthy.env.probeServer()).toBe(true); + expect(healthy.fetches).toHaveLength(1); + expect(healthy.fetches[0]?.input).toBe("/api/health"); + // A cached 200 from before the server died would report it as alive. + expect(healthy.fetches[0]?.init?.cache).toBe("no-store"); + + const wrongBody = environment({ respond: async () => ({ ok: true, body: "" }) }); + expect(await wrongBody.env.probeServer()).toBe(false); + + const failing = environment({ respond: async () => ({ ok: false, body: "" }) }); + expect(await failing.env.probeServer()).toBe(false); + }); + + it("reads a refused connection as a down server rather than propagating", async () => { + const down = environment({ + respond: async () => { + // oxlint-disable-next-line executor/no-error-constructor, executor/no-try-catch-or-throw -- boundary: fetch rejects when the origin is gone, which is the signal under test + throw new TypeError("Failed to fetch"); + }, + }); + + expect(await down.env.probeServer()).toBe(false); + }); + + it("shows one reconnect overlay however many times it is asked", () => { + const { env, body } = environment({}); + + env.showDisconnected(); + env.showDisconnected(); + + const overlays = body.children.filter((child) => child.id === "executor-server-disconnected"); + expect(overlays).toHaveLength(1); + expect(overlays[0]?.text).toContain("Lost connection to the Executor server"); + expect(overlays[0]?.text).toContain("RECONNECTING"); + }); + + it("latches the one-reload-per-session guard in storage", () => { + const { env, store } = environment({}); + + expect(env.readReloadFlag()).toBe(false); + env.writeReloadFlag(); + + expect(env.readReloadFlag()).toBe(true); + expect([...store.keys()]).toEqual(["executor:preload-reloaded"]); + }); + + it("reports rather than reloads when storage is unavailable", () => { + // Without a place to record the reload, an auto-reload could repeat + // forever; reading the flag as already spent degrades to reporting once. + const { env } = environment({ storage: "blocked" }); + + expect(env.readReloadFlag()).toBe(true); + }); + + it("reloads the page and reports through the shared handled-error reporter", () => { + const { env, reloads, reported } = environment({}); + // oxlint-disable-next-line executor/no-error-constructor -- test boundary: stands in for the chunk TypeError + const failure = new TypeError("Failed to fetch dynamically imported module"); + + env.reload(); + env.report(failure); + + expect(reloads.count).toBe(1); + // Reporting the raw TypeError would file this as an unhandled crash with no + // surface attached; it must arrive as the normalized handled-error envelope. + expect(reported).toHaveLength(1); + const report = reported[0] as { readonly context?: unknown; readonly cause?: unknown }; + expect(report.context).toMatchObject({ + surface: "renderer", + action: "load-route-chunk", + }); + expect(report.cause).toBe(failure); + }); +}); diff --git a/packages/app/src/server-disconnected.ts b/packages/app/src/server-disconnected.ts new file mode 100644 index 000000000..c1cde47bb --- /dev/null +++ b/packages/app/src/server-disconnected.ts @@ -0,0 +1,113 @@ +/** + * Browser wiring for the "server went away" recovery path. + * + * Owns the DOM half that `preload-error.ts` deliberately does not: the health + * probe against the origin serving this bundle, and the reconnect overlay shown + * while it is unreachable. The overlay is plain DOM with inline styles rather + * than a React surface because it has to render when the app's own route chunks + * can no longer be fetched. + */ + +import { installPreloadErrorHandler, type PreloadErrorEnvironment } from "./preload-error"; +import { reportRendererHandledError } from "./crash-reporting"; + +const OVERLAY_ID = "executor-server-disconnected"; +const RELOAD_FLAG_KEY = "executor:preload-reloaded"; + +/** The unauthenticated health endpoint every Executor server exposes. */ +const probeServer = async (): Promise => { + // oxlint-disable-next-line executor/no-try-catch-or-throw, executor/no-raw-fetch -- boundary: fetch rejects when the server is down, which is precisely the signal being read + try { + const response = await fetch("/api/health", { cache: "no-store" }); + return response.ok && (await response.text()).trim() === "ok"; + } catch { + return false; + } +}; + +const showDisconnectedOverlay = (): void => { + if (document.getElementById(OVERLAY_ID)) return; + const overlay = document.createElement("div"); + overlay.id = OVERLAY_ID; + overlay.style.cssText = [ + "position:fixed", + "inset:0", + "z-index:2147483647", + "display:grid", + "place-items:center", + "gap:0.5rem", + "background:light-dark(#f7f7f4,#0a0a0a)", + "color:light-dark(#18181b,#fafafa)", + "color-scheme:light dark", + "font-family:ui-sans-serif,-apple-system,BlinkMacSystemFont,'Segoe UI',sans-serif", + "text-align:center", + ].join(";"); + + const title = document.createElement("div"); + title.textContent = "Lost connection to the Executor server"; + title.style.cssText = "font-size:0.95rem;font-weight:600"; + + const detail = document.createElement("div"); + detail.textContent = "RECONNECTING…"; + detail.style.cssText = [ + "font-family:ui-monospace,SFMono-Regular,Menlo,monospace", + "font-size:0.72rem", + "letter-spacing:0.08em", + "opacity:0.6", + ].join(";"); + + const stack = document.createElement("div"); + stack.style.cssText = "display:grid;gap:0.5rem;justify-items:center"; + stack.append(title, detail); + overlay.append(stack); + document.body.append(overlay); +}; + +const sessionFlag = (): Pick | null => { + // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: sessionStorage throws when storage is blocked or full + try { + return window.sessionStorage; + } catch { + return null; + } +}; + +/** + * The real browser half of the environment `respondToPreloadError` consumes. + * Exported so a test can check the mapping itself — an inverted probe or a + * reload guard that never latches would leave every behavioural test green + * while the app reload-loops. + */ +export const browserEnvironment = (): PreloadErrorEnvironment => { + const storage = sessionFlag(); + return { + target: window, + probeServer, + showDisconnected: showDisconnectedOverlay, + reload: () => window.location.reload(), + // Routed through the same reporter the rest of the UI uses, so a chunk that + // stays missing with a healthy server arrives as a HANDLED error carrying + // its surface and action — not as a bare global `reportError`, which the + // crash reporter files as an unhandled crash with that context dropped. + report: (error) => + reportRendererHandledError(error, { + surface: "renderer", + action: "load-route-chunk", + message: "A route chunk stayed unreachable with the server answering", + }), + // Without storage the guard degrades to "never auto-reload", which is the + // safe direction: a missing chunk gets reported instead of looping. + readReloadFlag: () => (storage ? storage.getItem(RELOAD_FLAG_KEY) !== null : true), + writeReloadFlag: () => storage?.setItem(RELOAD_FLAG_KEY, "1"), + delay: (ms) => new Promise((resolve) => setTimeout(resolve, ms)), + }; +}; + +/** + * Install the preload-error recovery path. Safe to call outside a browser + * (SSR/tests), where it is a no-op. + */ +export const installServerDisconnectedRecovery = (): void => { + if (typeof window === "undefined") return; + installPreloadErrorHandler(browserEnvironment()); +};