diff --git a/src/renderer/src/components/error-boundaries/RecoverableRenderErrorBoundary.never-landed-reload.test.tsx b/src/renderer/src/components/error-boundaries/RecoverableRenderErrorBoundary.never-landed-reload.test.tsx new file mode 100644 index 000000000..ea32cc997 --- /dev/null +++ b/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 ( + + Loading...}>{children} + + ) +} + +describe('RecoverableRenderErrorBoundary after a recovery reload never lands', () => { + let root: Root | null = null + let container: HTMLDivElement | null = null + let consoleError: ReturnType + + 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( + + + + ) + }) + + // 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() + }) +}) diff --git a/src/renderer/src/components/error-boundaries/RecoverableRenderErrorBoundary.tsx b/src/renderer/src/components/error-boundaries/RecoverableRenderErrorBoundary.tsx index eddfbd142..e4c03e96a 100644 --- a/src/renderer/src/components/error-boundaries/RecoverableRenderErrorBoundary.tsx +++ b/src/renderer/src/components/error-boundaries/RecoverableRenderErrorBoundary.tsx @@ -50,6 +50,7 @@ export class RecoverableRenderErrorBoundary extends React.Component 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 } + +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) + }) +}) diff --git a/src/renderer/src/lib/lazy-with-retry.test.ts b/src/renderer/src/lib/lazy-with-retry.test.ts index b38386d2f..86b0bcef2 100644 --- a/src/renderer/src/lib/lazy-with-retry.test.ts +++ b/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) }) diff --git a/src/renderer/src/lib/lazy-with-retry.ts b/src/renderer/src/lib/lazy-with-retry.ts index b052fa8e0..77650b67d 100644 --- a/src/renderer/src/lib/lazy-with-retry.ts +++ b/src/renderer/src/lib/lazy-with-retry.ts @@ -130,6 +130,13 @@ function recordReloadBreadcrumb( const wait = (ms: number): Promise => 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( 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( 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 }