From 8e5030028f19b5325f3ef61379f1114082d71d42 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 2 Aug 2026 01:03:47 -0700 Subject: [PATCH] fix(editor): clean up failed lazy chunk reload requests --- src/preload/index.ts | 30 +++---- src/preload/renderer-restart-wiring.test.ts | 65 ++++++++------- src/preload/renderer-restart-wiring.ts | 39 +++++++++ .../src/components/editor/EditorContent.tsx | 3 - ...hMarkdownErrorBoundary.lazy-chunk.test.tsx | 28 ------- .../editor/RichMarkdownErrorBoundary.tsx | 12 +-- ...bleRenderErrorBoundary.lazy-chunk.test.tsx | 2 - .../src/lib/lazy-chunk-recovery-reload.ts | 80 +++++++++---------- ...h-retry.right-sidebar-syntax-error.test.ts | 30 ------- src/renderer/src/lib/lazy-with-retry.test.ts | 71 ++++++++-------- src/renderer/src/lib/lazy-with-retry.ts | 52 +++++------- 11 files changed, 181 insertions(+), 231 deletions(-) create mode 100644 src/preload/renderer-restart-wiring.ts diff --git a/src/preload/index.ts b/src/preload/index.ts index 63817ee6b12..3c3fd23ef70 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -236,10 +236,8 @@ import type { AgentType } from '../shared/native-chat-types' import { ORCA_APP_RESTART_ABORTED_EVENT, ORCA_APP_RESTART_STARTED_EVENT, - ORCA_UPDATER_QUIT_AND_INSTALL_ABORTED_EVENT, - ORCA_UPDATER_QUIT_AND_INSTALL_STARTED_EVENT + ORCA_UPDATER_QUIT_AND_INSTALL_ABORTED_EVENT } from '../shared/updater-renderer-events' -import { ORCA_RENDERER_UNLOAD_PREVENTED_EVENT } from '../shared/renderer-shutdown-events' import { ORCA_INTERNAL_FILE_DRAG_TYPE, createNativeFileDropPayload, @@ -279,6 +277,11 @@ import { createUpdaterQuitAbortRelay, prepareRendererForAppRestart } from '../shared/renderer-restart-preparation' +import { + prepareAndInvokeUpdaterInstall, + relayRendererUnloadPrevented, + relayUpdaterStatus +} from './renderer-restart-wiring' type NativeFileDropCallback = (data: NativeFileDropPayload) => void @@ -290,10 +293,10 @@ const updaterQuitAbortRelay = createUpdaterQuitAbortRelay( ) ipcRenderer.on('updater:status', (_event, status: UpdateStatus) => { - updaterQuitAbortRelay.handleStatus(status) + relayUpdaterStatus(updaterQuitAbortRelay, status) }) ipcRenderer.on('window:unload-prevented', () => { - window.dispatchEvent(new Event(ORCA_RENDERER_UNLOAD_PREVENTED_EVENT)) + relayRendererUnloadPrevented(window) }) function getLinuxDisplayServer(): 'wayland' | 'x11' | null { @@ -2958,19 +2961,10 @@ const api = { dismissNudge: () => ipcRenderer.invoke('updater:dismissNudge'), dismissAvailableUpdate: () => ipcRenderer.invoke('updater:dismissAvailableUpdate'), listBuilds: (channel) => ipcRenderer.invoke('updater:listBuilds', channel), - quitAndInstall: async (): Promise => { - await prepareRendererForAppRestart(window, { - startedEventName: ORCA_UPDATER_QUIT_AND_INSTALL_STARTED_EVENT, - abortedEventName: ORCA_UPDATER_QUIT_AND_INSTALL_ABORTED_EVENT - }) - updaterQuitAbortRelay.markPrepared() - try { - return await ipcRenderer.invoke('updater:quitAndInstall') - } catch (error) { - updaterQuitAbortRelay.abort() - throw error - } - }, + quitAndInstall: (): Promise => + prepareAndInvokeUpdaterInstall(window, updaterQuitAbortRelay, () => + ipcRenderer.invoke('updater:quitAndInstall') + ), onStatus: (callback) => { const listener = (_event: Electron.IpcRendererEvent, status: UpdateStatus) => callback(status) ipcRenderer.on('updater:status', listener) diff --git a/src/preload/renderer-restart-wiring.test.ts b/src/preload/renderer-restart-wiring.test.ts index 7fa0e8b5aed..048da8074d8 100644 --- a/src/preload/renderer-restart-wiring.test.ts +++ b/src/preload/renderer-restart-wiring.test.ts @@ -1,36 +1,45 @@ -import { readFileSync } from 'node:fs' -import { join } from 'node:path' -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' +import { ORCA_RENDERER_UNLOAD_PREVENTED_EVENT } from '../shared/renderer-shutdown-events' +import { ORCA_UPDATER_QUIT_AND_INSTALL_STARTED_EVENT } from '../shared/updater-renderer-events' +import { + prepareAndInvokeUpdaterInstall, + relayRendererUnloadPrevented, + relayUpdaterStatus +} from './renderer-restart-wiring' -describe('preload restart wiring', () => { - const source = readFileSync(join(process.cwd(), 'src/preload/index.ts'), 'utf8') +describe('renderer restart wiring', () => { + it('relays updater status and prevented unload events', () => { + const eventTarget = new EventTarget() + const unloadPrevented = vi.fn() + const handleStatus = vi.fn() + eventTarget.addEventListener(ORCA_RENDERER_UNLOAD_PREVENTED_EVENT, unloadPrevented) - it('relays prevented unload and async updater failure IPC into renderer lifecycle events', () => { - expect(source).toContain("ipcRenderer.on('updater:status'") - expect(source).toContain('updaterQuitAbortRelay.handleStatus(status)') - expect(source).toContain("ipcRenderer.on('window:unload-prevented'") - expect(source).toContain( - 'window.dispatchEvent(new Event(ORCA_RENDERER_UNLOAD_PREVENTED_EVENT))' - ) + relayUpdaterStatus({ handleStatus }, { state: 'error', message: 'install failed' }) + relayRendererUnloadPrevented(eventTarget) + + expect(handleStatus).toHaveBeenCalledWith({ state: 'error', message: 'install failed' }) + expect(unloadPrevented).toHaveBeenCalledTimes(1) }) - it('marks updater preparation before invoking main and aborts it on immediate IPC failure', () => { - const start = source.indexOf('quitAndInstall: async (): Promise => {') - const end = source.indexOf('onStatus: (callback) => {', start) - expect(start).toBeGreaterThanOrEqual(0) - expect(end).toBeGreaterThan(start) - const block = source.slice(start, end) - const prepare = block.indexOf('await prepareRendererForAppRestart(window, {') - const markPrepared = block.indexOf('updaterQuitAbortRelay.markPrepared()') - const invoke = block.indexOf("ipcRenderer.invoke('updater:quitAndInstall')") - const abort = block.indexOf('updaterQuitAbortRelay.abort()') + it('marks preparation before invoking main and aborts on IPC failure', async () => { + const eventTarget = new EventTarget() + const calls: string[] = [] + eventTarget.addEventListener(ORCA_UPDATER_QUIT_AND_INSTALL_STARTED_EVENT, () => { + calls.push('prepared') + }) + const relay = { + markPrepared: () => calls.push('marked'), + abort: () => calls.push('aborted') + } + const invoke = vi.fn(async () => { + calls.push('invoked') + throw new Error('IPC failed') + }) - expect(prepare).toBeGreaterThanOrEqual(0) - expect(markPrepared).toBeGreaterThan(prepare) - expect(invoke).toBeGreaterThan(markPrepared) - expect(abort).toBeGreaterThan(invoke) - expect(block).toMatch( - /try \{\s*return await ipcRenderer\.invoke\('updater:quitAndInstall'\)\s*\} catch \(error\) \{\s*updaterQuitAbortRelay\.abort\(\)\s*throw error\s*\}/ + await expect(prepareAndInvokeUpdaterInstall(eventTarget, relay, invoke)).rejects.toThrow( + 'IPC failed' ) + + expect(calls).toEqual(['prepared', 'marked', 'invoked', 'aborted']) }) }) diff --git a/src/preload/renderer-restart-wiring.ts b/src/preload/renderer-restart-wiring.ts new file mode 100644 index 00000000000..80770c52a3f --- /dev/null +++ b/src/preload/renderer-restart-wiring.ts @@ -0,0 +1,39 @@ +import { ORCA_RENDERER_UNLOAD_PREVENTED_EVENT } from '../shared/renderer-shutdown-events' +import { + prepareRendererForAppRestart, + type UpdaterQuitAbortRelay +} from '../shared/renderer-restart-preparation' +import type { UpdateStatus } from '../shared/types' +import { + ORCA_UPDATER_QUIT_AND_INSTALL_ABORTED_EVENT, + ORCA_UPDATER_QUIT_AND_INSTALL_STARTED_EVENT +} from '../shared/updater-renderer-events' + +export function relayUpdaterStatus( + relay: Pick, + status: UpdateStatus +): void { + relay.handleStatus(status) +} + +export function relayRendererUnloadPrevented(eventTarget: EventTarget): void { + eventTarget.dispatchEvent(new Event(ORCA_RENDERER_UNLOAD_PREVENTED_EVENT)) +} + +export async function prepareAndInvokeUpdaterInstall( + eventTarget: EventTarget, + relay: Pick, + invoke: () => Promise +): Promise { + await prepareRendererForAppRestart(eventTarget, { + startedEventName: ORCA_UPDATER_QUIT_AND_INSTALL_STARTED_EVENT, + abortedEventName: ORCA_UPDATER_QUIT_AND_INSTALL_ABORTED_EVENT + }) + relay.markPrepared() + try { + await invoke() + } catch (error) { + relay.abort() + throw error + } +} diff --git a/src/renderer/src/components/editor/EditorContent.tsx b/src/renderer/src/components/editor/EditorContent.tsx index 3fd78ba01e9..52b079dc526 100644 --- a/src/renderer/src/components/editor/EditorContent.tsx +++ b/src/renderer/src/components/editor/EditorContent.tsx @@ -33,9 +33,6 @@ import { ExternalFileChangeBanner } from './ExternalFileChangeBanner' const MonacoEditor = lazy(() => import('./MonacoEditor')) const DiffViewer = lazy(() => import('./DiffViewer')) const CombinedDiffViewer = lazy(() => import('./CombinedDiffViewer')) -// Why keyed: `reloadKey` rides on LazyChunkLoadError and on the vetoed-reload -// breadcrumb, so the report names this chunk instead of one of the eight lazy sites -// below. b860def2's own pre-reload crumb had already been evicted from the ring. const RichMarkdownEditor = lazy(() => import('./RichMarkdownEditor'), { reloadKey: 'rich-markdown-editor' }) diff --git a/src/renderer/src/components/editor/RichMarkdownErrorBoundary.lazy-chunk.test.tsx b/src/renderer/src/components/editor/RichMarkdownErrorBoundary.lazy-chunk.test.tsx index ff19cc89915..a9ac63ac6b0 100644 --- a/src/renderer/src/components/editor/RichMarkdownErrorBoundary.lazy-chunk.test.tsx +++ b/src/renderer/src/components/editor/RichMarkdownErrorBoundary.lazy-chunk.test.tsx @@ -1,7 +1,5 @@ // @vitest-environment happy-dom -import { readFileSync } from 'node:fs' -import { join } from 'node:path' import { Suspense, act, type ReactElement, type ReactNode } from 'react' import { createRoot, type Root } from 'react-dom/client' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' @@ -24,8 +22,6 @@ vi.mock('@/lib/crash-breadcrumb-recorder', () => ({ })) const RELOAD_GUARD_KEY = 'orca:lazy-chunk-reload-attempted' -// A guard left by a *different* document is what proves the reload landed; only then -// does lazy-with-retry produce the sentinel this boundary suppresses. const LANDED_RELOAD_GUARD_VALUE = 'doc-before-the-reload' globalThis.IS_REACT_ACT_ENVIRONMENT = true @@ -36,8 +32,6 @@ function createContainer(): { container: HTMLDivElement; root: Root } { return { container, root: createRoot(container) } } -// Mirrors EditorContent: the Suspense sits above the boundary, which EditorContent -// re-keys per open file (`key={viewStateScopeId}`), and which wraps the lazy editor. function BoundaryHarness({ children, boundaryKey = 'pane-a' @@ -60,8 +54,6 @@ async function flushReactWork(): Promise { }) } -// The parse error a corrupt rich-editor chunk rejects with; reused across tests to -// prove the module-scoped dedupe is reset rather than dodged by unique messages. const CORRUPT_CHUNK_PARSE_ERROR = "Unexpected token ':'" describe('RichMarkdownErrorBoundary lazy chunk containment', () => { @@ -72,7 +64,6 @@ describe('RichMarkdownErrorBoundary lazy chunk containment', () => { beforeEach(() => { reportCrashMock.mockReset() recordBreadcrumbMock.mockReset() - // Dedupe is module-scoped so it can outlive boundary remounts; reset it per test. clearLazyChunkBreadcrumbDedupeForTest() window.sessionStorage.clear() consoleError = vi.spyOn(console, 'error').mockImplementation(() => undefined) @@ -89,9 +80,6 @@ describe('RichMarkdownErrorBoundary lazy chunk containment', () => { consoleError.mockRestore() }) - // The only honest sentinel case: a landed reload already refetched the chunk and it - // still fails to parse, so the report is not actionable. (b860def2 itself is NOT this - // case — its reload was vetoed, so lazy-with-retry now re-throws the real error.) it('renders the fallback without reporting after guarded dynamic import exhaustion', async () => { window.sessionStorage.setItem(RELOAD_GUARD_KEY, LANDED_RELOAD_GUARD_VALUE) const LazyRejectingImport = lazyWithRetry( @@ -112,7 +100,6 @@ describe('RichMarkdownErrorBoundary lazy chunk containment', () => { expect(container?.textContent).toContain('rich markdown editor') expect(reportCrashMock).not.toHaveBeenCalled() - // The suppressed report was the only artifact naming this surface; keep evidence. expect(recordBreadcrumbMock).toHaveBeenCalledWith('lazy_chunk_boundary_degraded', { boundaryId: 'editor.rich-markdown', reloadKey: 'rich-markdown-editor', @@ -120,9 +107,6 @@ describe('RichMarkdownErrorBoundary lazy chunk containment', () => { }) }) - // The breadcrumb ring holds 30 entries and does not coalesce this name, so an - // impatient Retry streak on a permanently rejected chunk would erase the trail. - // Reuses the previous test's error to pin that the dedupe set is actually cleared. it('records the degraded breadcrumb once across repeated Retry clicks', async () => { window.sessionStorage.setItem(RELOAD_GUARD_KEY, LANDED_RELOAD_GUARD_VALUE) const LazyRejectingImport = lazyWithRetry( @@ -154,8 +138,6 @@ describe('RichMarkdownErrorBoundary lazy chunk containment', () => { expect(recordBreadcrumbMock).toHaveBeenCalledTimes(1) }) - // EditorContent re-keys the boundary per open file, so instance-scoped dedupe would - // re-record on every markdown tab switch — the same ring flush the Retry streak causes. it('records the degraded breadcrumb once across boundary remounts', async () => { window.sessionStorage.setItem(RELOAD_GUARD_KEY, LANDED_RELOAD_GUARD_VALUE) const LazyRejectingImport = lazyWithRetry( @@ -206,14 +188,4 @@ describe('RichMarkdownErrorBoundary lazy chunk containment', () => { }) ) }) - - // b860def2's pre-crash breadcrumb read `reloadKey: "unknown"`, which cannot tell - // the rich editor chunk apart from the seven other lazy chunks in EditorContent. - it('names the rich editor chunk at the EditorContent lazy call site', () => { - const source = readFileSync( - join(process.cwd(), 'src/renderer/src/components/editor/EditorContent.tsx'), - 'utf8' - ) - expect(source).toContain("reloadKey: 'rich-markdown-editor'") - }) }) diff --git a/src/renderer/src/components/editor/RichMarkdownErrorBoundary.tsx b/src/renderer/src/components/editor/RichMarkdownErrorBoundary.tsx index 71f0f9bba79..7ca68fd9415 100644 --- a/src/renderer/src/components/editor/RichMarkdownErrorBoundary.tsx +++ b/src/renderer/src/components/editor/RichMarkdownErrorBoundary.tsx @@ -14,12 +14,7 @@ type State = { fileId: string } -// Module-scoped, not per instance: EditorContent re-keys this boundary per open file, -// so an instance field would re-record on every markdown tab switch and flush the -// 30-entry breadcrumb ring that this very evidence lives in. The key is the cause's -// name+message, which embeds per-chunk URLs for fetch failures, so a session that -// exhausts recovery across many lazy sites can accumulate dozens of entries — small -// and bounded by distinct failures, not by one. +// Module scope deduplicates breadcrumbs across file-keyed boundary remounts. const recordedLazyChunkCauses = new Set() export function clearLazyChunkBreadcrumbDedupeForTest(): void { @@ -60,17 +55,12 @@ export class RichMarkdownErrorBoundary extends React.Component { componentDidCatch(error: Error, info: React.ErrorInfo): void { console.error('[RichMarkdownEditor] render crash contained by boundary', error, info) - // Retries and the one guarded reload are already spent, so a crash report is not - // actionable — but it was the only artifact naming this surface, so leave a - // breadcrumb. Fallback still renders (matches RecoverableRenderErrorBoundary). if (isLazyChunkLoadError(error)) { const cause = describeLazyChunkCause(error) if (!recordedLazyChunkCauses.has(cause)) { recordedLazyChunkCauses.add(cause) recordRendererCrashBreadcrumb('lazy_chunk_boundary_degraded', { boundaryId: 'editor.rich-markdown', - // Carried on the error itself: the pre-reload crumb naming the chunk may - // already have been evicted from the 30-entry ring (crash b860def2). reloadKey: error.reloadKey, cause }) diff --git a/src/renderer/src/components/error-boundaries/RecoverableRenderErrorBoundary.lazy-chunk.test.tsx b/src/renderer/src/components/error-boundaries/RecoverableRenderErrorBoundary.lazy-chunk.test.tsx index b6b19b2ec1f..ca84d98b744 100644 --- a/src/renderer/src/components/error-boundaries/RecoverableRenderErrorBoundary.lazy-chunk.test.tsx +++ b/src/renderer/src/components/error-boundaries/RecoverableRenderErrorBoundary.lazy-chunk.test.tsx @@ -14,8 +14,6 @@ vi.mock('@/lib/react-error-boundary-reporting', () => ({ })) const RELOAD_GUARD_KEY = 'orca:lazy-chunk-reload-attempted' -// Only a guard left by a *different* document proves the reload landed, which is -// what makes lazy-with-retry emit the sentinel this boundary suppresses. const LANDED_RELOAD_GUARD_VALUE = 'doc-before-the-reload' globalThis.IS_REACT_ACT_ENVIRONMENT = true diff --git a/src/renderer/src/lib/lazy-chunk-recovery-reload.ts b/src/renderer/src/lib/lazy-chunk-recovery-reload.ts index fc89ab66fac..5bc259a7ae4 100644 --- a/src/renderer/src/lib/lazy-chunk-recovery-reload.ts +++ b/src/renderer/src/lib/lazy-chunk-recovery-reload.ts @@ -5,16 +5,7 @@ import { ORCA_APP_RESTART_STARTED_EVENT } from '../../../shared/updater-renderer-events' -/** - * Why: `window.location.reload()` alone cannot recover a corrupt lazy chunk in an - * editor app. Terminal's beforeunload handler preventDefault()s whenever any open - * file is dirty, and Electron's will-prevent-unload cancels the navigation with no - * dialog — so the recovery reload for crash b860def2 was requested and silently - * dropped, leaving the pane dead for 44 minutes. The updater already solved this: - * back up hot-exit buffers, take the one synchronous session checkpoint, and latch - * `isIntentionalAppRestartInProgress()` so the dirty-tab guard stands down. This - * runs that same sequence for chunk recovery. - */ +// Bare reloads are vetoed by dirty tabs; restart preparation backs them up first. export type LazyChunkRecoveryReloadOutcome = /** Hot-exit backup or the session checkpoint refused; unsaved work stays put. */ @@ -23,36 +14,40 @@ export type LazyChunkRecoveryReloadOutcome = | 'unload-vetoed' /** No veto signal, but the document outlived the grace window. */ | 'never-landed' + /** The host rejected the reload request before navigation could begin. */ + | 'request-failed' -// Backstop only. The veto event is authoritative, but a headless/paired-web host may -// cancel navigation without one, so never suspend the boundary indefinitely. +// Paired-web hosts may veto navigation without emitting the Electron signal. const RELOAD_SETTLE_GRACE_MS = 10_000 -function waitForRefusedNavigation( - win: Window -): Promise> { - return new Promise((resolve) => { - let graceTimer: ReturnType | undefined - const settle = ( - outcome: Exclude - ): void => { - if (graceTimer !== undefined) { - clearTimeout(graceTimer) - } - win.removeEventListener(ORCA_RENDERER_UNLOAD_PREVENTED_EVENT, onUnloadPrevented) - resolve(outcome) - } - const onUnloadPrevented = (): void => settle('unload-vetoed') +type RefusedNavigationWait = { + outcome: Promise<'unload-vetoed' | 'never-landed'> + cancel: () => void +} +function waitForRefusedNavigation(win: Window): RefusedNavigationWait { + let graceTimer: ReturnType | undefined + let onUnloadPrevented: () => void = () => undefined + const cancel = (): void => { + if (graceTimer !== undefined) { + clearTimeout(graceTimer) + graceTimer = undefined + } + win.removeEventListener(ORCA_RENDERER_UNLOAD_PREVENTED_EVENT, onUnloadPrevented) + } + const outcome = new Promise<'unload-vetoed' | 'never-landed'>((resolve) => { + const settle = (result: 'unload-vetoed' | 'never-landed'): void => { + cancel() + resolve(result) + } + onUnloadPrevented = () => settle('unload-vetoed') win.addEventListener(ORCA_RENDERER_UNLOAD_PREVENTED_EVENT, onUnloadPrevented) graceTimer = setTimeout(() => settle('never-landed'), RELOAD_SETTLE_GRACE_MS) }) + return { outcome, cancel } } -/** - * Requests the one recovery reload. Resolves ONLY when the navigation was refused — - * a landed reload tears the document down instead, so the caller stays suspended. - */ +/** Resolves only if the navigation is refused; a landed reload destroys this document. */ export async function requestLazyChunkRecoveryReload( win: Window ): Promise { @@ -62,16 +57,21 @@ export async function requestLazyChunkRecoveryReload( abortedEventName: ORCA_APP_RESTART_ABORTED_EVENT }) } catch { - // prepareRendererForAppRestart already dispatched the abort; never reload over - // editor buffers that could not be backed up. + // Never reload over editor buffers that could not be backed up. return 'checkpoint-refused' } - const refused = waitForRefusedNavigation(win) - win.location.reload() - const outcome = await refused - // Why: the latch suppresses the unsaved-changes prompt, so a document that stayed - // alive must never keep it armed. - win.dispatchEvent(new Event(ORCA_APP_RESTART_ABORTED_EVENT)) - return outcome + let cancelRefusalWait = (): void => undefined + try { + const refused = waitForRefusedNavigation(win) + cancelRefusalWait = refused.cancel + win.location.reload() + return await refused.outcome + } catch { + return 'request-failed' + } finally { + cancelRefusalWait() + // A surviving document must not retain the restart latch. + win.dispatchEvent(new Event(ORCA_APP_RESTART_ABORTED_EVENT)) + } } diff --git a/src/renderer/src/lib/lazy-with-retry.right-sidebar-syntax-error.test.ts b/src/renderer/src/lib/lazy-with-retry.right-sidebar-syntax-error.test.ts index 5746d875d9e..f0ba64513a3 100644 --- a/src/renderer/src/lib/lazy-with-retry.right-sidebar-syntax-error.test.ts +++ b/src/renderer/src/lib/lazy-with-retry.right-sidebar-syntax-error.test.ts @@ -25,10 +25,7 @@ import { isLazyChunkLoadError, loadLazyWithRetry } from './lazy-with-retry' // corrupt-chunk failure; these tests pin that behavior. const RELOAD_GUARD_KEY = 'orca:lazy-chunk-reload-attempted' -// Only a token left by a *different* document proves the reload landed; a token -// matching this document means the reload was requested and vetoed. const LANDED_RELOAD_GUARD_VALUE = 'doc-before-the-reload' -const VETOED_RELOAD_GUARD_VALUE = (): string => String(performance.timeOrigin) // The exact error the renderer received from the corrupt right-sidebar chunk. const reportedCrashError = (): SyntaxError => new SyntaxError("Unexpected token ')'") @@ -113,31 +110,4 @@ describe('right-sidebar lazy chunk SyntaxError crash (regression)', () => { // The parse error from the very same corrupt chunk must be recovered too. expect(isLazyChunkLoadError(parseOutcome)).toBe(true) }) - - // Deliberate narrowing of the recovery above: a guard this document wrote itself - // means its reload() was vetoed, so no refetch ever happened and the sentinel - // would hide a live failure from the boundary. - it('surfaces the raw SyntaxError when this document is the one whose reload was vetoed', async () => { - const reload = spyOnReload() - window.sessionStorage.setItem(RELOAD_GUARD_KEY, VETOED_RELOAD_GUARD_VALUE()) - const error = reportedCrashError() - const factory = vi.fn(() => Promise.reject(error)) - - const loaded = loadLazyWithRetry(factory, { retries: 0, reloadKey: 'right-sidebar' }) - // Track settlement rather than awaiting: a regression leaves this pending forever. - let caught: unknown = 'pending' - void loaded.then( - (value) => { - caught = value - }, - (rejection: unknown) => { - caught = rejection - } - ) - await vi.advanceTimersByTimeAsync(5000) - - expect(caught).toBe(error) - expect(isLazyChunkLoadError(caught)).toBe(false) - expect(reload).not.toHaveBeenCalled() // re-arming would loop against the veto - }) }) diff --git a/src/renderer/src/lib/lazy-with-retry.test.ts b/src/renderer/src/lib/lazy-with-retry.test.ts index a5b086c6b19..b38386d2f8c 100644 --- a/src/renderer/src/lib/lazy-with-retry.test.ts +++ b/src/renderer/src/lib/lazy-with-retry.test.ts @@ -20,8 +20,6 @@ import { } from '../../../shared/editor-save-events' const RELOAD_GUARD_KEY = 'orca:lazy-chunk-reload-attempted' -// A guard holding a *different* document's token is the only proof the reload -// landed; a token matching this document means the reload was vetoed. const LANDED_RELOAD_GUARD_VALUE = 'doc-before-the-reload' const Comp: ComponentType = () => null const chunkParseError = (): SyntaxError => new SyntaxError("Unexpected token ']'") @@ -59,7 +57,6 @@ function makeSessionStorageThrow(): void { beforeEach(() => { vi.useFakeTimers() window.sessionStorage.clear() - // The per-document reload cap is module state; a fresh document per test. resetLazyChunkReloadRequestsForTest() }) @@ -123,15 +120,10 @@ describe('loadLazyWithRetry', () => { expect(factory).toHaveBeenCalledTimes(3) expect(reload).toHaveBeenCalledTimes(1) - // The guard stores the requesting document's identity, not a bare flag. expect(window.sessionStorage.getItem(RELOAD_GUARD_KEY)).toBe(String(performance.timeOrigin)) - // The load promise must stay pending across the reload window, so the error - // boundary never flashes ahead of the navigation. expect(settled).toBe(false) }) - // Crash b860def2: reload() was called at 19:05 and the document was still alive at - // 19:49 (no renderer bootstrap in between), so the requesting pane suspended forever. it('surfaces the original error when the guarded reload never tears the document down', async () => { const reload = spyOnReload() stubCrashReportsBreadcrumb() @@ -151,7 +143,6 @@ describe('loadLazyWithRetry', () => { await vi.advanceTimersByTimeAsync(5000) expect(reload).toHaveBeenCalledTimes(1) - // Still suspended inside the grace window, so a landed reload shows no fallback. expect(settled).toBe('pending') await vi.advanceTimersByTimeAsync(10_000) @@ -178,9 +169,6 @@ describe('loadLazyWithRetry', () => { expect(isLazyChunkLoadError(caught)).toBe(true) }) - // Crash b860def2: the guarded reload() was vetoed, the document lived on for 44 - // minutes, and the still-set guard turned the next failure into the sentinel that - // error boundaries suppress — hiding a recovery that never ran. it('reports the original error when the guard belongs to this same document (reload vetoed)', async () => { const reload = spyOnReload() window.sessionStorage.setItem(RELOAD_GUARD_KEY, String(performance.timeOrigin)) @@ -192,15 +180,11 @@ describe('loadLazyWithRetry', () => { await vi.advanceTimersByTimeAsync(5000) await assertion - // Re-arming the reload here would loop against whatever vetoed the first one. expect(reload).not.toHaveBeenCalled() const caught = await loaded.catch((rejection) => rejection) expect(isLazyChunkLoadError(caught)).toBe(false) }) - // b860def2's only reload evidence was a 19:05 breadcrumb, evicted from the 30-entry - // ring long before the 19:49 report. This path now files the report, so it must leave - // its own proof in the same tick. it('records a lazy_chunk_reload_vetoed breadcrumb in the tick that reports the crash', async () => { spyOnReload() const recordBreadcrumb = stubCrashReportsBreadcrumb() @@ -374,26 +358,20 @@ describe('loadLazyWithRetry', () => { }) }) -// Crash b860def2: `window.location.reload()` was requested at 19:05:00.984Z and never -// landed — Terminal's beforeunload handler preventDefault()s whenever any open editor -// file is dirty and Electron's will-prevent-unload cancels the navigation with no -// dialog. In an editor app with one unsaved tab, chunk recovery could never run. describe('loadLazyWithRetry recovery reload vs the dirty-editor-tab unload veto', () => { type Harness = { navigations: string[] hotExitBackups: number restartLatchAtNavigation: boolean - cleanup: () => void } - // Mirrors production: Terminal.tsx's dirty-tab guard + Electron's will-prevent-unload - // relay (preload dispatches ORCA_RENDERER_UNLOAD_PREVENTED_EVENT on cancel). + let cleanupHarness: (() => void) | undefined + function installDirtyEditorTab(options: { hotExitBackupFails?: boolean } = {}): Harness { const harness: Harness = { navigations: [], hotExitBackups: 0, - restartLatchAtNavigation: false, - cleanup: () => {} + restartLatchAtNavigation: false } const cleanupBypass = registerUpdaterBeforeUnloadBypass() @@ -425,7 +403,7 @@ describe('loadLazyWithRetry recovery reload vs the dirty-editor-tab unload veto' } }) - harness.cleanup = () => { + cleanupHarness = () => { window.removeEventListener('beforeunload', dirtyTabGuard) window.removeEventListener(ORCA_EDITOR_PREPARE_HOT_EXIT_EVENT, hotExitBackup) cleanupBypass() @@ -440,6 +418,8 @@ describe('loadLazyWithRetry recovery reload vs the dirty-editor-tab unload veto' }) afterEach(() => { + cleanupHarness?.() + cleanupHarness = undefined vi.restoreAllMocks() vi.useRealTimers() delete (window as unknown as { api?: unknown }).api @@ -464,11 +444,8 @@ describe('loadLazyWithRetry recovery reload vs the dirty-editor-tab unload veto' expect(harness.navigations).toEqual(['landed']) expect(harness.restartLatchAtNavigation).toBe(true) - // Unsaved buffers are backed up before the document is thrown away. expect(harness.hotExitBackups).toBe(1) - // A landed navigation kills the document, so the boundary never sees the error. expect(settled).toBe('pending') - harness.cleanup() }) it('refuses to reload when unsaved buffers cannot be backed up', async () => { @@ -495,7 +472,6 @@ describe('loadLazyWithRetry recovery reload vs the dirty-editor-tab unload veto' expect(harness.navigations).toEqual([]) expect(settled).toBe(error) - // The latch suppresses the unsaved-changes prompt; never leave it armed. expect(restartAborted).toHaveBeenCalled() expect(isIntentionalAppRestartInProgress()).toBe(false) expect(recordBreadcrumb).toHaveBeenCalledWith({ @@ -507,12 +483,37 @@ describe('loadLazyWithRetry recovery reload vs the dirty-editor-tab unload veto' } }) window.removeEventListener(ORCA_APP_RESTART_ABORTED_EVENT, restartAborted) - harness.cleanup() + }) + + it('clears recovery state when the host rejects the reload request', async () => { + const harness = installDirtyEditorTab() + const recordBreadcrumb = stubCrashReportsBreadcrumb() + vi.mocked(window.location.reload).mockImplementation(() => { + throw new Error('reload unavailable') + }) + const error = chunkParseError() + + const settled = await loadLazyWithRetry(() => Promise.reject(error), { + retries: 0, + reloadKey: 'rich-markdown-editor' + }).catch((rejection: unknown) => rejection) + + expect(settled).toBe(error) + expect(harness.hotExitBackups).toBe(1) + expect(isIntentionalAppRestartInProgress()).toBe(false) + expect(window.sessionStorage.getItem(RELOAD_GUARD_KEY)).toBeNull() + expect(vi.getTimerCount()).toBe(0) + expect(recordBreadcrumb).toHaveBeenCalledWith({ + name: 'lazy_chunk_reload_vetoed', + data: { + reloadKey: 'rich-markdown-editor', + message: "Unexpected token ']'", + outcome: 'request-failed' + } + }) }) it('settles on the unload-prevented signal instead of waiting out the blind grace window', async () => { - // Chromium throttles background-tab timers, so a 10s blind wait can stretch far past - // a real veto — and the old timer was never cleared for a navigation that did land. vi.spyOn(window.location, 'reload').mockImplementation(() => { window.dispatchEvent(new Event(ORCA_RENDERER_UNLOAD_PREVENTED_EVENT)) }) @@ -555,14 +556,11 @@ describe('loadLazyWithRetry recovery reload vs the dirty-editor-tab unload veto' } expect(await attempt()).toBe(error) - // A guard left by a reload that never happened denies this document recovery for - // the rest of the session, even once the user saves the tab that blocked it. expect(window.sessionStorage.getItem(RELOAD_GUARD_KEY)).toBeNull() expect(await attempt()).toBe(error) expect(reload).toHaveBeenCalledTimes(2) - // ...but clearing the guard must not let a permanently vetoing window loop. expect(await attempt()).toBe(error) expect(reload).toHaveBeenCalledTimes(2) }) @@ -576,7 +574,6 @@ describe('loadLazyWithRetry recovery reload vs the dirty-editor-tab unload veto' () => undefined ) await vi.advanceTimersByTimeAsync(50) - // A second lazy site fails while the document is already tearing down. void loadLazyWithRetry(() => Promise.reject(error), { retries: 0 }).then( () => undefined, () => undefined diff --git a/src/renderer/src/lib/lazy-with-retry.ts b/src/renderer/src/lib/lazy-with-retry.ts index dba4c9c6d57..b052fa8e0e6 100644 --- a/src/renderer/src/lib/lazy-with-retry.ts +++ b/src/renderer/src/lib/lazy-with-retry.ts @@ -13,10 +13,8 @@ import { * ']'"). React.lazy permanently caches that rejection, so the error boundary's * "Retry" — which just re-renders the same Lazy — can never recover it; the * surface stays dead and reports a react-error-boundary crash. This wrapper first - * retries transient fetch failures, then performs ONE guarded full reload — routed - * through the intentional-restart path so Orca's dirty-tab beforeunload guard cannot - * silently veto it — to refetch fresh chunk bytes and rebuild the ES module map, - * before finally falling through to the error boundary. + * retries transient failures, then requests one restart-prepared reload before + * falling through to the error boundary. */ // eslint-disable-next-line @typescript-eslint/no-explicit-any -- mirror React.lazy's own ComponentType constraint so every existing call site type-checks unchanged. @@ -49,23 +47,15 @@ export function isLazyChunkLoadError(error: unknown): error is LazyChunkLoadErro return error instanceof LazyChunkLoadError } -// One recovery reload per session. The guard survives the reload itself (so we -// never loop) but resets when the window/app closes, so a later launch — e.g. -// after an update ships fresh chunks — can earn another reload. sessionStorage -// (not localStorage) gives exactly that lifetime; a healthy sibling load never -// clears it, otherwise an auto-mounted corrupt chunk would loop. The one exception -// is a reload this document proved never landed (see clearChunkReloadGuard). +// The session guard survives a landed reload to prevent loops, but a surviving +// document clears its own token so a later failure can retry recovery. const RELOAD_GUARD_KEY = 'orca:lazy-chunk-reload-attempted' -// Stand-in document identity for hosts without a usable performance.timeOrigin; a -// reload re-evaluates this module, so a fresh value still means the reload landed. +// Reloading reevaluates this fallback, giving the new document a different token. const FALLBACK_RELOAD_TOKEN = `doc-${Math.random().toString(36).slice(2)}` const DEFAULT_RETRIES = 2 const DEFAULT_BASE_DELAY_MS = 250 -// The guard stores the requesting document's identity, not a bare flag: reload() can be -// vetoed (beforeunload preventDefault → Electron cancels the navigation) and a bare flag -// would then read as "recovery ran" forever inside a document that never reloaded (crash -// b860def2). timeOrigin changes per navigation, so a value still ours means no reload. +// A matching token proves this document requested a reload that never landed. function currentDocumentReloadToken(): string { const timeOrigin = typeof performance === 'undefined' ? Number.NaN : performance.timeOrigin return Number.isFinite(timeOrigin) && timeOrigin > 0 ? String(timeOrigin) : FALLBACK_RELOAD_TOKEN @@ -98,9 +88,6 @@ function markChunkReloadAttempted(): boolean { } } -// Why: a token this document wrote itself proves no reload happened, so leaving it in -// place would deny the surface recovery for the whole session even after whatever -// vetoed the navigation is gone (e.g. the user saved the dirty tab). function clearChunkReloadGuard(): void { try { window.sessionStorage.removeItem(RELOAD_GUARD_KEY) @@ -109,12 +96,9 @@ function clearChunkReloadGuard(): void { } } -// sessionStorage cannot bound a document that keeps re-arming its own guard, so cap -// recovery requests in memory; the counter dies with the document a reload replaces. +// A per-document cap prevents repeated vetoes from creating a reload loop. const MAX_RELOAD_REQUESTS_PER_DOCUMENT = 2 let reloadRequestsThisDocument = 0 -// A sibling chunk failing mid-navigation reads the same 'reload-not-landed' guard; -// clearing it there would re-arm a document that is already tearing down. let reloadRequestInFlight = false export function resetLazyChunkReloadRequestsForTest(): void { @@ -211,11 +195,15 @@ export async function loadLazyWithRetry( reloadRequestsThisDocument += 1 reloadRequestInFlight = true recordReloadBreadcrumb('lazy_chunk_reload', reloadKey, failureMessage) - // Resolves only if the navigation was refused; a landed reload kills this document. - const outcome: LazyChunkRecoveryReloadOutcome = await requestLazyChunkRecoveryReload(window) - reloadRequestInFlight = false - clearChunkReloadGuard() - recordReloadBreadcrumb('lazy_chunk_reload_vetoed', reloadKey, failureMessage, outcome) + let outcome: LazyChunkRecoveryReloadOutcome = 'request-failed' + try { + // A landed reload tears down this document before the promise settles. + outcome = await requestLazyChunkRecoveryReload(window) + } finally { + reloadRequestInFlight = false + clearChunkReloadGuard() + recordReloadBreadcrumb('lazy_chunk_reload_vetoed', reloadKey, failureMessage, outcome) + } throw lastError } @@ -228,8 +216,7 @@ export async function loadLazyWithRetry( !reloadRequestInFlight && isKnownDynamicImportFailure(lastError) ) { - // Why here and not only at request time: this crumb lands in the same tick as the - // crash report, so the 30-entry ring cannot evict it the way it evicted b860def2's. + // Record the veto beside the resulting crash report before the ring can evict it. clearChunkReloadGuard() recordReloadBreadcrumb( 'lazy_chunk_reload_vetoed', @@ -239,10 +226,7 @@ export async function loadLazyWithRetry( ) } - // No proven reload (SSR / node / blocked storage / a reload this document asked - // for but never got) or unknown failure: re-throw the original error so normal - // error reporting semantics stay intact. Boundaries suppress the sentinel, so - // wrapping an unproven recovery here would hide the failure entirely. + // Without a proven reload, preserve normal error-reporting behavior. throw lastError }