fix(renderer): let boundary Retry and a real restart recover a stale lazy chunk

After an update swaps assets under a live renderer, a lazy chunk import rejects with a SyntaxError and the recovery reload never lands (all shipped lazy_chunk_reload_vetoed breadcrumbs carry outcome=never-landed). Since #12950 the boundary contains the failure silently, but React.lazy caches the rejection forever, so its Retry button could never re-invoke the factory: the surface stayed dead until a manual app restart. lazyWithRetry now returns a wrapper that mints a fresh React.lazy when a reset remounts it after a caught chunk failure. Minting is gated on a retry epoch advanced only in the boundary's componentDidCatch, so React's own settle-driven remounts reuse the cached rejection instead of refetching a broken chunk in a loop; each fresh load costs a Retry press or a resetKey change. For the case where the assets are still broken, a contained LazyChunkLoadError fallback now explains that Orca may have updated and offers Restart Orca via the existing main-driven app:relaunch IPC, which a swallowed renderer-side location.reload cannot veto.
2026-10-25 00:03:59 +00:00 · 2026-08-16 18:32:01 -07:00
parent 96b03bab4f
commit a8ff60a285
9 changed files with 287 additions and 25 deletions
@@ -0,0 +1,131 @@
+// @vitest-environment happy-dom
+
+// The user-recovery gap behind the contained lazy-chunk crashes: after a corrupt
+// chunk and a recovery reload that never lands, the boundary's Retry re-renders
+// the same React.lazy, whose cached rejection can never re-invoke the factory —
+// so a surface whose chunk has become loadable again (assets settled after an
+// update) still stays dead until a full app restart.
+
+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
+
+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
+}
+
+function BoundaryHarness({ children }: { children: ReactNode }): ReactElement {
+  return (
+    <RecoverableRenderErrorBoundary boundaryId="right-sidebar" surface="right-sidebar">
+      <Suspense fallback={<div>Loading...</div>}>{children}</Suspense>
+    </RecoverableRenderErrorBoundary>
+  )
+}
+
+describe('RecoverableRenderErrorBoundary Retry 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()
+    // Production truth: location.reload() produced zero navigations in the shipped bundles.
+    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()
+    delete (window as unknown as { api?: unknown }).api
+    vi.restoreAllMocks()
+    vi.useRealTimers()
+    consoleError.mockRestore()
+  })
+
+  it('re-invokes the chunk factory on Retry and recovers the surface once assets settle', async () => {
+    const breadcrumbs = installBreadcrumbSink()
+    const HealedSurface = (): ReactElement => <div data-testid="healed-surface">healed</div>
+    // First fetch reads the mid-update asset set; the retry reads the settled one.
+    const chunkFactory = vi
+      .fn<() => Promise<{ default: typeof HealedSurface }>>()
+      .mockRejectedValueOnce(new SyntaxError("Unexpected token '}'"))
+      .mockResolvedValue({ default: HealedSurface })
+    // No reloadKey: matches the shipped call sites (reloadKey=unknown breadcrumbs).
+    const LazyHealingChunk = lazyWithRetry(chunkFactory, { retries: 0 })
+
+    container = document.createElement('div')
+    document.body.appendChild(container)
+    root = createRoot(container)
+
+    await act(async () => {
+      root?.render(
+        <BoundaryHarness>
+          <LazyHealingChunk />
+        </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()
+    expect(chunkFactory).toHaveBeenCalledTimes(1)
+    expect(reportCrashMock).not.toHaveBeenCalled()
+    const reload = breadcrumbs.find((crumb) => crumb.name === 'lazy_chunk_reload')
+    expect(reload?.data.reloadKey).toBe('unknown')
+
+    const retryButton = container.querySelector<HTMLButtonElement>('[role="alert"] button')
+    expect(retryButton).not.toBeNull()
+    await act(async () => {
+      retryButton?.click()
+      await vi.advanceTimersByTimeAsync(0)
+    })
+    await act(async () => {
+      await vi.advanceTimersByTimeAsync(0)
+    })
+
+    // The whole defect: React.lazy cached the rejection, so Retry never re-invokes
+    // the factory and the surface stays dead even though the chunk now loads.
+    expect(chunkFactory).toHaveBeenCalledTimes(2)
+    expect(container.querySelector('[data-testid="healed-surface"]')).not.toBeNull()
+    expect(container.querySelector('[role="alert"]')).toBeNull()
+    expect(reportCrashMock).not.toHaveBeenCalled()
+  })
+})
@@ -2,7 +2,7 @@ import React from 'react'
 import { AlertTriangle, RotateCw } from 'lucide-react'
 import { Button } from '@/components/ui/button'
 import { cn } from '@/lib/utils'
-import { isLazyChunkLoadError } from '@/lib/lazy-with-retry'
+import { advanceLazyChunkRetryEpoch, isLazyChunkLoadError } from '@/lib/lazy-with-retry'
 import { reportReactErrorBoundaryCrash } from '@/lib/react-error-boundary-reporting'
 import type { ReactErrorBoundaryReportArgs } from '../../../../shared/crash-reporting'
 import { translate } from '@/i18n/i18n'
@@ -28,10 +28,11 @@ type Props = {
 type State = {
  error: Error | null
  resetKey: Props['resetKey']
+  relaunching: boolean
 }

 export class RecoverableRenderErrorBoundary extends React.Component<Props, State> {
-  state: State = { error: null, resetKey: this.props.resetKey }
+  state: State = { error: null, resetKey: this.props.resetKey, relaunching: false }

  static getDerivedStateFromProps(props: Props, state: State): Partial<State> | null {
    if (props.resetKey !== state.resetKey) {
@@ -46,6 +47,10 @@ export class RecoverableRenderErrorBoundary extends React.Component<Props, State

  componentDidCatch(error: Error, errorInfo: React.ErrorInfo): void {
    console.error(`[${this.props.boundaryId}] render crash contained by boundary`, error, errorInfo)
+    if (isLazyChunkLoadError(error)) {
+      // Why: lets the next reset (Retry or resetKey) mint a lazy that can load again.
+      advanceLazyChunkRetryEpoch()
+    }
    if (this.props.reportAsCrash === false) {
      return
    }
@@ -65,6 +70,15 @@ export class RecoverableRenderErrorBoundary extends React.Component<Props, State
    this.setState({ error: null })
  }

+  handleRelaunchApp = (): void => {
+    this.setState({ relaunching: true })
+    void window.api.app.relaunch().catch((error: unknown) => {
+      // Why: a refused pre-relaunch checkpoint keeps the app open; re-enable the button.
+      console.error(`[${this.props.boundaryId}] app relaunch failed`, error)
+      this.setState({ relaunching: false })
+    })
+  }
+
  render(): React.ReactNode {
    if (!this.state.error) {
      return this.props.children
@@ -74,6 +88,11 @@ export class RecoverableRenderErrorBoundary extends React.Component<Props, State
      return this.props.fallback({ error: this.state.error, reset: this.handleReset })
    }

+    // A contained chunk failure usually means the app updated under this renderer;
+    // a main-driven relaunch is the recovery that a swallowed in-place reload isn't.
+    const staleChunk = isLazyChunkLoadError(this.state.error)
+    const canRelaunch = staleChunk && typeof window.api?.app?.relaunch === 'function'
+
    return (
      <div
        className={cn(
@@ -89,26 +108,52 @@ export class RecoverableRenderErrorBoundary extends React.Component<Props, State
        <div className="space-y-1">
          <div className="font-medium text-foreground">
            {this.props.title ??
-              translate(
-                'auto.components.error.boundaries.RecoverableRenderErrorBoundary.ab855c11f4',
-                'This part of Orca hit an error.'
-              )}
+              (staleChunk
+                ? translate(
+                    'auto.components.error.boundaries.RecoverableRenderErrorBoundary.staleChunkTitle',
+                    'This part of Orca could not load.'
+                  )
+                : translate(
+                    'auto.components.error.boundaries.RecoverableRenderErrorBoundary.ab855c11f4',
+                    'This part of Orca hit an error.'
+                  ))}
          </div>
          <div className="max-w-md text-xs">
            {this.props.description ??
-              translate(
-                'auto.components.error.boundaries.RecoverableRenderErrorBoundary.34a189ae0f',
-                'The rest of the app is still running. Retry this surface or switch away and come back.'
-              )}
+              (staleChunk
+                ? translate(
+                    'auto.components.error.boundaries.RecoverableRenderErrorBoundary.staleChunkDescription',
+                    'Orca may have updated in the background. Retry, or restart Orca to finish applying the update.'
+                  )
+                : translate(
+                    'auto.components.error.boundaries.RecoverableRenderErrorBoundary.34a189ae0f',
+                    'The rest of the app is still running. Retry this surface or switch away and come back.'
+                  ))}
          </div>
        </div>
-        <Button type="button" variant="outline" size="sm" onClick={this.handleReset}>
-          <RotateCw className="size-3.5" />
-          {translate(
-            'auto.components.error.boundaries.RecoverableRenderErrorBoundary.55001880db',
-            'Retry'
-          )}
-        </Button>
+        <div className="flex items-center gap-2">
+          <Button type="button" variant="outline" size="sm" onClick={this.handleReset}>
+            <RotateCw className="size-3.5" />
+            {translate(
+              'auto.components.error.boundaries.RecoverableRenderErrorBoundary.55001880db',
+              'Retry'
+            )}
+          </Button>
+          {canRelaunch ? (
+            <Button
+              type="button"
+              variant="outline"
+              size="sm"
+              disabled={this.state.relaunching}
+              onClick={this.handleRelaunchApp}
+            >
+              {translate(
+                'auto.components.error.boundaries.RecoverableRenderErrorBoundary.restartOrca',
+                'Restart Orca'
+              )}
+            </Button>
+          ) : null}
+        </div>
      </div>
    )
  }
@@ -13767,7 +13767,10 @@
          "RecoverableRenderErrorBoundary": {
            "55001880db": "Retry",
            "34a189ae0f": "The rest of the app is still running. Retry this surface or switch away and come back.",
-            "ab855c11f4": "This part of Orca hit an error."
+            "ab855c11f4": "This part of Orca hit an error.",
+            "restartOrca": "Restart Orca",
+            "staleChunkDescription": "Orca may have updated in the background. Retry, or restart Orca to finish applying the update.",
+            "staleChunkTitle": "This part of Orca could not load."
          }
        }
      },
@@ -12468,7 +12468,10 @@
          "RecoverableRenderErrorBoundary": {
            "55001880db": "Reintentar",
            "34a189ae0f": "El resto de la app sigue ejecutándose. Reintenta esta vista o cambia a otra y vuelve.",
-            "ab855c11f4": "Esta parte de Orca encontró un error."
+            "ab855c11f4": "Esta parte de Orca encontró un error.",
+            "restartOrca": "Reiniciar Orca",
+            "staleChunkDescription": "Es posible que Orca se haya actualizado en segundo plano. Reintenta, o reinicia Orca para terminar de aplicar la actualización.",
+            "staleChunkTitle": "No se pudo cargar esta parte de Orca."
          }
        }
      },
@@ -12468,7 +12468,10 @@
          "RecoverableRenderErrorBoundary": {
            "55001880db": "再試行",
            "34a189ae0f": "アプリの残りの部分はまだ実行されています。このサーフェスを再試行するか、切り替えて戻ってきてください。",
-            "ab855c11f4": "Orca のこの部分でエラーが発生しました。"
+            "ab855c11f4": "Orca のこの部分でエラーが発生しました。",
+            "restartOrca": "Orca を再起動",
+            "staleChunkDescription": "バックグラウンドで Orca が更新された可能性があります。再試行するか、Orca を再起動して更新の適用を完了してください。",
+            "staleChunkTitle": "Orca のこの部分を読み込めませんでした。"
          }
        }
      },
