fix(renderer): contain corrupt lazy chunks when the recovery reload never lands (#12950)

* fix(renderer): contain corrupt lazy chunks when the recovery reload never lands

9 react-error-boundary crash reports across v1.4.171-1.4.175 (macOS, Linux,
Windows) all end the same way: a corrupt lazy chunk fails to import, recovery
requests a reload, the reload never lands, and loadLazyWithRetry re-throws the
raw SyntaxError/TypeError. RecoverableRenderErrorBoundary only suppresses
LazyChunkLoadError, so the raw error files a user-facing crash report.

LazyChunkLoadError was unreachable in production. Its precondition is a guard
written by a *different* document ('reload-landed'), but the finally block
clears that guard before the throw, so the only path that could construct it
never ran. Confirmed by the shipped bundles: 16/16 lazy_chunk_reload_vetoed
breadcrumbs carry outcome=never-landed, zero carry any other outcome, and no
bundle contains a boundary-degraded breadcrumb.

Route every exhausted-recovery path through exhaustedRecoveryFailure() so an
attempted-and-failed recovery yields a LazyChunkLoadError the boundary can
contain, and record a lazy_chunk_recovery_exhausted breadcrumb carrying the
call site, the real chunk error, and the outcome.

Deliberately unchanged: when recovery is never *attempted* (no window/SSR,
blocked sessionStorage, guard write failure) the raw error is still thrown so
normal crash reporting is unaffected. Only isKnownDynamicImportFailure matches
are contained, so module logic bugs keep reporting.

* perf(renderer): trim redundant work on the lazy-chunk failure path

Hoist the dynamic-import message patterns to module scope so classification
stops allocating seven RegExp objects per call, thread the already-computed
classification into exhaustedRecoveryFailure so the guard-not-landed path does
not re-run it, and bound recordedExhaustionKeys the way the breadcrumb and
renderer-error key stores are bounded, since error.name is library-controlled.

Failure path only; the success path is unchanged.

* refactor(renderer): remove a transposition trap on the lazy-chunk failure path

exhaustedRecoveryFailure ended in two adjacent booleans with opposite
consequences: transposing them would have returned the raw SyntaxError and
silently restored the crash this branch fixes, with no test able to catch it
(the only call site passed true for both). The isChunkFailure parameter saved
one regex scan on a path that only runs after a 10s reload wait, so drop it.

Also evict recordedExhaustionKeys oldest-first instead of clearing wholesale,
matching the breadcrumb and renderer-error key stores the comment cites, so an
overflow cannot re-open the entire set to a repeat burst.

* test(renderer): cover the exhaustion dedupe bound

The bound had no coverage, unlike the crash-breadcrumb store it mirrors, so a
refactor could drop it or invert the comparison with every test still green.
Drive 200 distinct error names through the contained path and assert the set
stays capped. Also move MAX_RECORDED_EXHAUSTION_KEYS above the comment that
describes the set, not between them.

* test(renderer): pin the exhaustion eviction policy, not just the cap

The bound test asserted only the size cap, so it stayed green under the old
wholesale clear(): after 200 distinct keys a clear-on-overflow leaves 72, which
still satisfies the cap. Replay a key that oldest-first eviction retains and
assert it emits no second breadcrumb — that fails under clear(), which would
otherwise silently re-open the whole set to a repeat burst and flush the
30-entry ring the dedupe exists to protect.

* refactor(renderer): cut the breadcrumb machinery down to the actual fix

The lazy_chunk_recovery_exhausted breadcrumb was an optional addition that paid
for itself in complexity and nothing else: it needed a dedupe set to avoid
flushing the 30-entry ring, the set needed a bound because error.name is
library-controlled, the bound needed an oldest-first eviction policy, and that
needed two more tests plus a boolean parameter that review flagged as a
transposition trap. On the dominant never-landed path it did not even fire,
because lazy_chunk_reload_vetoed already records the same reloadKey, message and
outcome.

Drop it. Observability on every path returns to the main baseline, and the fix
is what it always was: name an exhausted recovery so the boundary can contain
it. Also revert the unrelated regex hoist -- its only caller is the failure
path, so the saved allocations are noise.

* Verify ordinary errors bypass lazy chunk containment

Add test ensuring module evaluation bugs still surface despite
never-landed reload attempts. Clarify containment scope: recovery
only applies to known dynamic-import failures, not ordinary errors.

src/renderer/src/components/error-boundaries/RecoverableRenderErrorBoundary.never-landed-reload.test.tsx

@@ -0,0 +1,90 @@
+// @vitest-environment happy-dom
+
+// The end-to-end shape of the 9 shipped lazy-chunk crash reports: a corrupt chunk
+// fails, recovery requests a reload, the reload never lands, and the boundary files
+// a react-error-boundary crash report instead of containing the failure.
+
+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'
+
+import { lazyWithRetry, resetLazyChunkReloadRequestsForTest } from '@/lib/lazy-with-retry'
+import { RecoverableRenderErrorBoundary } from './RecoverableRenderErrorBoundary'
+
+const reportCrashMock = vi.hoisted(() => vi.fn())
+
+vi.mock('@/lib/react-error-boundary-reporting', () => ({
+  reportReactErrorBoundaryCrash: reportCrashMock
+}))
+
+const RELOAD_SETTLE_GRACE_MS = 10_000
+
+globalThis.IS_REACT_ACT_ENVIRONMENT = true
+
+function BoundaryHarness({ children }: { children: ReactNode }): ReactElement {
+  return (
+    <RecoverableRenderErrorBoundary boundaryId="right-sidebar" surface="right-sidebar">
+      <Suspense fallback={<div>Loading...</div>}>{children}</Suspense>
+    </RecoverableRenderErrorBoundary>
+  )
+}
+
+describe('RecoverableRenderErrorBoundary after a recovery reload never lands', () => {
+  let root: Root | null = null
+  let container: HTMLDivElement | null = null
+  let consoleError: ReturnType<typeof vi.spyOn>
+
+  beforeEach(() => {
+    vi.useFakeTimers()
+    reportCrashMock.mockReset()
+    window.sessionStorage.clear()
+    resetLazyChunkReloadRequestsForTest()
+    vi.spyOn(window.location, 'reload').mockImplementation(() => undefined)
+    consoleError = vi.spyOn(console, 'error').mockImplementation(() => undefined)
+  })
+
+  afterEach(() => {
+    if (root) {
+      act(() => root?.unmount())
+    }
+    container?.remove()
+    root = null
+    container = null
+    window.sessionStorage.clear()
+    resetLazyChunkReloadRequestsForTest()
+    vi.restoreAllMocks()
+    vi.useRealTimers()
+    consoleError.mockRestore()
+  })
+
+  it('shows the fallback without filing a crash report', async () => {
+    const LazyCorruptChunk = lazyWithRetry(
+      () => Promise.reject(new SyntaxError("Unexpected token '}'")),
+      { retries: 0, reloadKey: 'right-sidebar' }
+    )
+    container = document.createElement('div')
+    document.body.appendChild(container)
+    root = createRoot(container)
+
+    await act(async () => {
+      root?.render(
+        <BoundaryHarness>
+          <LazyCorruptChunk />
+        </BoundaryHarness>
+      )
+    })
+
+    // Outlive the reload settle grace window, then let React commit the rejection.
+    await act(async () => {
+      await vi.advanceTimersByTimeAsync(RELOAD_SETTLE_GRACE_MS + 50)
+    })
+    await act(async () => {
+      await vi.advanceTimersByTimeAsync(0)
+    })
+
+    expect(container?.querySelector('[role="alert"]')).not.toBeNull()
+    // Before the fix the boundary received the raw SyntaxError and filed a report;
+    // that is exactly what produced all 9 shipped crash reports.
+    expect(reportCrashMock).not.toHaveBeenCalled()
+  })
+})

src/renderer/src/components/error-boundaries/RecoverableRenderErrorBoundary.tsx

@@ -50,6 +50,7 @@ export class RecoverableRenderErrorBoundary extends React.Component<Props, State
       return
     }
     if (isLazyChunkLoadError(error)) {
+      // Contained by this fallback; recovery breadcrumbs live on the load path.
       return
     }
     void reportReactErrorBoundaryCrash({

src/renderer/src/lib/lazy-with-retry.never-landed-reload.test.ts

@@ -0,0 +1,144 @@
+// @vitest-environment happy-dom
+
+// Reproduces the production path behind all 9 lazy-chunk crash reports on
+// 1.4.171–1.4.175: guard 'not-attempted' -> reload requested -> the reload never
+// lands -> recovery gives up. 16/16 `lazy_chunk_reload_vetoed` breadcrumbs across
+// the shipped bundles carry outcome=never-landed and no bundle contains a
+// LazyChunkLoadError, so the boundary receives the raw SyntaxError and files a crash.
+
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+
+import { ORCA_RENDERER_UNLOAD_PREVENTED_EVENT } from '../../../shared/renderer-shutdown-events'
+import {
+  isLazyChunkLoadError,
+  loadLazyWithRetry,
+  resetLazyChunkReloadRequestsForTest
+} from './lazy-with-retry'
+
+const RELOAD_GUARD_KEY = 'orca:lazy-chunk-reload-attempted'
+const RELOAD_SETTLE_GRACE_MS = 10_000
+
+// The dominant crash-time message across the shipped bundles (7/9 reports).
+const CORRUPT_CHUNK_ERROR = (): SyntaxError => new SyntaxError("Unexpected token '}'")
+
+type Breadcrumb = { name: string; data: Record<string, unknown> }
+
+function installBreadcrumbSink(): Breadcrumb[] {
+  const breadcrumbs: Breadcrumb[] = []
+  ;(window as unknown as { api: unknown }).api = {
+    crashReports: {
+      recordBreadcrumb: (crumb: Breadcrumb) => {
+        breadcrumbs.push(crumb)
+      }
+    }
+  }
+  return breadcrumbs
+}
+
+describe('loadLazyWithRetry when the recovery reload never lands', () => {
+  beforeEach(() => {
+    vi.useFakeTimers()
+    window.sessionStorage.clear()
+    resetLazyChunkReloadRequestsForTest()
+    // Production truth: location.reload() produced zero navigations in all 10
+    // bundles — no renderer_bootstrap_started follows any lazy_chunk_reload.
+    vi.spyOn(window.location, 'reload').mockImplementation(() => undefined)
+  })
+
+  afterEach(() => {
+    vi.useRealTimers()
+    vi.restoreAllMocks()
+    window.sessionStorage.clear()
+    resetLazyChunkReloadRequestsForTest()
+    delete (window as unknown as { api?: unknown }).api
+  })
+
+  it('surfaces a recognizable LazyChunkLoadError so the boundary can contain it', async () => {
+    const breadcrumbs = installBreadcrumbSink()
+    const settled = loadLazyWithRetry(() => Promise.reject(CORRUPT_CHUNK_ERROR()), {
+      retries: 0,
+      reloadKey: 'right-sidebar'
+    }).then(
+      () => ({ ok: true }) as const,
+      (error: unknown) => ({ ok: false, error }) as const
+    )
+
+    // Let the reload request run, then expire the settle grace window.
+    await vi.advanceTimersByTimeAsync(0)
+    await vi.advanceTimersByTimeAsync(RELOAD_SETTLE_GRACE_MS + 1)
+
+    const result = await settled
+    expect(result.ok).toBe(false)
+
+    const vetoed = breadcrumbs.find((crumb) => crumb.name === 'lazy_chunk_reload_vetoed')
+    expect(vetoed?.data.outcome).toBe('never-landed')
+
+    // The boundary only suppresses LazyChunkLoadError; a raw SyntaxError files a crash report.
+    expect(isLazyChunkLoadError((result as { error: unknown }).error)).toBe(true)
+  })
+
+  it('does not strand a sibling lazy import that fails while a reload is pending', async () => {
+    installBreadcrumbSink()
+    const first = loadLazyWithRetry(() => Promise.reject(CORRUPT_CHUNK_ERROR()), {
+      retries: 0,
+      reloadKey: 'app.root'
+    }).catch((error: unknown) => error)
+
+    await vi.advanceTimersByTimeAsync(0)
+
+    // A second surface resolves its lazy chunk while the first reload is still in
+    // flight — the shape of 131d2ed2 and a7bc7be0, which filed raw crash reports
+    // 250 ms / 95 ms after the reload request with no explanatory breadcrumb.
+    const sibling = loadLazyWithRetry(() => Promise.reject(CORRUPT_CHUNK_ERROR()), {
+      retries: 0,
+      reloadKey: 'app.root.sibling'
+    }).catch((error: unknown) => error)
+
+    await vi.advanceTimersByTimeAsync(RELOAD_SETTLE_GRACE_MS + 1)
+
+    expect(isLazyChunkLoadError(await first)).toBe(true)
+    expect(isLazyChunkLoadError(await sibling)).toBe(true)
+  })
+
+  it('contains an unload-vetoed reload and records it as a distinct outcome', async () => {
+    const breadcrumbs = installBreadcrumbSink()
+    vi.spyOn(window.location, 'reload').mockImplementation(() => {
+      window.dispatchEvent(new Event(ORCA_RENDERER_UNLOAD_PREVENTED_EVENT))
+    })
+
+    const settled = loadLazyWithRetry(() => Promise.reject(CORRUPT_CHUNK_ERROR()), {
+      retries: 0
+    }).catch((error: unknown) => error)
+    await vi.advanceTimersByTimeAsync(0)
+
+    const vetoed = breadcrumbs.find((crumb) => crumb.name === 'lazy_chunk_reload_vetoed')
+    expect(vetoed?.data.outcome).toBe('unload-vetoed')
+    // A veto is still an attempted-and-failed recovery, so it is contained too.
+    expect(isLazyChunkLoadError(await settled)).toBe(true)
+  })
+
+  it('leaves no guard behind that would block a later document from recovering', async () => {
+    installBreadcrumbSink()
+    const settled = loadLazyWithRetry(() => Promise.reject(CORRUPT_CHUNK_ERROR()), {
+      retries: 0
+    }).catch((error: unknown) => error)
+    await vi.advanceTimersByTimeAsync(0)
+    await vi.advanceTimersByTimeAsync(RELOAD_SETTLE_GRACE_MS + 1)
+
+    expect(isLazyChunkLoadError(await settled)).toBe(true)
+    expect(window.sessionStorage.getItem(RELOAD_GUARD_KEY)).toBeNull()
+  })
+
+  it('still surfaces ordinary evaluation bugs after a never-landed reload attempt', async () => {
+    const error = new Error('render bug from lazy module evaluation')
+    const settled = loadLazyWithRetry(() => Promise.reject(error), { retries: 0 }).catch(
+      (rejection: unknown) => rejection
+    )
+    await vi.advanceTimersByTimeAsync(0)
+    await vi.advanceTimersByTimeAsync(RELOAD_SETTLE_GRACE_MS + 1)
+
+    // Containment is only for known dynamic-import failures; real bugs must still report.
+    expect(await settled).toBe(error)
+    expect(isLazyChunkLoadError(error)).toBe(false)
+  })
+})

src/renderer/src/lib/lazy-with-retry.test.ts

@@ -124,7 +124,7 @@ describe('loadLazyWithRetry', () => {
     expect(settled).toBe(false)
   })
 
-  it('surfaces the original error when the guarded reload never tears the document down', async () => {
+  it('contains the failure when the guarded reload never tears the document down', async () => {
     const reload = spyOnReload()
     stubCrashReportsBreadcrumb()
     const error = chunkParseError()
@@ -146,7 +146,9 @@ describe('loadLazyWithRetry', () => {
     expect(settled).toBe('pending')
 
     await vi.advanceTimersByTimeAsync(10_000)
-    expect(settled).toBe(error)
+    // The reload was the last recovery step, so the boundary gets a nameable error.
+    expect(isLazyChunkLoadError(settled)).toBe(true)
+    expect(settled).toMatchObject({ cause: error })
     expect(reload).toHaveBeenCalledTimes(1)
   })
 
@@ -169,23 +171,26 @@ describe('loadLazyWithRetry', () => {
     expect(isLazyChunkLoadError(caught)).toBe(true)
   })
 
-  it('reports the original error when the guard belongs to this same document (reload vetoed)', async () => {
+  it('contains the failure when the guard belongs to this same document (reload vetoed)', async () => {
     const reload = spyOnReload()
     window.sessionStorage.setItem(RELOAD_GUARD_KEY, String(performance.timeOrigin))
     const error = chunkParseError()
     const factory = vi.fn(() => Promise.reject(error))
 
     const loaded = loadLazyWithRetry(factory, { retries: 0 })
-    const assertion = expect(loaded).rejects.toBe(error)
+    const assertion = expect(loaded).rejects.toMatchObject({
+      name: 'LazyChunkLoadError',
+      cause: error
+    })
     await vi.advanceTimersByTimeAsync(5000)
     await assertion
 
     expect(reload).not.toHaveBeenCalled()
     const caught = await loaded.catch((rejection) => rejection)
-    expect(isLazyChunkLoadError(caught)).toBe(false)
+    expect(isLazyChunkLoadError(caught)).toBe(true)
   })
 
-  it('records a lazy_chunk_reload_vetoed breadcrumb in the tick that reports the crash', async () => {
+  it('records a lazy_chunk_reload_vetoed breadcrumb in the tick that contains the failure', async () => {
     spyOnReload()
     const recordBreadcrumb = stubCrashReportsBreadcrumb()
     window.sessionStorage.setItem(RELOAD_GUARD_KEY, String(performance.timeOrigin))
@@ -195,7 +200,7 @@ describe('loadLazyWithRetry', () => {
       retries: 0,
       reloadKey: 'rich-markdown-editor'
     })
-    const assertion = expect(loaded).rejects.toBe(error)
+    const assertion = expect(loaded).rejects.toMatchObject({ name: 'LazyChunkLoadError' })
     await vi.advanceTimersByTimeAsync(1)
     await assertion
 
@@ -471,7 +476,8 @@ describe('loadLazyWithRetry recovery reload vs the dirty-editor-tab unload veto'
     await vi.advanceTimersByTimeAsync(5000)
 
     expect(harness.navigations).toEqual([])
-    expect(settled).toBe(error)
+    expect(isLazyChunkLoadError(settled)).toBe(true)
+    expect(settled).toMatchObject({ cause: error })
     expect(restartAborted).toHaveBeenCalled()
     expect(isIntentionalAppRestartInProgress()).toBe(false)
     expect(recordBreadcrumb).toHaveBeenCalledWith({
@@ -498,7 +504,8 @@ describe('loadLazyWithRetry recovery reload vs the dirty-editor-tab unload veto'
       reloadKey: 'rich-markdown-editor'
     }).catch((rejection: unknown) => rejection)
 
-    expect(settled).toBe(error)
+    expect(isLazyChunkLoadError(settled)).toBe(true)
+    expect(settled).toMatchObject({ cause: error })
     expect(harness.hotExitBackups).toBe(1)
     expect(isIntentionalAppRestartInProgress()).toBe(false)
     expect(window.sessionStorage.getItem(RELOAD_GUARD_KEY)).toBeNull()
@@ -531,7 +538,8 @@ describe('loadLazyWithRetry recovery reload vs the dirty-editor-tab unload veto'
     )
     await vi.advanceTimersByTimeAsync(50)
 
-    expect(settled).toBe(error)
+    expect(isLazyChunkLoadError(settled)).toBe(true)
+    expect(settled).toMatchObject({ cause: error })
     expect(vi.getTimerCount()).toBe(0)
   })
 
@@ -555,13 +563,14 @@ describe('loadLazyWithRetry recovery reload vs the dirty-editor-tab unload veto'
       return settled
     }
 
-    expect(await attempt()).toBe(error)
+    expect(isLazyChunkLoadError(await attempt())).toBe(true)
     expect(window.sessionStorage.getItem(RELOAD_GUARD_KEY)).toBeNull()
 
-    expect(await attempt()).toBe(error)
+    expect(isLazyChunkLoadError(await attempt())).toBe(true)
     expect(reload).toHaveBeenCalledTimes(2)
 
-    expect(await attempt()).toBe(error)
+    // Cap reached: still contained, but no third navigation.
+    expect(isLazyChunkLoadError(await attempt())).toBe(true)
     expect(reload).toHaveBeenCalledTimes(2)
   })
 

src/renderer/src/lib/lazy-with-retry.ts

@@ -130,6 +130,13 @@ function recordReloadBreadcrumb(
 
 const wait = (ms: number): Promise<void> => new Promise((resolve) => setTimeout(resolve, ms))
 
+/** Recovery is spent, so name the failure in the one way the boundary can contain. */
+function containedChunkFailure(lastError: unknown, reloadKey: string): unknown {
+  return isKnownDynamicImportFailure(lastError)
+    ? new LazyChunkLoadError(lastError, reloadKey)
+    : lastError
+}
+
 function isKnownDynamicImportFailure(error: unknown): boolean {
   if (!(error instanceof Error)) {
     return false
@@ -140,11 +147,14 @@ function isKnownDynamicImportFailure(error: unknown): boolean {
   }
 
   // Why: a stale/truncated/corrupt chunk parses as invalid JS, so import()
-  // rejects with a native SyntaxError ("Unexpected token ')'", "Unexpected end
-  // of input", …). That reaches this catch only from the chunk's fetch+parse
-  // phase — a recoverable corrupt-chunk failure. Genuine module-evaluation
-  // logic bugs throw ordinary Errors (still surfaced raw) or fail later during
-  // React render (outside this load path), so they are unaffected.
+  // rejects with a native SyntaxError ("Unexpected token '}'", "Unexpected
+  // string", "Illegal return statement", …  — all four observed in shipped
+  // reports). That reaches this catch only from the chunk's fetch+parse phase.
+  // Trade-off: a SyntaxError thrown while *evaluating* a lazily imported module
+  // (e.g. a top-level JSON.parse) is indistinguishable here by message, so it is
+  // contained too. Stack-frame discrimination was tried and rejected: parser
+  // errors carry no frames today, but that is not guaranteed across V8 versions
+  // and a false negative silently restores the crash this guards against.
   if (error.name === 'SyntaxError') {
     return true
   }
@@ -190,6 +200,7 @@ export async function loadLazyWithRetry<T extends AnyComponent>(
     reloadRequestsThisDocument < MAX_RELOAD_REQUESTS_PER_DOCUMENT
   ) {
     if (!markChunkReloadAttempted()) {
+      // No recovery was attempted, so keep normal reporting rather than containing.
       throw lastError
     }
     reloadRequestsThisDocument += 1
@@ -204,29 +215,38 @@ export async function loadLazyWithRetry<T extends AnyComponent>(
       clearChunkReloadGuard()
       recordReloadBreadcrumb('lazy_chunk_reload_vetoed', reloadKey, failureMessage, outcome)
     }
-    throw lastError
+    // The reload was this document's last recovery step for this chunk, whether it
+    // was refused outright or simply never navigated.
+    throw containedChunkFailure(lastError, reloadKey)
   }
 
-  if (reloadGuardState === 'reload-landed' && isKnownDynamicImportFailure(lastError)) {
-    throw new LazyChunkLoadError(lastError, reloadKey)
+  if (reloadGuardState === 'reload-landed') {
+    throw containedChunkFailure(lastError, reloadKey)
   }
 
-  if (
-    reloadGuardState === 'reload-not-landed' &&
-    !reloadRequestInFlight &&
-    isKnownDynamicImportFailure(lastError)
-  ) {
-    // Record the veto beside the resulting crash report before the ring can evict it.
-    clearChunkReloadGuard()
-    recordReloadBreadcrumb(
-      'lazy_chunk_reload_vetoed',
-      reloadKey,
-      failureMessage,
-      'guard-not-landed'
-    )
+  if (reloadGuardState === 'reload-not-landed') {
+    // A sibling failing under a pending reload must not clear the guard, and an
+    // unrelated failure must not clear a guard it did not set.
+    if (!reloadRequestInFlight && isKnownDynamicImportFailure(lastError)) {
+      // Record the veto before the ring can evict it; the failure is then contained.
+      clearChunkReloadGuard()
+      recordReloadBreadcrumb(
+        'lazy_chunk_reload_vetoed',
+        reloadKey,
+        failureMessage,
+        'guard-not-landed'
+      )
+    }
+    throw containedChunkFailure(lastError, reloadKey)
   }
 
-  // Without a proven reload, preserve normal error-reporting behavior.
+  if (reloadGuardState === 'not-attempted') {
+    // The per-document reload cap is spent; further failures cannot recover.
+    throw containedChunkFailure(lastError, reloadKey)
+  }
+
+  // No window or no usable storage: recovery was never attempted, so preserve
+  // normal error reporting instead of containing a failure we never acted on.
   throw lastError
 }