mirror of
https://github.com/stablyai/orca.git
synced 2026-10-28 00:02:54 +00:00
fix(react): escalate swallowed React #185 instead of painting it as UI state
A try/catch around an async setState is not an error boundary. When React's nested-update limit trips, the throw lands in whichever awaited continuation called setState next — usually a catch written for network failures. The catch then paints the digest as ordinary copy and lets React's module-global counter reset to 0, so the loop re-arms and throws again on a different fiber. Adds escalateReactUpdateDepthError(error, siteId): one guard, reusing the #185 detector from #16153, that re-reports with the catching site's identity (the only attribution #185 has) and re-raises on the host. Applied to the six ranked sinks, GitHub and GitLab together.
This commit is contained in:
@@ -1,6 +1,7 @@
|
||||
import { useEffect, useRef } from 'react'
|
||||
import { useShallow } from 'zustand/react/shallow'
|
||||
import { syncZoomCSSVar } from '@/lib/ui-zoom'
|
||||
import { escalateReactUpdateDepthError } from '@/lib/react-update-depth-escalation'
|
||||
import { installCodexDetachedPaneRestartExecutor } from '@/components/terminal-pane/codex-detached-pane-restart-scheduler'
|
||||
import { useAppStore } from '../store'
|
||||
import { WORKTREE_REFRESH_CONCURRENCY } from '../store/slices/worktrees'
|
||||
@@ -34,6 +35,9 @@ import {
|
||||
import { mapWithConcurrency } from '../../../shared/map-with-concurrency'
|
||||
import type { OnboardingState } from '../../../shared/onboarding-state-types'
|
||||
|
||||
const STARTUP_CATALOG_REFRESH_CATCH = 'startup-hydration.remote-catalog-refresh'
|
||||
const STARTUP_WORKTREE_REFRESH_CATCH = 'startup-hydration.deferred-worktree-refresh'
|
||||
|
||||
async function listRuntimeSessionHostIdsForStartup(): Promise<ExecutionHostId[]> {
|
||||
try {
|
||||
return (await window.api.runtimeEnvironments.list()).map((environment) =>
|
||||
@@ -297,7 +301,10 @@ export function useAppStartupHydration(onOnboardingLoaded: (state: OnboardingSta
|
||||
await actions.fetchFolderWorkspacesForAllHosts()
|
||||
})
|
||||
} catch (err) {
|
||||
console.warn('Remote startup catalog refresh failed:', err)
|
||||
// A console.warn is no signal: a #185 swallowed here half-completes startup.
|
||||
if (!escalateReactUpdateDepthError(err, STARTUP_CATALOG_REFRESH_CATCH)) {
|
||||
console.warn('Remote startup catalog refresh failed:', err)
|
||||
}
|
||||
}
|
||||
if (!cancelled) {
|
||||
try {
|
||||
@@ -311,7 +318,9 @@ export function useAppStartupHydration(onOnboardingLoaded: (state: OnboardingSta
|
||||
await actions.fetchWorktreeLineage()
|
||||
})
|
||||
} catch (err) {
|
||||
console.warn('Deferred startup worktree refresh failed:', err)
|
||||
if (!escalateReactUpdateDepthError(err, STARTUP_WORKTREE_REFRESH_CATCH)) {
|
||||
console.warn('Deferred startup worktree refresh failed:', err)
|
||||
}
|
||||
}
|
||||
}
|
||||
} finally {
|
||||
|
||||
@@ -0,0 +1,122 @@
|
||||
// @vitest-environment happy-dom
|
||||
|
||||
/**
|
||||
* Provider parity: both rate-limit hooks must escalate React #185 rather than paint it as a
|
||||
* provider outage. A GitHub-only guard would leave GitLab users unprotected.
|
||||
*/
|
||||
|
||||
import { act } from 'react'
|
||||
import { createRoot, type Root } from 'react-dom/client'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { useGitHubRateLimitSnapshot } from './github/github-rate-limit-display'
|
||||
import { useGitLabRateLimitSnapshot } from './gitlab/gitlab-rate-limit-display'
|
||||
import { clearReactErrorBoundaryReportingForTest } from '@/lib/react-error-boundary-reporting'
|
||||
import {
|
||||
flushReactUpdateDepthEscalationsForTest,
|
||||
resetReactUpdateDepthEscalationForTest
|
||||
} from '@/lib/react-update-depth-escalation'
|
||||
|
||||
const REACT_185 =
|
||||
'Minified React error #185; visit https://react.dev/errors/185 for the full message.'
|
||||
|
||||
const mockStoreState = {
|
||||
settings: {},
|
||||
activeView: 'settings',
|
||||
activeModal: 'settings',
|
||||
activeTabType: null,
|
||||
rightSidebarTab: null,
|
||||
activeWorktreeId: null
|
||||
}
|
||||
|
||||
vi.mock('@/store', () => ({
|
||||
useAppStore: Object.assign((selector: (state: unknown) => unknown) => selector(mockStoreState), {
|
||||
getState: () => mockStoreState
|
||||
})
|
||||
}))
|
||||
|
||||
vi.mock('@/runtime/runtime-rpc-client', () => ({
|
||||
getActiveRuntimeTarget: () => ({ kind: 'local' }),
|
||||
callRuntimeRpc: vi.fn()
|
||||
}))
|
||||
|
||||
const recordRendererError = vi.fn()
|
||||
const ghRateLimit = vi.fn()
|
||||
const glRateLimit = vi.fn()
|
||||
const roots: Root[] = []
|
||||
|
||||
type HookResult = { hasError: boolean }
|
||||
|
||||
function renderHook(useHook: () => HookResult): { current: HookResult | null } {
|
||||
const ref: { current: HookResult | null } = { current: null }
|
||||
function Probe(): null {
|
||||
ref.current = useHook()
|
||||
return null
|
||||
}
|
||||
const container = document.createElement('div')
|
||||
document.body.appendChild(container)
|
||||
const root = createRoot(container)
|
||||
roots.push(root)
|
||||
act(() => {
|
||||
root.render(<Probe />)
|
||||
})
|
||||
return ref
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
globalThis.IS_REACT_ACT_ENVIRONMENT = true
|
||||
recordRendererError.mockReset()
|
||||
recordRendererError.mockResolvedValue({ ok: true, report: null, deduped: false })
|
||||
ghRateLimit.mockReset().mockRejectedValue(new Error(REACT_185))
|
||||
glRateLimit.mockReset().mockRejectedValue(new Error(REACT_185))
|
||||
resetReactUpdateDepthEscalationForTest()
|
||||
clearReactErrorBoundaryReportingForTest()
|
||||
Object.assign(globalThis.window, {
|
||||
api: {
|
||||
gh: { rateLimit: ghRateLimit },
|
||||
gl: { rateLimit: glRateLimit },
|
||||
crashReports: { recordRendererError }
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
act(() => {
|
||||
for (const root of roots.splice(0)) {
|
||||
root.unmount()
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
describe.each([
|
||||
['github', () => useGitHubRateLimitSnapshot({ autoRefresh: false })],
|
||||
['gitlab', () => useGitLabRateLimitSnapshot({ autoRefresh: false })]
|
||||
])('%s rate limit refresh', (provider, useHook) => {
|
||||
it('escalates React #185 instead of reporting a provider error', async () => {
|
||||
const hook = renderHook(useHook as () => HookResult)
|
||||
|
||||
await act(async () => {
|
||||
await (hook.current as unknown as { refresh: () => Promise<void> }).refresh()
|
||||
})
|
||||
await flushReactUpdateDepthEscalationsForTest()
|
||||
|
||||
expect(hook.current?.hasError).toBe(false)
|
||||
expect(recordRendererError).toHaveBeenCalledTimes(1)
|
||||
expect(recordRendererError.mock.calls[0]?.[0]?.boundaryId).toContain(
|
||||
`${provider}-rate-limit-display.refreshSnapshot`
|
||||
)
|
||||
expect(recordRendererError.mock.calls[0]?.[0]?.attribution).toBe('unreliable')
|
||||
})
|
||||
|
||||
it('still reports an ordinary provider failure', async () => {
|
||||
ghRateLimit.mockRejectedValue(new Error('gh: HTTP 503'))
|
||||
glRateLimit.mockRejectedValue(new Error('glab: HTTP 503'))
|
||||
const hook = renderHook(useHook as () => HookResult)
|
||||
|
||||
await act(async () => {
|
||||
await (hook.current as unknown as { refresh: () => Promise<void> }).refresh()
|
||||
})
|
||||
|
||||
expect(hook.current?.hasError).toBe(true)
|
||||
expect(recordRendererError).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
@@ -2,6 +2,7 @@ import React, { useCallback, useEffect, useRef, useState } from 'react'
|
||||
import { Gauge, RefreshCw } from 'lucide-react'
|
||||
import { cn } from '@/lib/utils'
|
||||
import { installWindowVisibilityInterval } from '@/lib/window-visibility-interval'
|
||||
import { escalateReactUpdateDepthError } from '@/lib/react-update-depth-escalation'
|
||||
import { useAppStore } from '@/store'
|
||||
import { callRuntimeRpc, getActiveRuntimeTarget } from '@/runtime/runtime-rpc-client'
|
||||
import type {
|
||||
@@ -13,6 +14,7 @@ import { ProviderHostScopeControl } from '@/components/settings/ProviderHostScop
|
||||
import { translate } from '@/i18n/i18n'
|
||||
|
||||
const REFRESH_INTERVAL_MS = 60_000
|
||||
const GITHUB_RATE_LIMIT_CATCH = 'github-rate-limit-display.refreshSnapshot'
|
||||
|
||||
type BucketKey = 'core' | 'search' | 'graphql'
|
||||
|
||||
@@ -110,7 +112,12 @@ export function useGitHubRateLimitSnapshot(options?: { autoRefresh?: boolean }):
|
||||
} else {
|
||||
setHasError(true)
|
||||
}
|
||||
} catch {
|
||||
} catch (error) {
|
||||
// Why first: the call can succeed and setSnapshot still throw #185; hasError would
|
||||
// then read as a provider outage. Guarded on both providers by repo parity rule.
|
||||
if (escalateReactUpdateDepthError(error, GITHUB_RATE_LIMIT_CATCH)) {
|
||||
return
|
||||
}
|
||||
if (token === latestToken.current) {
|
||||
setHasError(true)
|
||||
}
|
||||
|
||||
@@ -3,6 +3,7 @@ import { Gauge, RefreshCw } from 'lucide-react'
|
||||
import { Button } from '@/components/ui/button'
|
||||
import { cn } from '@/lib/utils'
|
||||
import { installWindowVisibilityInterval } from '@/lib/window-visibility-interval'
|
||||
import { escalateReactUpdateDepthError } from '@/lib/react-update-depth-escalation'
|
||||
import { useAppStore } from '@/store'
|
||||
import { callRuntimeRpc, getActiveRuntimeTarget } from '@/runtime/runtime-rpc-client'
|
||||
import type {
|
||||
@@ -14,6 +15,7 @@ import { ProviderHostScopeControl } from '@/components/settings/ProviderHostScop
|
||||
import { translate } from '@/i18n/i18n'
|
||||
|
||||
const REFRESH_INTERVAL_MS = 60_000
|
||||
const GITLAB_RATE_LIMIT_CATCH = 'gitlab-rate-limit-display.refreshSnapshot'
|
||||
|
||||
export function formatGitLabRateLimitReset(resetAt: number | null): string {
|
||||
if (resetAt === null) {
|
||||
@@ -79,7 +81,12 @@ export function useGitLabRateLimitSnapshot(options?: { autoRefresh?: boolean }):
|
||||
} else {
|
||||
setHasError(true)
|
||||
}
|
||||
} catch {
|
||||
} catch (error) {
|
||||
// Why first: the call can succeed and setSnapshot still throw #185; hasError would
|
||||
// then read as a provider outage. Guarded on both providers by repo parity rule.
|
||||
if (escalateReactUpdateDepthError(error, GITLAB_RATE_LIMIT_CATCH)) {
|
||||
return
|
||||
}
|
||||
if (token === latestToken.current) {
|
||||
setHasError(true)
|
||||
}
|
||||
|
||||
@@ -15,9 +15,12 @@ import {
|
||||
syncSurfacesAfterAgentSkillRecheck
|
||||
} from './agent-skill-recheck-surface-sync'
|
||||
import { isOrcaCliAvailableOnPath } from '@/lib/agent-skill-cli-prerequisite'
|
||||
import { escalateReactUpdateDepthError } from '@/lib/react-update-depth-escalation'
|
||||
import { cn } from '@/lib/utils'
|
||||
import { translate } from '@/i18n/i18n'
|
||||
|
||||
const PRE_INSTALL_NOTICE_CATCH = 'settings.AgentSkillSetupPanel.applyPreInstallNotice'
|
||||
|
||||
export function AgentSkillSetupPanel({
|
||||
title,
|
||||
description,
|
||||
@@ -138,6 +141,28 @@ export function AgentSkillSetupPanel({
|
||||
void (shouldRecheck && recheckSurfacesAfterAgentSkillTerminal(onRecheck, freshnessSkillName))
|
||||
}, [freshnessSkillName, mountedRef, onRecheck])
|
||||
|
||||
// Why shared: the focus listener and the pre-terminal refresh ran the same probe with
|
||||
// different liveness guards, so one #185 escalation now covers both callers.
|
||||
const applyPreInstallNotice = useCallback(
|
||||
async (isLive: () => boolean): Promise<void> => {
|
||||
try {
|
||||
const status = await readPrerequisiteStatus()
|
||||
if (isLive()) {
|
||||
setPreInstallNoticeVisible(!isPrerequisiteAvailable(status))
|
||||
}
|
||||
} catch (error) {
|
||||
// Why first: openSetupTerminal awaits this too, and its catch would swallow it again.
|
||||
if (escalateReactUpdateDepthError(error, PRE_INSTALL_NOTICE_CATCH)) {
|
||||
return
|
||||
}
|
||||
if (isLive()) {
|
||||
setPreInstallNoticeVisible(true)
|
||||
}
|
||||
}
|
||||
},
|
||||
[isPrerequisiteAvailable, readPrerequisiteStatus]
|
||||
)
|
||||
|
||||
useEffect(() => {
|
||||
if (!preInstallNotice) {
|
||||
setPreInstallNoticeVisible(false)
|
||||
@@ -145,18 +170,7 @@ export function AgentSkillSetupPanel({
|
||||
}
|
||||
|
||||
let canceled = false
|
||||
const refreshCliNotice = async (): Promise<void> => {
|
||||
try {
|
||||
const status = await readPrerequisiteStatus()
|
||||
if (!canceled) {
|
||||
setPreInstallNoticeVisible(!isPrerequisiteAvailable(status))
|
||||
}
|
||||
} catch {
|
||||
if (!canceled) {
|
||||
setPreInstallNoticeVisible(true)
|
||||
}
|
||||
}
|
||||
}
|
||||
const refreshCliNotice = (): Promise<void> => applyPreInstallNotice(() => !canceled)
|
||||
|
||||
void refreshCliNotice()
|
||||
window.addEventListener('focus', refreshCliNotice)
|
||||
@@ -164,21 +178,11 @@ export function AgentSkillSetupPanel({
|
||||
canceled = true
|
||||
window.removeEventListener('focus', refreshCliNotice)
|
||||
}
|
||||
}, [isPrerequisiteAvailable, preInstallNotice, readPrerequisiteStatus])
|
||||
}, [applyPreInstallNotice, preInstallNotice])
|
||||
|
||||
const refreshPreInstallNotice = async (): Promise<void> => {
|
||||
if (!preInstallNotice) {
|
||||
return
|
||||
}
|
||||
try {
|
||||
const status = await readPrerequisiteStatus()
|
||||
if (mountedRef.current) {
|
||||
setPreInstallNoticeVisible(!isPrerequisiteAvailable(status))
|
||||
}
|
||||
} catch {
|
||||
if (mountedRef.current) {
|
||||
setPreInstallNoticeVisible(true)
|
||||
}
|
||||
if (preInstallNotice) {
|
||||
await applyPreInstallNotice(() => mountedRef.current)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,108 @@
|
||||
// @vitest-environment happy-dom
|
||||
|
||||
/**
|
||||
* Field sighting: #185 rendered as red inline text under the build picker while the app crawled.
|
||||
* That text was `loadError`, not a boundary fallback — loadBuilds' catch swallowed the throw.
|
||||
*/
|
||||
|
||||
import { act } from 'react'
|
||||
import { createRoot, type Root } from 'react-dom/client'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { ReleaseChannelSection } from './ReleaseChannelSection'
|
||||
import { TooltipProvider } from '../ui/tooltip'
|
||||
import { clearReactErrorBoundaryReportingForTest } from '@/lib/react-error-boundary-reporting'
|
||||
import {
|
||||
flushReactUpdateDepthEscalationsForTest,
|
||||
resetReactUpdateDepthEscalationForTest
|
||||
} from '@/lib/react-update-depth-escalation'
|
||||
|
||||
const REACT_185 =
|
||||
'Minified React error #185; visit https://react.dev/errors/185 for the full message or use the non-minified dev environment for full errors and additional helpful warnings.'
|
||||
|
||||
const mockStoreState = {
|
||||
updateStatus: { state: 'idle' as const },
|
||||
releaseChannelOverride: null,
|
||||
setReleaseChannelOverride: vi.fn(),
|
||||
activeView: 'settings',
|
||||
activeModal: 'settings',
|
||||
activeTabType: null,
|
||||
rightSidebarTab: null,
|
||||
activeWorktreeId: null
|
||||
}
|
||||
|
||||
vi.mock('@/store', () => ({
|
||||
useAppStore: Object.assign((selector: (state: unknown) => unknown) => selector(mockStoreState), {
|
||||
getState: () => mockStoreState
|
||||
})
|
||||
}))
|
||||
|
||||
vi.mock('sonner', () => ({ toast: { error: vi.fn(), success: vi.fn() } }))
|
||||
|
||||
const recordRendererError = vi.fn()
|
||||
const listBuilds = vi.fn()
|
||||
const roots: Root[] = []
|
||||
|
||||
beforeEach(() => {
|
||||
globalThis.IS_REACT_ACT_ENVIRONMENT = true
|
||||
recordRendererError.mockReset()
|
||||
recordRendererError.mockResolvedValue({ ok: true, report: null, deduped: false })
|
||||
listBuilds.mockReset()
|
||||
resetReactUpdateDepthEscalationForTest()
|
||||
clearReactErrorBoundaryReportingForTest()
|
||||
Object.assign(globalThis.window, {
|
||||
api: {
|
||||
updater: {
|
||||
getVersion: vi.fn().mockResolvedValue('1.4.163'),
|
||||
listBuilds
|
||||
},
|
||||
crashReports: { recordRendererError }
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
act(() => {
|
||||
for (const root of roots.splice(0)) {
|
||||
root.unmount()
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
async function render(): Promise<HTMLDivElement> {
|
||||
const container = document.createElement('div')
|
||||
document.body.appendChild(container)
|
||||
const root = createRoot(container)
|
||||
roots.push(root)
|
||||
await act(async () => {
|
||||
root.render(
|
||||
<TooltipProvider>
|
||||
<ReleaseChannelSection />
|
||||
</TooltipProvider>
|
||||
)
|
||||
})
|
||||
return container
|
||||
}
|
||||
|
||||
describe('ReleaseChannelSection build load', () => {
|
||||
it('escalates a swallowed React #185 instead of painting it as a load error', async () => {
|
||||
listBuilds.mockRejectedValue(new Error(REACT_185))
|
||||
|
||||
const container = await render()
|
||||
await flushReactUpdateDepthEscalationsForTest()
|
||||
|
||||
expect(container.textContent).not.toContain('Minified React error #185')
|
||||
expect(recordRendererError).toHaveBeenCalledTimes(1)
|
||||
expect(recordRendererError.mock.calls[0]?.[0]?.attribution).toBe('unreliable')
|
||||
expect(recordRendererError.mock.calls[0]?.[0]?.boundaryId).toContain('ReleaseChannelSection')
|
||||
})
|
||||
|
||||
it('still shows an ordinary load failure inline', async () => {
|
||||
listBuilds.mockRejectedValue(new Error('getaddrinfo ENOTFOUND api.github.com'))
|
||||
|
||||
const container = await render()
|
||||
await flushReactUpdateDepthEscalationsForTest()
|
||||
|
||||
expect(container.textContent).toContain('getaddrinfo ENOTFOUND api.github.com')
|
||||
expect(recordRendererError).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
@@ -9,6 +9,7 @@ import { SettingsSegmentedControl, SettingsSubsectionHeader } from './SettingsFo
|
||||
import { Badge } from '../ui/badge'
|
||||
import { translate } from '@/i18n/i18n'
|
||||
import { getShortcutPlatform } from '@/lib/shortcut-platform'
|
||||
import { escalateReactUpdateDepthError } from '@/lib/react-update-depth-escalation'
|
||||
import {
|
||||
DEV_CHANNEL_PLATFORM_LABEL,
|
||||
RELEASE_CHANNELS,
|
||||
@@ -23,6 +24,8 @@ import {
|
||||
type ReleaseChannel
|
||||
} from '../../../../shared/release-channel'
|
||||
|
||||
const LOAD_BUILDS_CATCH = 'settings.ReleaseChannelSection.loadBuilds'
|
||||
|
||||
const CHANNEL_DESCRIPTIONS: Record<ReleaseChannel, string> = {
|
||||
stable: 'Shipped releases. What everyone else is running.',
|
||||
rc: 'Release candidates cut ahead of each stable.',
|
||||
@@ -135,6 +138,11 @@ export function ReleaseChannelSection(): React.JSX.Element {
|
||||
setLoadError(result.message)
|
||||
}
|
||||
} catch (error) {
|
||||
// Why first: listBuilds can succeed and the setState above it still throw React #185.
|
||||
// Painting that as loadError showed the digest as red inline copy under "No builds found".
|
||||
if (escalateReactUpdateDepthError(error, LOAD_BUILDS_CATCH)) {
|
||||
return
|
||||
}
|
||||
if (isStale()) {
|
||||
return
|
||||
}
|
||||
|
||||
@@ -15,6 +15,7 @@ import {
|
||||
import { useCallback, useEffect, useRef, useState } from 'react'
|
||||
import { toast } from 'sonner'
|
||||
import { useMountedRef } from '@/hooks/useMountedRef'
|
||||
import { escalateReactUpdateDepthError } from '@/lib/react-update-depth-escalation'
|
||||
import type { GlobalSettings } from '../../../../shared/global-settings-types'
|
||||
import {
|
||||
isUserManagedRuntimeEnvironment,
|
||||
@@ -63,6 +64,9 @@ import {
|
||||
} from './RemoteServerUpdateStatus'
|
||||
import { RuntimeHostAccessForm, type RuntimeHostAccessFailure } from './RuntimeHostAccessForm'
|
||||
|
||||
const RUNTIME_LOAD_CATCH = 'settings.RuntimeEnvironmentsPane.loadEnvironments'
|
||||
const RUNTIME_PROBE_CATCH = 'settings.RuntimeEnvironmentsPane.probeEnvironmentStatus'
|
||||
|
||||
const LOCAL_RUNTIME_VALUE = '__local__'
|
||||
const NO_RUNTIME_VALUE = '__none__'
|
||||
|
||||
@@ -381,6 +385,12 @@ export function RuntimeEnvironmentsPane({
|
||||
}
|
||||
}))
|
||||
} catch (error) {
|
||||
// Why first: allSettled would swallow a #185 a second time, and the probe
|
||||
// itself succeeded — status:null here blames the host for a local render
|
||||
// loop and marks a reachable server unverifiable.
|
||||
if (escalateReactUpdateDepthError(error, RUNTIME_PROBE_CATCH)) {
|
||||
return
|
||||
}
|
||||
// Why: record the failed probe (null status) so the sidebar can
|
||||
// distinguish unreachable from never-checked.
|
||||
useAppStore.getState().setRuntimeEnvironmentStatus(environment.id, {
|
||||
@@ -403,6 +413,9 @@ export function RuntimeEnvironmentsPane({
|
||||
})
|
||||
)
|
||||
} catch (error) {
|
||||
if (escalateReactUpdateDepthError(error, RUNTIME_LOAD_CATCH)) {
|
||||
return
|
||||
}
|
||||
if (mountedRef.current) {
|
||||
toast.error(
|
||||
error instanceof Error
|
||||
|
||||
@@ -4,6 +4,11 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { DaemonSession } from './resource-usage-merge-types'
|
||||
import { notifyDaemonSessionInventoryInvalidated } from './daemon-session-inventory-invalidation'
|
||||
import { useResourceSessionInventory } from './use-resource-session-inventory'
|
||||
import { clearReactErrorBoundaryReportingForTest } from '@/lib/react-error-boundary-reporting'
|
||||
import {
|
||||
flushReactUpdateDepthEscalationsForTest,
|
||||
resetReactUpdateDepthEscalationForTest
|
||||
} from '@/lib/react-update-depth-escalation'
|
||||
|
||||
function session(id: string): DaemonSession {
|
||||
return { id, cwd: '/workspace', title: id, agentOwnership: 'absent' as const }
|
||||
@@ -84,6 +89,29 @@ describe('useResourceSessionInventory', () => {
|
||||
expect(result.current.sessionsError).toBe(false)
|
||||
})
|
||||
|
||||
it('escalates React #185 instead of flagging the PTY host as unreachable', async () => {
|
||||
const recordRendererError = vi.fn().mockResolvedValue({ ok: true, report: null, deduped: false })
|
||||
resetReactUpdateDepthEscalationForTest()
|
||||
clearReactErrorBoundaryReportingForTest()
|
||||
;(window as unknown as { api: { crashReports: unknown } }).api.crashReports = {
|
||||
recordRendererError
|
||||
}
|
||||
listSessions.mockRejectedValue(
|
||||
new Error('Minified React error #185; visit https://react.dev/errors/185')
|
||||
)
|
||||
|
||||
const { result } = renderHook(() => useResourceSessionInventory(true))
|
||||
await act(async () => {
|
||||
await result.current.refreshSessions()
|
||||
})
|
||||
await flushReactUpdateDepthEscalationsForTest()
|
||||
|
||||
expect(result.current.sessionsError).toBe(false)
|
||||
expect(recordRendererError.mock.calls[0]?.[0]?.boundaryId).toContain(
|
||||
'use-resource-session-inventory.refreshSessions'
|
||||
)
|
||||
})
|
||||
|
||||
it('refreshes for background spawns without depending on mounted pane state', async () => {
|
||||
listSessions
|
||||
.mockResolvedValueOnce([session('one')])
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { useCallback, useEffect, useRef, useState } from 'react'
|
||||
import { useMountedRef } from '@/hooks/useMountedRef'
|
||||
import { escalateReactUpdateDepthError } from '@/lib/react-update-depth-escalation'
|
||||
import { subscribeDaemonSessionInventoryInvalidated } from './daemon-session-inventory-invalidation'
|
||||
import {
|
||||
EMPTY_DAEMON_SESSION_INVENTORY,
|
||||
@@ -9,6 +10,8 @@ import {
|
||||
type DaemonSessionInventory
|
||||
} from './resource-session-inventory'
|
||||
|
||||
const RESOURCE_SESSION_REFRESH_CATCH = 'status-bar.use-resource-session-inventory.refreshSessions'
|
||||
|
||||
type ResourceSessionInventory = {
|
||||
sessionInventory: DaemonSessionInventory
|
||||
sessionsError: boolean
|
||||
@@ -79,7 +82,12 @@ export function useResourceSessionInventory(ready: boolean): ResourceSessionInve
|
||||
sessionInventory: inventoryFromSessions(liveSessions),
|
||||
sessionsError: false
|
||||
})
|
||||
} catch {
|
||||
} catch (error) {
|
||||
// Why first: listSessions can succeed and the fresh-object setStoredState above still
|
||||
// throw #185; sessionsError would then blame the PTY host for a local render loop.
|
||||
if (escalateReactUpdateDepthError(error, RESOURCE_SESSION_REFRESH_CATCH)) {
|
||||
return
|
||||
}
|
||||
if (mountedRef.current && generation === refreshGenerationRef.current) {
|
||||
setStoredState((current) => ({ ...current, sessionsError: true }))
|
||||
}
|
||||
|
||||
@@ -0,0 +1,125 @@
|
||||
// @vitest-environment happy-dom
|
||||
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import {
|
||||
escalateReactUpdateDepthError,
|
||||
flushReactUpdateDepthEscalationsForTest,
|
||||
resetReactUpdateDepthEscalationForTest
|
||||
} from './react-update-depth-escalation'
|
||||
import { clearReactErrorBoundaryReportingForTest } from './react-error-boundary-reporting'
|
||||
|
||||
const REACT_185_MINIFIED =
|
||||
'Minified React error #185; visit https://react.dev/errors/185 for the full message or use the non-minified dev environment for full errors and additional helpful warnings.'
|
||||
const REACT_185_LEGACY =
|
||||
'Minified React error #185; visit https://reactjs.org/docs/error-decoder.html?invariant=185 for the full message.'
|
||||
const REACT_185_DEV =
|
||||
'Maximum update depth exceeded. This can happen when a component repeatedly calls setState inside componentWillUpdate or componentDidUpdate.'
|
||||
|
||||
const recordRendererError = vi.fn()
|
||||
|
||||
vi.mock('@/store', () => ({
|
||||
useAppStore: {
|
||||
getState: () => ({
|
||||
activeView: 'settings',
|
||||
activeModal: 'settings',
|
||||
activeTabType: null,
|
||||
rightSidebarTab: null,
|
||||
activeWorktreeId: null
|
||||
})
|
||||
}
|
||||
}))
|
||||
|
||||
const SITE = 'settings.ReleaseChannelSection.loadBuilds'
|
||||
|
||||
let captured: ErrorEvent[] = []
|
||||
|
||||
function captureHostError(event: Event): void {
|
||||
captured.push(event as ErrorEvent)
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
captured = []
|
||||
recordRendererError.mockReset()
|
||||
recordRendererError.mockResolvedValue({ ok: true, report: null, deduped: false })
|
||||
resetReactUpdateDepthEscalationForTest()
|
||||
clearReactErrorBoundaryReportingForTest()
|
||||
window.api = {
|
||||
crashReports: { recordRendererError }
|
||||
} as unknown as typeof window.api
|
||||
window.addEventListener('error', captureHostError)
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
window.removeEventListener('error', captureHostError)
|
||||
})
|
||||
|
||||
describe('react update depth escalation', () => {
|
||||
it('leaves ordinary errors to the catching site', () => {
|
||||
expect(escalateReactUpdateDepthError(new TypeError('fetch failed'), SITE)).toBe(false)
|
||||
expect(escalateReactUpdateDepthError('HTTP 500: rate limited', SITE)).toBe(false)
|
||||
expect(escalateReactUpdateDepthError(null, SITE)).toBe(false)
|
||||
expect(recordRendererError).not.toHaveBeenCalled()
|
||||
expect(captured).toHaveLength(0)
|
||||
})
|
||||
|
||||
it.each([
|
||||
['minified production digest', REACT_185_MINIFIED],
|
||||
['legacy invariant digest', REACT_185_LEGACY],
|
||||
['development message', REACT_185_DEV]
|
||||
])('escalates the %s', async (_label, message) => {
|
||||
expect(escalateReactUpdateDepthError(new Error(message), SITE)).toBe(true)
|
||||
await flushReactUpdateDepthEscalationsForTest()
|
||||
expect(recordRendererError).toHaveBeenCalledTimes(1)
|
||||
expect(recordRendererError.mock.calls[0]?.[0]).toMatchObject({
|
||||
errorMessage: message,
|
||||
attribution: 'unreliable'
|
||||
})
|
||||
// The catching site is the only attribution #185 has; any boundary that caught it names a bystander.
|
||||
expect(recordRendererError.mock.calls[0]?.[0]?.boundaryId).toContain(
|
||||
'settings.ReleaseChannelSection.loadBuilds'
|
||||
)
|
||||
expect(captured).toHaveLength(1)
|
||||
expect(captured[0]?.error).toBeInstanceOf(Error)
|
||||
})
|
||||
|
||||
it('does not match neighbouring React error numbers', () => {
|
||||
for (const digest of ['#1850', '#1852', '#18', '#310']) {
|
||||
expect(
|
||||
escalateReactUpdateDepthError(
|
||||
new Error(`Minified React error ${digest}; visit https://react.dev/errors/x`),
|
||||
SITE
|
||||
)
|
||||
).toBe(false)
|
||||
}
|
||||
expect(recordRendererError).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('reports a runaway site once, so the loop cannot flood the crash reporter', async () => {
|
||||
for (let i = 0; i < 200; i += 1) {
|
||||
expect(escalateReactUpdateDepthError(new Error(REACT_185_MINIFIED), SITE)).toBe(true)
|
||||
}
|
||||
await flushReactUpdateDepthEscalationsForTest()
|
||||
expect(recordRendererError).toHaveBeenCalledTimes(1)
|
||||
expect(captured).toHaveLength(1)
|
||||
})
|
||||
|
||||
it('still escalates a second, distinct catching site', async () => {
|
||||
escalateReactUpdateDepthError(new Error(REACT_185_MINIFIED), SITE)
|
||||
escalateReactUpdateDepthError(new Error(REACT_185_MINIFIED), 'startup-hydration.catalog')
|
||||
await flushReactUpdateDepthEscalationsForTest()
|
||||
expect(recordRendererError).toHaveBeenCalledTimes(2)
|
||||
expect(captured).toHaveLength(2)
|
||||
})
|
||||
|
||||
it('never throws when the crash-report bridge is missing or fails', () => {
|
||||
window.api = undefined as unknown as typeof window.api
|
||||
expect(() => escalateReactUpdateDepthError(new Error(REACT_185_MINIFIED), SITE)).not.toThrow()
|
||||
|
||||
resetReactUpdateDepthEscalationForTest()
|
||||
recordRendererError.mockImplementation(() => {
|
||||
throw new Error('IPC bridge torn down')
|
||||
})
|
||||
window.api = { crashReports: { recordRendererError } } as unknown as typeof window.api
|
||||
expect(() => escalateReactUpdateDepthError(new Error(REACT_185_DEV), SITE)).not.toThrow()
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,85 @@
|
||||
/**
|
||||
* A try/catch around an async setState is not an error boundary. When React's nested-update
|
||||
* limit trips (#185), the throw surfaces wherever the next setState happened to run — often
|
||||
* inside an awaited continuation whose catch was written for network failures. Swallowing it
|
||||
* there hides the runaway loop, paints its message as ordinary UI copy, and lets React's
|
||||
* module-global counter reset to 0 so the loop re-arms and throws again on a different fiber.
|
||||
*
|
||||
* React cannot be told about an error raised outside its call stack, so containment here means
|
||||
* re-reporting with the *catching site's* identity — the only attribution #185 has, since any
|
||||
* boundary that catches it names a bystander picked by commit order.
|
||||
*/
|
||||
|
||||
import { isReactUpdateDepthError } from '../../../shared/react-update-depth-attribution'
|
||||
import { reportReactErrorBoundaryCrash } from './react-error-boundary-reporting'
|
||||
|
||||
// Marks the report as a swallowing catch rather than a mounted boundary; boundaryId caps at 120 chars.
|
||||
const ASYNC_CATCH_BOUNDARY_PREFIX = 'async-catch:'
|
||||
|
||||
const escalatedSiteIds = new Set<string>()
|
||||
const pendingEscalations = new Set<Promise<void>>()
|
||||
|
||||
/**
|
||||
* Returns true when `error` is React's nested-update-limit throw, after routing it to the crash
|
||||
* reporter and the host error handler. Callers should then bail out of their catch rather than
|
||||
* apply their normal failure handling: the message is not a user-facing status, and the extra
|
||||
* setState calls would feed the same loop.
|
||||
*
|
||||
* Returns false — and does nothing — for every other error, so expected failures keep their path.
|
||||
*
|
||||
* `siteId` names the catch, e.g. `settings.ReleaseChannelSection.loadBuilds`.
|
||||
*/
|
||||
export function escalateReactUpdateDepthError(error: unknown, siteId: string): boolean {
|
||||
if (!isReactUpdateDepthError(error)) {
|
||||
return false
|
||||
}
|
||||
// Why once per site: the loop that produced this re-enters the same catch thousands of times a second.
|
||||
if (escalatedSiteIds.has(siteId)) {
|
||||
return true
|
||||
}
|
||||
escalatedSiteIds.add(siteId)
|
||||
try {
|
||||
trackEscalation(
|
||||
reportReactErrorBoundaryCrash({
|
||||
boundaryId: `${ASYNC_CATCH_BOUNDARY_PREFIX}${siteId}`,
|
||||
// No boundary caught this, so no boundary surface describes it; siteId carries the identity.
|
||||
surface: 'app-root',
|
||||
error
|
||||
})
|
||||
)
|
||||
dispatchToHostErrorHandler(error, siteId)
|
||||
} catch (escalationError) {
|
||||
// A guard that throws would replace the loop with a second, less informative failure.
|
||||
console.warn('[react-update-depth] Failed to escalate a swallowed #185:', escalationError)
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
function trackEscalation(escalation: Promise<void>): void {
|
||||
pendingEscalations.add(escalation)
|
||||
void escalation.catch(() => undefined).finally(() => pendingEscalations.delete(escalation))
|
||||
}
|
||||
|
||||
/** Re-raises on the host so global renderer diagnostics see it; a synchronous rethrow would be re-swallowed. */
|
||||
function dispatchToHostErrorHandler(error: unknown, siteId: string): void {
|
||||
if (typeof window === 'undefined' || typeof ErrorEvent !== 'function') {
|
||||
return
|
||||
}
|
||||
const message = error instanceof Error ? error.message : String(error)
|
||||
window.dispatchEvent(
|
||||
new ErrorEvent('error', {
|
||||
error,
|
||||
message: `[${ASYNC_CATCH_BOUNDARY_PREFIX}${siteId}] ${message}`
|
||||
})
|
||||
)
|
||||
}
|
||||
|
||||
/** Join point for tests: reporting is deliberately fire-and-forget in the app. */
|
||||
export async function flushReactUpdateDepthEscalationsForTest(): Promise<void> {
|
||||
await Promise.allSettled(pendingEscalations)
|
||||
}
|
||||
|
||||
export function resetReactUpdateDepthEscalationForTest(): void {
|
||||
escalatedSiteIds.clear()
|
||||
pendingEscalations.clear()
|
||||
}
|
||||
@@ -31,10 +31,12 @@ function messageOf(error: unknown): string {
|
||||
return ''
|
||||
}
|
||||
|
||||
export function isReactUpdateDepthError(error: unknown): boolean {
|
||||
return REACT_UPDATE_DEPTH_ERROR.test(messageOf(error))
|
||||
}
|
||||
|
||||
export function getReactErrorBoundaryAttribution(
|
||||
error: unknown
|
||||
): CrashReportAttribution | undefined {
|
||||
return REACT_UPDATE_DEPTH_ERROR.test(messageOf(error))
|
||||
? UNRELIABLE_BOUNDARY_ATTRIBUTION
|
||||
: undefined
|
||||
return isReactUpdateDepthError(error) ? UNRELIABLE_BOUNDARY_ATTRIBUTION : undefined
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user