@@ -12468,7 +12468,10 @@
          "RecoverableRenderErrorBoundary": {
            "55001880db": "다시 시도",
            "34a189ae0f": "앱의 나머지 부분은 아직 실행 중입니다. 이 표면을 다시 시도하거나 다른 곳으로 전환했다가 다시 돌아오세요.",
-            "ab855c11f4": "Orca의 이 부분에 오류가 발생했습니다."
+            "ab855c11f4": "Orca의 이 부분에 오류가 발생했습니다.",
+            "restartOrca": "Orca 재시작",
+            "staleChunkDescription": "백그라운드에서 Orca가 업데이트되었을 수 있습니다. 다시 시도하거나 Orca를 재시작하여 업데이트 적용을 완료하세요.",
+            "staleChunkTitle": "Orca의 이 부분을 불러오지 못했습니다."
          }
        }
      },
@@ -12488,7 +12488,10 @@
          "RecoverableRenderErrorBoundary": {
            "55001880db": "重试",
            "34a189ae0f": "应用程序的其余部分仍在运行。重试此表面或切换出去然后再回来。",
-            "ab855c11f4": "Orca 的这一部分遇到了错误。"
+            "ab855c11f4": "Orca 的这一部分遇到了错误。",
+            "restartOrca": "重启 Orca",
+            "staleChunkDescription": "Orca 可能已在后台完成更新。请重试，或重启 Orca 以完成更新。",
+            "staleChunkTitle": "无法加载 Orca 的这一部分。"
          }
        }
      },
