From 4e63bb0e637646a64e8419ec4455b9362d2bb5a1 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 23 Jun 2026 15:36:48 -0700 Subject: [PATCH] Keep pages usable after lazy chunk reload failures (#6206) --- ...bleRenderErrorBoundary.lazy-chunk.test.tsx | 112 ++++++++++++++++++ .../RecoverableRenderErrorBoundary.tsx | 4 + src/renderer/src/lib/lazy-with-retry.test.ts | 53 ++++++++- src/renderer/src/lib/lazy-with-retry.ts | 66 +++++++++-- 4 files changed, 219 insertions(+), 16 deletions(-) create mode 100644 src/renderer/src/components/error-boundaries/RecoverableRenderErrorBoundary.lazy-chunk.test.tsx 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 new file mode 100644 index 00000000000..a4730f59e92 --- /dev/null +++ b/src/renderer/src/components/error-boundaries/RecoverableRenderErrorBoundary.lazy-chunk.test.tsx @@ -0,0 +1,112 @@ +// @vitest-environment happy-dom + +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 } 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_GUARD_KEY = 'orca:lazy-chunk-reload-attempted' + +globalThis.IS_REACT_ACT_ENVIRONMENT = true + +function createContainer(): { container: HTMLDivElement; root: Root } { + const container = document.createElement('div') + document.body.appendChild(container) + return { container, root: createRoot(container) } +} + +function BoundaryHarness({ children }: { children: ReactNode }): ReactElement { + return ( + + Loading...}>{children} + + ) +} + +async function flushReactWork(): Promise { + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)) + }) +} + +describe('RecoverableRenderErrorBoundary lazy chunk containment', () => { + let root: Root | null = null + let container: HTMLDivElement | null = null + let consoleError: ReturnType + + beforeEach(() => { + reportCrashMock.mockReset() + window.sessionStorage.clear() + consoleError = vi.spyOn(console, 'error').mockImplementation(() => undefined) + }) + + afterEach(() => { + if (root) { + act(() => root?.unmount()) + } + container?.remove() + root = null + container = null + window.sessionStorage.clear() + consoleError.mockRestore() + }) + + it('renders the fallback without reporting after guarded dynamic import exhaustion', async () => { + window.sessionStorage.setItem(RELOAD_GUARD_KEY, '1') + const LazyRejectingImport = lazyWithRetry( + () => + Promise.reject( + new TypeError('Failed to fetch dynamically imported module: file://redacted/chunk.js') + ), + { retries: 0 } + ) + ;({ container, root } = createContainer()) + + await act(async () => { + root?.render( + + + + ) + }) + await flushReactWork() + await flushReactWork() + + expect(container?.querySelector('[role="alert"]')).not.toBeNull() + expect(reportCrashMock).not.toHaveBeenCalled() + }) + + it('still reports ordinary render errors', async () => { + const error = new Error('ordinary render failure') + function BrokenSurface(): ReactElement { + throw error + } + ;({ container, root } = createContainer()) + + await act(async () => { + root?.render( + + + + ) + }) + + expect(container?.querySelector('[role="alert"]')).not.toBeNull() + expect(reportCrashMock).toHaveBeenCalledTimes(1) + expect(reportCrashMock).toHaveBeenCalledWith( + expect.objectContaining({ + boundaryId: 'page.automations', + surface: 'page', + error + }) + ) + }) +}) diff --git a/src/renderer/src/components/error-boundaries/RecoverableRenderErrorBoundary.tsx b/src/renderer/src/components/error-boundaries/RecoverableRenderErrorBoundary.tsx index afa7fbf3dc9..eddfbd14252 100644 --- a/src/renderer/src/components/error-boundaries/RecoverableRenderErrorBoundary.tsx +++ b/src/renderer/src/components/error-boundaries/RecoverableRenderErrorBoundary.tsx @@ -2,6 +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 { reportReactErrorBoundaryCrash } from '@/lib/react-error-boundary-reporting' import type { ReactErrorBoundaryReportArgs } from '../../../../shared/crash-reporting' import { translate } from '@/i18n/i18n' @@ -48,6 +49,9 @@ export class RecoverableRenderErrorBoundary extends React.Component null const chunkParseError = (): SyntaxError => new SyntaxError("Unexpected token ']'") +const chunkFetchError = (): TypeError => + new TypeError('Failed to fetch dynamically imported module: file://redacted/chunk.js') function spyOnReload(): ReturnType { const reload = vi.fn() @@ -107,21 +109,58 @@ describe('loadLazyWithRetry', () => { expect(settled).toBe(false) }) - it('does NOT reload twice — re-throws once the guard is already set', async () => { + it('does NOT reload twice — wraps known chunk failures once the guard is already set', async () => { const reload = spyOnReload() window.sessionStorage.setItem(RELOAD_GUARD_KEY, '1') - const error = chunkParseError() + const error = chunkFetchError() const factory = vi.fn(() => Promise.reject(error)) const loaded = loadLazyWithRetry(factory, { retries: 2, baseDelayMs: 250 }) + 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(true) + }) + + it('preserves the original error when the guarded failure is not a dynamic import failure', async () => { + const reload = spyOnReload() + window.sessionStorage.setItem(RELOAD_GUARD_KEY, '1') + const error = new Error('render bug from lazy module evaluation') + const factory = vi.fn(() => Promise.reject(error)) + + const loaded = loadLazyWithRetry(factory, { retries: 1, baseDelayMs: 100 }) const assertion = expect(loaded).rejects.toBe(error) await vi.advanceTimersByTimeAsync(5000) await assertion expect(reload).not.toHaveBeenCalled() + const caught = await loaded.catch((rejection) => rejection) + expect(isLazyChunkLoadError(caught)).toBe(false) }) - it('fails closed (re-throws, never reloads) when sessionStorage reads throw', async () => { + it('preserves bare parse errors so lazy module evaluation bugs still report', async () => { + const reload = spyOnReload() + window.sessionStorage.setItem(RELOAD_GUARD_KEY, '1') + const error = chunkParseError() + const factory = vi.fn(() => Promise.reject(error)) + + const loaded = loadLazyWithRetry(factory, { retries: 1, baseDelayMs: 100 }) + const assertion = expect(loaded).rejects.toBe(error) + await vi.advanceTimersByTimeAsync(5000) + await assertion + + expect(reload).not.toHaveBeenCalled() + const caught = await loaded.catch((rejection) => rejection) + expect(isLazyChunkLoadError(caught)).toBe(false) + }) + + it('fails closed with the original error when sessionStorage reads throw', async () => { const reload = spyOnReload() // Private-mode / sandboxed storage makes reads throw. The guard must treat // this as "already reloaded" so a broken chunk can NEVER cause a reload loop. @@ -135,6 +174,8 @@ describe('loadLazyWithRetry', () => { await assertion expect(reload).not.toHaveBeenCalled() + const caught = await loaded.catch((rejection) => rejection) + expect(isLazyChunkLoadError(caught)).toBe(false) }) it('records a lazy_chunk_reload breadcrumb (with reloadKey) before reloading', async () => { @@ -166,7 +207,7 @@ describe('loadLazyWithRetry', () => { expect(settled).toBe(false) }) - it('re-throws without reloading when there is no window (SSR / node)', async () => { + it('re-throws the original error without reloading when there is no window (SSR / node)', async () => { vi.stubGlobal('window', undefined) const error = chunkParseError() const factory = vi.fn(() => Promise.reject(error)) @@ -177,6 +218,8 @@ describe('loadLazyWithRetry', () => { await assertion expect(factory).toHaveBeenCalledTimes(2) + const caught = await loaded.catch((rejection) => rejection) + expect(isLazyChunkLoadError(caught)).toBe(false) }) it('keeps the reload guard set across a successful load (no second reload in one session)', async () => { diff --git a/src/renderer/src/lib/lazy-with-retry.ts b/src/renderer/src/lib/lazy-with-retry.ts index 5a09f6b2a95..7c3cc604382 100644 --- a/src/renderer/src/lib/lazy-with-retry.ts +++ b/src/renderer/src/lib/lazy-with-retry.ts @@ -18,6 +18,8 @@ type AnyComponent = ComponentType type LazyFactory = () => Promise<{ default: T }> +type ReloadGuardState = 'not-attempted' | 'attempted' | 'unavailable' + export type LazyWithRetryOptions = { retries?: number baseDelayMs?: number @@ -25,6 +27,18 @@ export type LazyWithRetryOptions = { reloadKey?: string } +export class LazyChunkLoadError extends Error { + constructor(cause: unknown) { + super('Lazy chunk load failed after reload recovery was exhausted') + this.name = 'LazyChunkLoadError' + ;(this as { cause?: unknown }).cause = cause + } +} + +export function isLazyChunkLoadError(error: unknown): error is LazyChunkLoadError { + 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 @@ -35,21 +49,26 @@ const RELOAD_GUARD_KEY = 'orca:lazy-chunk-reload-attempted' const DEFAULT_RETRIES = 2 const DEFAULT_BASE_DELAY_MS = 250 -function hasAttemptedChunkReload(): boolean { +function readChunkReloadGuardState(): ReloadGuardState { + if (typeof window === 'undefined') { + return 'unavailable' + } try { - return window.sessionStorage.getItem(RELOAD_GUARD_KEY) === '1' + return window.sessionStorage.getItem(RELOAD_GUARD_KEY) === '1' ? 'attempted' : 'not-attempted' } catch { - // Why: when sessionStorage is unavailable (private mode / sandboxed), fail - // closed (treat as already-reloaded) so we never risk an infinite reload loop. - return true + // Why: when storage is blocked we cannot prove a reload happened, but still + // fail closed on reloads so a broken chunk never loops. + return 'unavailable' } } -function markChunkReloadAttempted(): void { +function markChunkReloadAttempted(): boolean { try { window.sessionStorage.setItem(RELOAD_GUARD_KEY, '1') + return true } catch { - // Best-effort; if writing throws, hasAttemptedChunkReload() also fails closed. + // A reload without a durable guard can loop, so treat write failure as unavailable. + return false } } @@ -71,6 +90,24 @@ const wait = (ms: number): Promise => new Promise((resolve) => setTimeout( // down, so the error fallback never flashes in the moment before the reload lands. const SUSPEND_UNTIL_RELOAD = new Promise(() => undefined) +function isKnownDynamicImportFailure(error: unknown): boolean { + if (!(error instanceof Error)) { + return false + } + + if (error.name === 'ChunkLoadError') { + return true + } + + return [ + /failed to fetch dynamically imported module/i, + /error loading dynamically imported module/i, + /importing a module script failed/i, + /failed to load module script/i, + /loading chunk .+ failed/i + ].some((pattern) => pattern.test(error.message)) +} + export async function loadLazyWithRetry( factory: LazyFactory, options: LazyWithRetryOptions = {} @@ -91,8 +128,11 @@ export async function loadLazyWithRetry( } } - if (typeof window !== 'undefined' && !hasAttemptedChunkReload()) { - markChunkReloadAttempted() + const reloadGuardState = readChunkReloadGuardState() + if (typeof window !== 'undefined' && reloadGuardState === 'not-attempted') { + if (!markChunkReloadAttempted()) { + throw lastError + } recordReloadBreadcrumb( options.reloadKey ?? 'unknown', lastError instanceof Error ? lastError.message : String(lastError) @@ -101,8 +141,12 @@ export async function loadLazyWithRetry( return SUSPEND_UNTIL_RELOAD } - // Already reloaded once this session, or no window (SSR / node): re-throw so - // RecoverableRenderErrorBoundary catches and reports it instead of looping. + if (reloadGuardState === 'attempted' && isKnownDynamicImportFailure(lastError)) { + throw new LazyChunkLoadError(lastError) + } + + // No proven reload attempt (SSR / node / blocked storage) or unknown failure: + // re-throw the original error so normal error reporting semantics stay intact. throw lastError }