From 13e14eb80d57a3f6efb5b080790fc886de69cbc9 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 14 Aug 2026 18:13:08 -0700 Subject: [PATCH] fix(renderer): extend the stale-chunk Restart Orca recovery row to the rich-markdown editor boundary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extract the reviewer-hardened stale-chunk action row (Restart Orca primary, Retry secondary, sync double-click guard, resolve-anchored settle grace, stalled notice, unmount teardown) out of RecoverableRenderErrorBoundary into the shared StaleChunkRecoveryActions component, and render it from RichMarkdownErrorBoundary's fallback for LazyChunkLoadError on relaunch-capable hosts — the editor surface where the 9-hour field incident actually happened previously offered only a Retry that re-fetches dead assets forever. --- ...arkdownErrorBoundary.restart-orca.test.tsx | 154 ++++++++++++++++++ .../editor/RichMarkdownErrorBoundary.tsx | 30 ++++ ...eRenderErrorBoundary.restart-orca.test.tsx | 12 +- .../RecoverableRenderErrorBoundary.tsx | 126 +++----------- .../StaleChunkRecoveryActions.tsx | 126 ++++++++++++++ .../app-relaunch-capability.ts | 5 + 6 files changed, 344 insertions(+), 109 deletions(-) create mode 100644 src/renderer/src/components/editor/RichMarkdownErrorBoundary.restart-orca.test.tsx create mode 100644 src/renderer/src/components/error-boundaries/StaleChunkRecoveryActions.tsx create mode 100644 src/renderer/src/components/error-boundaries/app-relaunch-capability.ts diff --git a/src/renderer/src/components/editor/RichMarkdownErrorBoundary.restart-orca.test.tsx b/src/renderer/src/components/editor/RichMarkdownErrorBoundary.restart-orca.test.tsx new file mode 100644 index 00000000000..dcd54e91b49 --- /dev/null +++ b/src/renderer/src/components/editor/RichMarkdownErrorBoundary.restart-orca.test.tsx @@ -0,0 +1,154 @@ +// @vitest-environment happy-dom + +// The editor boundary is where the 9-hour field incident happened: an app +// update swaps assets under a live renderer, Retry re-fetches dead chunks +// forever, and without a "Restart Orca" affordance the user has no recovery. +// These tests pin that the shared stale-chunk recovery row renders here too — +// and only for stale chunks on relaunch-capable hosts. + +import { act, type ReactElement } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import { LazyChunkLoadError } from '@/lib/lazy-with-retry' +import { RELAUNCH_SETTLE_GRACE_MS } from '@/components/error-boundaries/StaleChunkRecoveryActions' +import { + clearLazyChunkBreadcrumbDedupeForTest, + RichMarkdownErrorBoundary +} from './RichMarkdownErrorBoundary' + +const reportCrashMock = vi.hoisted(() => vi.fn()) +const recordBreadcrumbMock = vi.hoisted(() => vi.fn()) + +vi.mock('@/lib/react-error-boundary-reporting', () => ({ + reportReactErrorBoundaryCrash: reportCrashMock +})) + +vi.mock('@/lib/crash-breadcrumb-recorder', () => ({ + recordRendererCrashBreadcrumb: recordBreadcrumbMock +})) + +globalThis.IS_REACT_ACT_ENVIRONMENT = true + +function ThrowingChild({ error }: { error: Error }): ReactElement { + throw error +} + +function findButtonByText(container: HTMLElement, text: string): HTMLButtonElement | null { + return ( + Array.from(container.querySelectorAll('button')).find((button) => + button.textContent?.includes(text) + ) ?? null + ) +} + +describe('RichMarkdownErrorBoundary Restart Orca row', () => { + let root: Root | null = null + let container: HTMLDivElement | null = null + let consoleError: ReturnType + + beforeEach(() => { + reportCrashMock.mockReset() + recordBreadcrumbMock.mockReset() + clearLazyChunkBreadcrumbDedupeForTest() + consoleError = vi.spyOn(console, 'error').mockImplementation(() => undefined) + }) + + afterEach(() => { + if (root) { + act(() => root?.unmount()) + } + container?.remove() + root = null + container = null + delete (window as unknown as { api?: unknown }).api + consoleError.mockRestore() + vi.useRealTimers() + }) + + function renderBoundaryWith(error: Error, fileId = 'file-1'): void { + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) + act(() => { + root?.render( + + + + ) + }) + } + + it('renders the shared recovery row for a stale chunk when the host can relaunch', () => { + const relaunch = vi.fn<() => Promise>().mockReturnValue(new Promise(() => undefined)) + ;(window as unknown as { api: unknown }).api = { app: { relaunch } } + + renderBoundaryWith(new LazyChunkLoadError(new SyntaxError("Unexpected token '}'"))) + + expect(container!.textContent).toContain('This part of Orca could not load.') + const restart = findButtonByText(container!, 'Restart Orca') + expect(restart).not.toBeNull() + + // Restart leads as primary; Retry is the lower-emphasis sibling. + const buttons = Array.from(container!.querySelectorAll('button')) + expect(buttons[0]).toBe(restart) + expect(restart?.dataset.variant).toBe('default') + expect(findButtonByText(container!, 'Retry')?.dataset.variant).toBe('secondary') + + act(() => restart?.click()) + expect(relaunch).toHaveBeenCalledTimes(1) + expect(restart?.disabled).toBe(true) + expect(findButtonByText(container!, 'Retry')?.disabled).toBe(true) + }) + + it('keeps the plain editor fallback for ordinary errors even with the bridge', () => { + ;(window as unknown as { api: unknown }).api = { app: { relaunch: vi.fn() } } + + renderBoundaryWith(new Error('ordinary render failure')) + + expect(findButtonByText(container!, 'Restart Orca')).toBeNull() + expect(container!.textContent).toContain('rich markdown editor hit an unexpected error') + expect(findButtonByText(container!, 'Retry')).not.toBeNull() + }) + + it('keeps the plain editor fallback for a stale chunk on hosts without the bridge', () => { + delete (window as unknown as { api?: unknown }).api + + renderBoundaryWith(new LazyChunkLoadError(new SyntaxError("Unexpected token '}'"))) + + expect(findButtonByText(container!, 'Restart Orca')).toBeNull() + expect(container!.textContent).toContain('rich markdown editor hit an unexpected error') + expect(findButtonByText(container!, 'Retry')).not.toBeNull() + }) + + it('does not leak the stalled-restart notice into a later error after a fileId re-key', async () => { + vi.useFakeTimers() + const relaunch = vi.fn<() => Promise>().mockResolvedValue(undefined) + ;(window as unknown as { api: unknown }).api = { app: { relaunch } } + + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) + const renderWith = (fileId: string, error: Error): void => { + act(() => { + root?.render( + + + + ) + }) + } + + renderWith('file-a', new LazyChunkLoadError(new SyntaxError("Unexpected token '}'"))) + act(() => findButtonByText(container!, 'Restart Orca')?.click()) + await act(async () => { + await vi.advanceTimersByTimeAsync(RELAUNCH_SETTLE_GRACE_MS + 1) + }) + expect(container!.textContent).toContain("Restarting didn't complete.") + + // The re-key abandons that fallback; a fresh stale error must start clean. + renderWith('file-b', new LazyChunkLoadError(new SyntaxError("Unexpected token ']'"))) + expect(findButtonByText(container!, 'Restart Orca')).not.toBeNull() + expect(container!.textContent).not.toContain("Restarting didn't complete.") + }) +}) diff --git a/src/renderer/src/components/editor/RichMarkdownErrorBoundary.tsx b/src/renderer/src/components/editor/RichMarkdownErrorBoundary.tsx index 3c7dd8fcc8d..d10e266cc36 100644 --- a/src/renderer/src/components/editor/RichMarkdownErrorBoundary.tsx +++ b/src/renderer/src/components/editor/RichMarkdownErrorBoundary.tsx @@ -2,6 +2,8 @@ import React from 'react' import { recordRendererCrashBreadcrumb } from '@/lib/crash-breadcrumb-recorder' import { advanceLazyChunkRetryEpoch, isLazyChunkLoadError } from '@/lib/lazy-with-retry' import { reportReactErrorBoundaryCrash } from '@/lib/react-error-boundary-reporting' +import { isAppRelaunchCapable } from '@/components/error-boundaries/app-relaunch-capability' +import { StaleChunkRecoveryActions } from '@/components/error-boundaries/StaleChunkRecoveryActions' import { translate } from '@/i18n/i18n' type Props = { @@ -83,6 +85,34 @@ export class RichMarkdownErrorBoundary extends React.Component { render(): React.ReactNode { if (this.state.error) { + // Stale chunks after a background update: Retry alone re-fetches dead + // assets forever, so lead with the main-driven restart (same row as + // RecoverableRenderErrorBoundary) whenever the host can relaunch. + const canRelaunch = isLazyChunkLoadError(this.state.error) && isAppRelaunchCapable() + if (canRelaunch) { + return ( +
+
+ {translate( + 'auto.components.error.boundaries.RecoverableRenderErrorBoundary.staleChunkTitle', + 'This part of Orca could not load.' + )} +
+
+ {translate( + 'auto.components.error.boundaries.RecoverableRenderErrorBoundary.staleChunkDescription', + 'Orca may have updated in the background. Restart Orca to finish applying the update, or retry this part.' + )} +
+ {/* Catching an error rebuilds this subtree, so the row remounts fresh + per error — no stalled-notice leak (pinned by the leak test). */} + +
+ ) + } return (
diff --git a/src/renderer/src/components/error-boundaries/RecoverableRenderErrorBoundary.restart-orca.test.tsx b/src/renderer/src/components/error-boundaries/RecoverableRenderErrorBoundary.restart-orca.test.tsx index da7c6c6c89f..8e5a9dd39ee 100644 --- a/src/renderer/src/components/error-boundaries/RecoverableRenderErrorBoundary.restart-orca.test.tsx +++ b/src/renderer/src/components/error-boundaries/RecoverableRenderErrorBoundary.restart-orca.test.tsx @@ -13,10 +13,8 @@ import { createRoot, type Root } from 'react-dom/client' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { LazyChunkLoadError } from '@/lib/lazy-with-retry' -import { - RecoverableRenderErrorBoundary, - RELAUNCH_SETTLE_GRACE_MS -} from './RecoverableRenderErrorBoundary' +import { RecoverableRenderErrorBoundary } from './RecoverableRenderErrorBoundary' +import { RELAUNCH_SETTLE_GRACE_MS } from './StaleChunkRecoveryActions' const reportCrashMock = vi.hoisted(() => vi.fn()) @@ -207,6 +205,12 @@ describe('RecoverableRenderErrorBoundary Restart Orca button', () => { expect(container!.querySelector('[role="alert"]')).not.toBeNull() expect(findRestartButton(container!)).toBeNull() expect(container!.textContent).not.toContain("Restarting didn't complete.") + + // A later stale error renders the row again — remounted fresh, not with the + // doc-a row's stalled/disabled state (catching an error rebuilds the subtree). + renderWith('doc-c', new LazyChunkLoadError(new SyntaxError("Unexpected token ']'"))) + expect(findRestartButton(container!)?.disabled).toBe(false) + expect(container!.textContent).not.toContain("Restarting didn't complete.") }) it('drops the settle timer when the boundary unmounts around the relaunch', async () => { diff --git a/src/renderer/src/components/error-boundaries/RecoverableRenderErrorBoundary.tsx b/src/renderer/src/components/error-boundaries/RecoverableRenderErrorBoundary.tsx index 23191b522e6..5839d196b94 100644 --- a/src/renderer/src/components/error-boundaries/RecoverableRenderErrorBoundary.tsx +++ b/src/renderer/src/components/error-boundaries/RecoverableRenderErrorBoundary.tsx @@ -4,6 +4,8 @@ import { Button } from '@/components/ui/button' import { cn } from '@/lib/utils' import { advanceLazyChunkRetryEpoch, isLazyChunkLoadError } from '@/lib/lazy-with-retry' import { reportReactErrorBoundaryCrash } from '@/lib/react-error-boundary-reporting' +import { isAppRelaunchCapable } from '@/components/error-boundaries/app-relaunch-capability' +import { StaleChunkRecoveryActions } from '@/components/error-boundaries/StaleChunkRecoveryActions' import type { ReactErrorBoundaryReportArgs } from '../../../../shared/crash-reporting' import { translate } from '@/i18n/i18n' @@ -28,35 +30,17 @@ type Props = { type State = { error: Error | null resetKey: Props['resetKey'] - relaunching: boolean - relaunchStalled: boolean } -// How long a clicked relaunch may leave this document alive before the fallback -// takes the buttons back. An Electron relaunch tears the window down ~150ms after -// the IPC resolves; a paired-web relaunch is an in-place reload that a broken -// document can swallow entirely, which would otherwise pin the button disabled. -export const RELAUNCH_SETTLE_GRACE_MS = 5_000 - export class RecoverableRenderErrorBoundary extends React.Component { state: State = { error: null, - resetKey: this.props.resetKey, - relaunching: false, - relaunchStalled: false + resetKey: this.props.resetKey } - // Sync double-click guard: the second click can reach the handler before the - // disabled re-render commits, and a doubled app.relaunch() spawns two instances. - private relaunchRequested = false - private relaunchSettleTimer: ReturnType | null = null - private unmounted = false - static getDerivedStateFromProps(props: Props, state: State): Partial | null { if (props.resetKey !== state.resetKey) { - // A reset abandons the old fallback, so a later error must not inherit its - // stalled-restart notice ("Try again" would name a button that isn't there). - return { error: null, resetKey: props.resetKey, relaunchStalled: false } + return { error: null, resetKey: props.resetKey } } return null } @@ -86,55 +70,8 @@ export class RecoverableRenderErrorBoundary extends React.Component { - this.setState({ error: null, relaunchStalled: false }) - } - - handleRelaunchApp = (): void => { - if (this.relaunchRequested) { - return - } - this.relaunchRequested = true - this.setState({ relaunching: true, relaunchStalled: false }) - void window.api.app.relaunch().then( - () => { - // Why: a resolved relaunch that leaves this document alive went nowhere - // (swallowed in-place reload, teardown that never came); give the - // buttons back with a notice instead of leaving a dead disabled control. - // The grace must NOT start while the invoke is still pending — a slow - // pre-relaunch checkpoint is normal, and re-arming the button mid-invoke - // could double app.relaunch() into two replacement instances. - if (!this.unmounted) { - this.relaunchSettleTimer = setTimeout( - () => this.markRelaunchStalled(), - RELAUNCH_SETTLE_GRACE_MS - ) - } - }, - (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.markRelaunchStalled() - } - ) - } - - private clearRelaunchSettleTimer(): void { - if (this.relaunchSettleTimer !== null) { - clearTimeout(this.relaunchSettleTimer) - this.relaunchSettleTimer = null - } - } - - private markRelaunchStalled(): void { - this.clearRelaunchSettleTimer() - this.relaunchRequested = false - this.setState({ relaunching: false, relaunchStalled: true }) + this.setState({ error: null }) } render(): React.ReactNode { @@ -149,7 +86,7 @@ export class RecoverableRenderErrorBoundary extends React.Component
-
- {canRelaunch ? ( - // A broken document is not the place to recover itself: the main-driven - // relaunch leads, and the in-place Retry stays as the lighter sibling - // for the case where assets have already settled. - - ) : null} - -
- {this.state.relaunchStalled ? ( -
- {translate( - 'auto.components.error.boundaries.RecoverableRenderErrorBoundary.restartStalled', - "Restarting didn't complete. Try again, or retry this part of Orca." - )}
- ) : null} + )}
) } diff --git a/src/renderer/src/components/error-boundaries/StaleChunkRecoveryActions.tsx b/src/renderer/src/components/error-boundaries/StaleChunkRecoveryActions.tsx new file mode 100644 index 00000000000..137c3a1efed --- /dev/null +++ b/src/renderer/src/components/error-boundaries/StaleChunkRecoveryActions.tsx @@ -0,0 +1,126 @@ +import { useEffect, useRef, useState, type ReactElement } from 'react' +import { RotateCw } from 'lucide-react' +import { Button } from '@/components/ui/button' +import { translate } from '@/i18n/i18n' + +// How long a clicked relaunch may leave this document alive before the fallback +// takes the buttons back. An Electron relaunch tears the window down ~150ms after +// the IPC resolves; a paired-web relaunch is an in-place reload that a broken +// document can swallow entirely, which would otherwise pin the button disabled. +export const RELAUNCH_SETTLE_GRACE_MS = 5_000 + +type Props = { + /** Names the owning boundary on relaunch-failure logs. */ + boundaryId: string + onRetry: () => void +} + +/** + * The stale-chunk recovery row shared by every lazy-chunk error boundary: a + * broken document is not the place to recover itself, so the main-driven + * "Restart Orca" leads and the in-place Retry stays as the lighter sibling for + * the case where assets have already settled. + * + * Render only from an error boundary's fallback: catching an error rebuilds the + * boundary's subtree, which is what keeps a stalled-restart notice from leaking + * onto a later error's fallback (pinned by the boundaries' leak tests). + */ +export function StaleChunkRecoveryActions({ boundaryId, onRetry }: Props): ReactElement { + const [relaunching, setRelaunching] = useState(false) + const [relaunchStalled, setRelaunchStalled] = useState(false) + // Sync double-click guard: the second click can reach the handler before the + // disabled re-render commits, and a doubled app.relaunch() spawns two instances. + const relaunchRequestedRef = useRef(false) + const relaunchSettleTimerRef = useRef | null>(null) + const unmountedRef = useRef(false) + + useEffect(() => { + // Re-arm after a StrictMode probe cleanup; real teardown must both stop a + // pending invoke from scheduling and drop an already-armed settle timer. + unmountedRef.current = false + return () => { + unmountedRef.current = true + if (relaunchSettleTimerRef.current !== null) { + clearTimeout(relaunchSettleTimerRef.current) + relaunchSettleTimerRef.current = null + } + } + }, []) + + const markRelaunchStalled = (): void => { + if (relaunchSettleTimerRef.current !== null) { + clearTimeout(relaunchSettleTimerRef.current) + relaunchSettleTimerRef.current = null + } + relaunchRequestedRef.current = false + setRelaunching(false) + setRelaunchStalled(true) + } + + const handleRelaunchApp = (): void => { + if (relaunchRequestedRef.current) { + return + } + relaunchRequestedRef.current = true + setRelaunching(true) + setRelaunchStalled(false) + void window.api.app.relaunch().then( + () => { + // Why: a resolved relaunch that leaves this document alive went nowhere + // (swallowed in-place reload, teardown that never came); give the + // buttons back with a notice instead of leaving a dead disabled control. + // The grace must NOT start while the invoke is still pending — a slow + // pre-relaunch checkpoint is normal, and re-arming the button mid-invoke + // could double app.relaunch() into two replacement instances. + if (!unmountedRef.current) { + relaunchSettleTimerRef.current = setTimeout(markRelaunchStalled, RELAUNCH_SETTLE_GRACE_MS) + } + }, + (error: unknown) => { + // Why: a refused pre-relaunch checkpoint keeps the app open; re-enable the button. + console.error(`[${boundaryId}] app relaunch failed`, error) + markRelaunchStalled() + } + ) + } + + return ( + <> +
+ + +
+ {relaunchStalled ? ( +
+ {translate( + 'auto.components.error.boundaries.RecoverableRenderErrorBoundary.restartStalled', + "Restarting didn't complete. Try again, or retry this part of Orca." + )} +
+ ) : null} + + ) +} diff --git a/src/renderer/src/components/error-boundaries/app-relaunch-capability.ts b/src/renderer/src/components/error-boundaries/app-relaunch-capability.ts new file mode 100644 index 00000000000..1d1260a0929 --- /dev/null +++ b/src/renderer/src/components/error-boundaries/app-relaunch-capability.ts @@ -0,0 +1,5 @@ +// Degraded/partial preload hosts without the app bridge simply do not get a +// "Restart Orca" affordance; recovery falls back to plain Retry. +export function isAppRelaunchCapable(): boolean { + return typeof window.api?.app?.relaunch === 'function' +}