Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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(
<MessageRenderBoundary resetKey="a">
<Boom error={lookupError} />
<Boom error={error} />
</MessageRenderBoundary>
)

Expand Down
220 changes: 220 additions & 0 deletions apps/desktop/src/components/error-boundary.test.tsx
Original file line number Diff line number Diff line change
@@ -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 <div>recovered</div>
}
}

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(
<RootErrorBoundary>
<Bomb />
</RootErrorBoundary>
)

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(
<StrictMode>
<RootErrorBoundary>
<Bomb />
</RootErrorBoundary>
</StrictMode>
)

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(
<RootErrorBoundary>
<Bomb />
</RootErrorBoundary>
)

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(
<RootErrorBoundary>
<Bomb />
</RootErrorBoundary>
)

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(
<RootErrorBoundary>
<Bomb />
</RootErrorBoundary>
)
}
}

expect(recoveryWarningCount(warnSpy.mock.calls)).toBe(3)
act(() => vi.advanceTimersByTime(5_000))

box.error = CURRENT_LOOKUP_ERROR
view.rerender(
<RootErrorBoundary>
<Bomb />
</RootErrorBoundary>
)
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(
<RootErrorBoundary>
<Bomb />
</RootErrorBoundary>
)

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(
<RootErrorBoundary>
<Bomb />
</RootErrorBoundary>
)

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(
<ErrorBoundary fallback={() => <div>scoped fallback</div>} label="thread">
<Bomb />
</ErrorBoundary>
)

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(
<RootErrorBoundary>
<Bomb />
</RootErrorBoundary>
)

act(() => vi.runAllTimers())

expect(screen.getByRole(RELOAD_WINDOW.role, { name: RELOAD_WINDOW.name })).toBeTruthy()
expect(recoveryWarningCount(warnSpy.mock.calls)).toBe(0)
})
})
71 changes: 71 additions & 0 deletions apps/desktop/src/components/error-boundary.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<ErrorBoundaryProps, ErrorBoundaryState> {
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 })
}

Expand All @@ -52,6 +119,10 @@ export class ErrorBoundary extends Component<ErrorBoundaryProps, ErrorBoundarySt
}
}

export function RootErrorBoundary({ children }: { children: ReactNode }) {
return <ErrorBoundary label="root">{children}</ErrorBoundary>
}

function RootErrorFallback({ error, reset }: ErrorBoundaryFallbackProps) {
const { t } = useI18n()

Expand Down
6 changes: 3 additions & 3 deletions apps/desktop/src/main.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -48,7 +48,7 @@ if (winParam === 'overlay') {
} else {
createRoot(document.getElementById('root')!).render(
<StrictMode>
<ErrorBoundary label="root">
<RootErrorBoundary>
<QueryClientProvider client={queryClient}>
<I18nProvider>
<ThemeProvider>
Expand Down Expand Up @@ -76,7 +76,7 @@ if (winParam === 'overlay') {
</ThemeProvider>
</I18nProvider>
</QueryClientProvider>
</ErrorBoundary>
</RootErrorBoundary>
</StrictMode>
)
}
Loading