From 24b7ca725812e9afd3c1a1aebce084a2e43aaa35 Mon Sep 17 00:00:00 2001 From: Jakub Wolniewicz <4850809+frizikk@users.noreply.github.com> Date: Thu, 30 Jul 2026 07:17:14 +0000 Subject: [PATCH] fix(desktop): preserve root recovery through StrictMode replay --- .../message-render-boundary.test.tsx | 10 +- .../src/components/error-boundary.test.tsx | 130 ++++++++++++++++-- .../desktop/src/components/error-boundary.tsx | 21 ++- apps/desktop/src/main.tsx | 6 +- 4 files changed, 149 insertions(+), 18 deletions(-) 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 e4a890ef5a..e9dd8ee7d6 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 @@ -40,6 +40,12 @@ class RecordingBoundary extends Component<{ children: ReactNode }, { error: Erro } } +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( @@ -51,12 +57,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 index 549ab4aad9..853fe22036 100644 --- a/apps/desktop/src/components/error-boundary.test.tsx +++ b/apps/desktop/src/components/error-boundary.test.tsx @@ -1,7 +1,8 @@ -import { act, cleanup, render, screen } from '@testing-library/react' +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 } from './error-boundary' +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 @@ -22,6 +23,7 @@ const recoveryWarningCount = (calls: unknown[][]) => describe('ErrorBoundary assistant-ui lookup recovery', () => { beforeEach(() => { vi.useFakeTimers() + vi.setSystemTime(0) vi.spyOn(console, 'error').mockImplementation(() => undefined) }) @@ -35,15 +37,15 @@ describe('ErrorBoundary assistant-ui lookup recovery', () => { ['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 root boundary after %s clears', (_label, error) => { + ])('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 @@ -54,15 +56,36 @@ describe('ErrorBoundary assistant-ui lookup recovery', () => { 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) { @@ -74,6 +97,91 @@ describe('ErrorBoundary assistant-ui lookup recovery', () => { 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 }) @@ -91,17 +199,17 @@ describe('ErrorBoundary assistant-ui lookup recovery', () => { }) 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 box: { error: Error | null } = { error } - const Bomb = makeBomb(box) + const Bomb = makeBomb({ error }) render( - + - + ) act(() => vi.runAllTimers()) diff --git a/apps/desktop/src/components/error-boundary.tsx b/apps/desktop/src/components/error-boundary.tsx index 5f5a8eaef4..0597d777c8 100644 --- a/apps/desktop/src/components/error-boundary.tsx +++ b/apps/desktop/src/components/error-boundary.tsx @@ -23,7 +23,7 @@ interface ErrorBoundaryState { // 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/i +const ASSISTANT_UI_LOOKUP_ERROR = /(useClientLookup|tapClient(Lookup|Resource)).*out of bounds/ const MAX_AUTO_RECOVERIES = 3 const AUTO_RECOVERY_WINDOW_MS = 5_000 @@ -32,6 +32,7 @@ const isTransientAssistantUiLookupError = (error: Error): boolean => ASSISTANT_U export class ErrorBoundary extends Component { state: ErrorBoundaryState = { error: null } private autoRecoveryCount = 0 + private autoRecoveryPending = false private autoRecoveryTimer: number | null = null private autoRecoveryWindowStart = 0 @@ -39,6 +40,15 @@ export class ErrorBoundary extends Component { this.clearAutoRecoveryTimer() + this.autoRecoveryPending = false this.autoRecoveryCount = 0 this.autoRecoveryWindowStart = 0 this.setState({ error: null }) @@ -64,7 +76,7 @@ export class ErrorBoundary extends Component AUTO_RECOVERY_WINDOW_MS) { + if (this.autoRecoveryCount === 0 || now - this.autoRecoveryWindowStart >= AUTO_RECOVERY_WINDOW_MS) { this.autoRecoveryWindowStart = now this.autoRecoveryCount = 0 } @@ -88,6 +100,7 @@ export class ErrorBoundary extends Component { this.autoRecoveryTimer = null + this.autoRecoveryPending = false this.setState({ error: null }) } @@ -106,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 a71427c748..7356ca59d4 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') { - + ) }