@@ -129,6 +129,28 @@ describe('loadLazyWithRetry when the recovery reload never lands', () => {
    expect(window.sessionStorage.getItem(RELOAD_GUARD_KEY)).toBeNull()
  })

+  it('caps reload requests at two per document even when every call site defaults to reloadKey unknown', async () => {
+    const breadcrumbs = installBreadcrumbSink()
+
+    // Three sequential failures without a reloadKey: the guard is keyless, so
+    // 'unknown' must not defeat the per-document cap.
+    for (let failure = 0; failure < 3; failure += 1) {
+      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)
+    }
+
+    const reloads = breadcrumbs.filter((crumb) => crumb.name === 'lazy_chunk_reload')
+    expect(reloads).toHaveLength(2)
+    expect(reloads.every((crumb) => crumb.data.reloadKey === 'unknown')).toBe(true)
+    const vetoed = breadcrumbs.filter((crumb) => crumb.name === 'lazy_chunk_reload_vetoed')
+    expect(vetoed).toHaveLength(2)
+    expect(vetoed.every((crumb) => crumb.data.outcome === 'never-landed')).toBe(true)
+  })
+
  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(
@@ -1,4 +1,12 @@
-import { lazy, type ComponentType, type LazyExoticComponent } from 'react'
+import {
+  createElement,
+  lazy,
+  useState,
+  type ComponentProps,
+  type ComponentType,
+  type LazyExoticComponent,
+  type ReactElement
+} from 'react'

 import {
  requestLazyChunkRecoveryReload,
@@ -250,9 +258,50 @@ export async function loadLazyWithRetry<T extends AnyComponent>(
  throw lastError
 }

+// Epoch advances only when a boundary catches a chunk failure (a committed,
+// user-visible fallback). A remount from a LATER epoch is a deliberate reset
+// (Retry press or resetKey change); a remount from the SAME epoch is React
+// retrying the suspended tree after the load promise settled, and minting there
+// would re-fetch a broken chunk in an unbounded loop.
+let chunkRetryEpoch = 0
+
+export function advanceLazyChunkRetryEpoch(): void {
+  chunkRetryEpoch += 1
+}
+
 export function lazyWithRetry<T extends AnyComponent>(
  factory: LazyFactory<T>,
  options?: LazyWithRetryOptions
-): LazyExoticComponent<T> {
-  return lazy(() => loadLazyWithRetry(factory, options))
+): ComponentType<ComponentProps<T>> {
+  // -1 = last load did not fail; otherwise the epoch the failure settled in.
+  let failedAtEpoch = -1
+  const mintLazy = (): LazyExoticComponent<T> =>
+    lazy(() =>
+      loadLazyWithRetry(factory, options).then(
+        (loaded) => {
+          failedAtEpoch = -1
+          return loaded
+        },
+        (error: unknown) => {
+          failedAtEpoch = chunkRetryEpoch
+          throw error
+        }
+      )
+    )
+  let current = mintLazy()
+
+  // Why: React.lazy caches a rejection forever, so a boundary Retry that re-renders
+  // the same Lazy can never recover it. A reset remounts this wrapper; minting a
+  // fresh Lazy then lets the retry re-invoke the factory (the chunk may load fine
+  // after an update finishes). Bounded: one mint per caught failure per reset.
+  return function LazyWithRetry(props: ComponentProps<T>): ReactElement {
+    const [Component] = useState<LazyExoticComponent<T>>(() => {
+      if (failedAtEpoch !== -1 && failedAtEpoch !== chunkRetryEpoch) {
+        failedAtEpoch = -1
+        current = mintLazy()
+      }
+      return current
+    })
+    return createElement(Component, props)
+  }
 }