diff --git a/apps/desktop/src/components/assistant-ui/message-render-boundary.test.tsx b/apps/desktop/src/components/assistant-ui/message-render-boundary.test.tsx index 8e1b70f7934f..2a8d8d85c65c 100644 --- a/apps/desktop/src/components/assistant-ui/message-render-boundary.test.tsx +++ b/apps/desktop/src/components/assistant-ui/message-render-boundary.test.tsx @@ -15,6 +15,12 @@ function Boom({ error }: { error: Error | null }): null { const lookupError = new Error('useClientLookup: Index 2 out of bounds (length: 2)') +const lookupErrors = [ + ['useClientLookup', lookupError], + ['tapClientLookup', new Error('tapClientLookup: Index 2 out of bounds (length: 2)')], + ['tapClientResource', new Error('tapClientResource: Index 2 out of bounds (length: 2)')] +] as const + describe('MessageRenderBoundary', () => { it('renders children when nothing throws', () => { render( @@ -26,12 +32,12 @@ describe('MessageRenderBoundary', () => { expect(screen.getByText('content')).toBeTruthy() }) - it('swallows the transient useClientLookup out-of-bounds store race', () => { + it.each(lookupErrors)('swallows the transient %s out-of-bounds store race', (_label, error) => { const spy = vi.spyOn(console, 'error').mockImplementation(() => undefined) const { container } = render( - + ) diff --git a/apps/desktop/src/components/error-boundary.test.tsx b/apps/desktop/src/components/error-boundary.test.tsx new file mode 100644 index 000000000000..853fe22036da --- /dev/null +++ b/apps/desktop/src/components/error-boundary.test.tsx @@ -0,0 +1,220 @@ +import { act, cleanup, fireEvent, render, screen } from '@testing-library/react' +import { StrictMode } from 'react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import { ErrorBoundary, RootErrorBoundary } from './error-boundary' + +const CURRENT_LOOKUP_ERROR = new Error('useClientLookup: Index 6 out of bounds (length: 2)') +const RELOAD_WINDOW = { name: 'Reload window', role: 'button' } as const + +function makeBomb(box: { error: Error | null }) { + return function Bomb() { + if (box.error) { + throw box.error + } + + return
recovered
+ } +} + +const recoveryWarningCount = (calls: unknown[][]) => + calls.filter(call => call.some(value => String(value).includes('auto-recovering from assistant-ui lookup'))).length + +describe('ErrorBoundary assistant-ui lookup recovery', () => { + beforeEach(() => { + vi.useFakeTimers() + vi.setSystemTime(0) + vi.spyOn(console, 'error').mockImplementation(() => undefined) + }) + + afterEach(() => { + cleanup() + vi.useRealTimers() + vi.restoreAllMocks() + }) + + it.each([ + ['the current useClientLookup error', CURRENT_LOOKUP_ERROR], + ['the legacy tapClientLookup error', new Error('tapClientLookup: Index 6 out of bounds (length: 2)')], + ['the legacy tapClientResource error', new Error('tapClientResource: Index 6 out of bounds (length: 2)')] + ])('recovers the production root composition without StrictMode after %s clears', (_label, error) => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined) + const box: { error: Error | null } = { error } + const Bomb = makeBomb(box) + + render( + + + + ) + + box.error = null + act(() => vi.runOnlyPendingTimers()) + + expect(screen.getByText('recovered')).toBeTruthy() + expect(screen.queryByRole(RELOAD_WINDOW.role, { name: RELOAD_WINDOW.name })).toBeNull() + expect(recoveryWarningCount(warnSpy.mock.calls)).toBe(1) + }) + + it('recovers the production root composition when StrictMode replays its mount lifecycle', () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined) + const box: { error: Error | null } = { error: CURRENT_LOOKUP_ERROR } + const Bomb = makeBomb(box) + + render( + + + + + + ) + + box.error = null + act(() => vi.runOnlyPendingTimers()) + + expect(screen.getByText('recovered')).toBeTruthy() + expect(recoveryWarningCount(warnSpy.mock.calls)).toBe(1) + expect(vi.getTimerCount()).toBe(0) + }) + + it('stops retrying a persistent lookup error after the recovery budget is exhausted', () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined) + const box: { error: Error | null } = { error: CURRENT_LOOKUP_ERROR } + const Bomb = makeBomb(box) + + render( + + + + ) + + for (let attempt = 0; attempt < 4; attempt += 1) { + act(() => vi.runOnlyPendingTimers()) + } + + expect(screen.getByRole(RELOAD_WINDOW.role, { name: RELOAD_WINDOW.name })).toBeTruthy() + expect(recoveryWarningCount(warnSpy.mock.calls)).toBe(3) + expect(vi.getTimerCount()).toBe(0) + }) + + it('starts a fresh automatic recovery budget at the 5 second window boundary', () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined) + const box: { error: Error | null } = { error: CURRENT_LOOKUP_ERROR } + const Bomb = makeBomb(box) + + const view = render( + + + + ) + + for (let attempt = 0; attempt < 3; attempt += 1) { + box.error = null + act(() => vi.runOnlyPendingTimers()) + expect(screen.getByText('recovered')).toBeTruthy() + + if (attempt < 2) { + box.error = CURRENT_LOOKUP_ERROR + view.rerender( + + + + ) + } + } + + expect(recoveryWarningCount(warnSpy.mock.calls)).toBe(3) + act(() => vi.advanceTimersByTime(5_000)) + + box.error = CURRENT_LOOKUP_ERROR + view.rerender( + + + + ) + expect(vi.getTimerCount()).toBe(1) + + box.error = null + act(() => vi.runOnlyPendingTimers()) + + expect(screen.getByText('recovered')).toBeTruthy() + expect(recoveryWarningCount(warnSpy.mock.calls)).toBe(4) + }) + + it('manual retry replaces an active timer, resets the budget, and can exhaust again', () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined) + const box: { error: Error | null } = { error: CURRENT_LOOKUP_ERROR } + const Bomb = makeBomb(box) + + render( + + + + ) + + expect(vi.getTimerCount()).toBe(1) + fireEvent.click(screen.getByRole('button', { name: 'Retry' })) + expect(vi.getTimerCount()).toBe(1) + + for (let attempt = 0; attempt < 3; attempt += 1) { + act(() => vi.runOnlyPendingTimers()) + } + + expect(screen.getByRole(RELOAD_WINDOW.role, { name: RELOAD_WINDOW.name })).toBeTruthy() + expect(recoveryWarningCount(warnSpy.mock.calls)).toBe(4) + expect(vi.getTimerCount()).toBe(0) + }) + + it('cancels an owned automatic recovery timer when unmounted', () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined) + const Bomb = makeBomb({ error: CURRENT_LOOKUP_ERROR }) + + const view = render( + + + + ) + + expect(vi.getTimerCount()).toBe(1) + view.unmount() + expect(vi.getTimerCount()).toBe(0) + act(() => vi.runAllTimers()) + expect(recoveryWarningCount(warnSpy.mock.calls)).toBe(1) + }) + + it('does not auto-recover the same error in a scoped boundary', () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined) + const Bomb = makeBomb({ error: CURRENT_LOOKUP_ERROR }) + + render( +
scoped fallback
} label="thread"> + +
+ ) + + act(() => vi.runAllTimers()) + + expect(screen.getByText('scoped fallback')).toBeTruthy() + expect(recoveryWarningCount(warnSpy.mock.calls)).toBe(0) + }) + + it.each([ + ['a differently cased classifier near-miss', new Error('UseClientLookup: Index 6 out of bounds (length: 2)')], + ['a non-bounds lookup error', new Error('useClientLookup: Key "missing" not found')], + ['an unrelated render error', new Error('some unrelated application error')] + ])('does not auto-recover %s at root', (_label, error) => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined) + const Bomb = makeBomb({ error }) + + render( + + + + ) + + act(() => vi.runAllTimers()) + + expect(screen.getByRole(RELOAD_WINDOW.role, { name: RELOAD_WINDOW.name })).toBeTruthy() + expect(recoveryWarningCount(warnSpy.mock.calls)).toBe(0) + }) +}) diff --git a/apps/desktop/src/components/error-boundary.tsx b/apps/desktop/src/components/error-boundary.tsx index f89f1d50673b..0597d777c8a6 100644 --- a/apps/desktop/src/components/error-boundary.tsx +++ b/apps/desktop/src/components/error-boundary.tsx @@ -20,20 +20,87 @@ interface ErrorBoundaryState { error: Error | null } +// Some assistant-ui lookup races escape the message-local boundary and reach +// the root. Retry only that exact transient error class, never arbitrary render +// failures, and cap retries so a persistent failure still exposes the fallback. +const ASSISTANT_UI_LOOKUP_ERROR = /(useClientLookup|tapClient(Lookup|Resource)).*out of bounds/ +const MAX_AUTO_RECOVERIES = 3 +const AUTO_RECOVERY_WINDOW_MS = 5_000 + +const isTransientAssistantUiLookupError = (error: Error): boolean => ASSISTANT_UI_LOOKUP_ERROR.test(error.message) + export class ErrorBoundary extends Component { state: ErrorBoundaryState = { error: null } + private autoRecoveryCount = 0 + private autoRecoveryPending = false + private autoRecoveryTimer: number | null = null + private autoRecoveryWindowStart = 0 static getDerivedStateFromError(error: Error): ErrorBoundaryState { return { error } } + componentDidMount() { + // StrictMode replays mount lifecycles in development. Its synthetic + // componentWillUnmount clears the timer scheduled by componentDidCatch, + // so restore the still-owned recovery on the matching remount. + if (this.autoRecoveryPending && this.autoRecoveryTimer === null) { + this.scheduleAutoRecovery() + } + } + componentDidCatch(error: Error, info: ErrorInfo) { const tag = this.props.label ? `[error-boundary:${this.props.label}]` : '[error-boundary]' console.error(tag, error, info.componentStack) this.props.onError?.(error, info) + + if (this.props.label === 'root' && isTransientAssistantUiLookupError(error) && this.takeAutoRecoveryAttempt()) { + console.warn(`${tag} auto-recovering from assistant-ui lookup render race`, error.message) + this.autoRecoveryPending = true + this.scheduleAutoRecovery() + } + } + + componentWillUnmount() { + this.clearAutoRecoveryTimer() } reset = () => { + this.clearAutoRecoveryTimer() + this.autoRecoveryPending = false + this.autoRecoveryCount = 0 + this.autoRecoveryWindowStart = 0 + this.setState({ error: null }) + } + + private takeAutoRecoveryAttempt(): boolean { + const now = Date.now() + + if (this.autoRecoveryCount === 0 || now - this.autoRecoveryWindowStart >= AUTO_RECOVERY_WINDOW_MS) { + this.autoRecoveryWindowStart = now + this.autoRecoveryCount = 0 + } + + this.autoRecoveryCount += 1 + + return this.autoRecoveryCount <= MAX_AUTO_RECOVERIES + } + + private clearAutoRecoveryTimer() { + if (this.autoRecoveryTimer !== null) { + window.clearTimeout(this.autoRecoveryTimer) + this.autoRecoveryTimer = null + } + } + + private scheduleAutoRecovery() { + this.clearAutoRecoveryTimer() + this.autoRecoveryTimer = window.setTimeout(this.autoRecover, 0) + } + + private autoRecover = () => { + this.autoRecoveryTimer = null + this.autoRecoveryPending = false this.setState({ error: null }) } @@ -52,6 +119,10 @@ export class ErrorBoundary extends Component{children} +} + function RootErrorFallback({ error, reset }: ErrorBoundaryFallbackProps) { const { t } = useI18n() diff --git a/apps/desktop/src/main.tsx b/apps/desktop/src/main.tsx index a71427c74818..7356ca59d445 100644 --- a/apps/desktop/src/main.tsx +++ b/apps/desktop/src/main.tsx @@ -19,7 +19,7 @@ import { createRoot } from 'react-dom/client' import { HashRouter } from 'react-router' import App from './app' -import { ErrorBoundary } from './components/error-boundary' +import { RootErrorBoundary } from './components/error-boundary' import { HapticsProvider } from './components/haptics-provider' import { RootTooltipProvider } from './components/ui/tooltip' import { I18nProvider } from './i18n' @@ -48,7 +48,7 @@ if (winParam === 'overlay') { } else { createRoot(document.getElementById('root')!).render( - + @@ -76,7 +76,7 @@ if (winParam === 'overlay') { - + ) }