mirror of
https://github.com/stablyai/orca.git
synced 2026-10-20 16:03:20 +00:00
refactor(renderer): cut the breadcrumb machinery down to the actual fix
The lazy_chunk_recovery_exhausted breadcrumb was an optional addition that paid for itself in complexity and nothing else: it needed a dedupe set to avoid flushing the 30-entry ring, the set needed a bound because error.name is library-controlled, the bound needed an oldest-first eviction policy, and that needed two more tests plus a boolean parameter that review flagged as a transposition trap. On the dominant never-landed path it did not even fire, because lazy_chunk_reload_vetoed already records the same reloadKey, message and outcome. Drop it. Observability on every path returns to the main baseline, and the fix is what it always was: name an exhausted recovery so the boundary can contain it. Also revert the unrelated regex hoist -- its only caller is the failure path, so the saved allocations are noise.
This commit is contained in:
@@ -10,7 +10,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import { ORCA_RENDERER_UNLOAD_PREVENTED_EVENT } from '../../../shared/renderer-shutdown-events'
|
||||
import {
|
||||
getRecordedExhaustionKeyCountForTest,
|
||||
isLazyChunkLoadError,
|
||||
loadLazyWithRetry,
|
||||
resetLazyChunkReloadRequestsForTest
|
||||
@@ -73,17 +72,13 @@ describe('loadLazyWithRetry when the recovery reload never lands', () => {
|
||||
|
||||
const vetoed = breadcrumbs.find((crumb) => crumb.name === 'lazy_chunk_reload_vetoed')
|
||||
expect(vetoed?.data.outcome).toBe('never-landed')
|
||||
// That crumb already carries the outcome, so the exhaustion crumb is not duplicated.
|
||||
expect(breadcrumbs.filter((crumb) => crumb.name === 'lazy_chunk_recovery_exhausted')).toEqual(
|
||||
[]
|
||||
)
|
||||
|
||||
// The boundary only suppresses LazyChunkLoadError; a raw SyntaxError files a crash report.
|
||||
expect(isLazyChunkLoadError((result as { error: unknown }).error)).toBe(true)
|
||||
})
|
||||
|
||||
it('does not strand a sibling lazy import that fails while a reload is pending', async () => {
|
||||
const breadcrumbs = installBreadcrumbSink()
|
||||
installBreadcrumbSink()
|
||||
const first = loadLazyWithRetry(() => Promise.reject(CORRUPT_CHUNK_ERROR()), {
|
||||
retries: 0,
|
||||
reloadKey: 'app.root'
|
||||
@@ -103,36 +98,6 @@ describe('loadLazyWithRetry when the recovery reload never lands', () => {
|
||||
|
||||
expect(isLazyChunkLoadError(await first)).toBe(true)
|
||||
expect(isLazyChunkLoadError(await sibling)).toBe(true)
|
||||
|
||||
// The sibling has no vetoed crumb of its own, so it records the exhaustion.
|
||||
const exhausted = breadcrumbs.find((crumb) => crumb.name === 'lazy_chunk_recovery_exhausted')
|
||||
expect(exhausted?.data).toEqual({
|
||||
reloadKey: 'app.root.sibling',
|
||||
message: "Unexpected token '}'",
|
||||
outcome: 'reload-in-flight'
|
||||
})
|
||||
})
|
||||
|
||||
it('records one exhaustion breadcrumb per call site, not per sibling failure', async () => {
|
||||
const breadcrumbs = installBreadcrumbSink()
|
||||
void loadLazyWithRetry(() => Promise.reject(CORRUPT_CHUNK_ERROR()), {
|
||||
retries: 0,
|
||||
reloadKey: 'app.root'
|
||||
}).catch(() => undefined)
|
||||
await vi.advanceTimersByTimeAsync(0)
|
||||
|
||||
// A corrupt shared chunk fails many siblings at once under the pending reload.
|
||||
for (let index = 0; index < 5; index += 1) {
|
||||
void loadLazyWithRetry(() => Promise.reject(CORRUPT_CHUNK_ERROR()), {
|
||||
retries: 0,
|
||||
reloadKey: 'sidebar'
|
||||
}).catch(() => undefined)
|
||||
}
|
||||
await vi.advanceTimersByTimeAsync(RELOAD_SETTLE_GRACE_MS + 1)
|
||||
|
||||
expect(
|
||||
breadcrumbs.filter((crumb) => crumb.name === 'lazy_chunk_recovery_exhausted')
|
||||
).toHaveLength(1)
|
||||
})
|
||||
|
||||
it('contains an unload-vetoed reload and records it as a distinct outcome', async () => {
|
||||
@@ -152,39 +117,6 @@ describe('loadLazyWithRetry when the recovery reload never lands', () => {
|
||||
expect(isLazyChunkLoadError(await settled)).toBe(true)
|
||||
})
|
||||
|
||||
it('bounds the exhaustion dedupe set so a hostile error name cannot grow it forever', async () => {
|
||||
const breadcrumbs = installBreadcrumbSink()
|
||||
// error.name is library-controlled, so drive far more distinct keys than the cap.
|
||||
window.sessionStorage.setItem(RELOAD_GUARD_KEY, 'doc-before-the-reload')
|
||||
const failWithName = (name: string): void => {
|
||||
const error = new SyntaxError("Unexpected token '}'")
|
||||
error.name = name
|
||||
void loadLazyWithRetry(() => Promise.reject(error), {
|
||||
retries: 0,
|
||||
reloadKey: 'right-sidebar'
|
||||
}).catch(() => undefined)
|
||||
}
|
||||
for (let index = 0; index < 200; index += 1) {
|
||||
failWithName(`SyntaxError${index}`)
|
||||
}
|
||||
await vi.advanceTimersByTimeAsync(0)
|
||||
|
||||
expect(getRecordedExhaustionKeyCountForTest()).toBeLessThanOrEqual(128)
|
||||
|
||||
// Eviction must be oldest-first, not wholesale: key 100 is among the newest
|
||||
// 128 so it is still deduped. Clearing the set on overflow would drop it and
|
||||
// let a repeat burst re-flush the 30-entry breadcrumb ring.
|
||||
const before = breadcrumbs.filter(
|
||||
(crumb) => crumb.name === 'lazy_chunk_recovery_exhausted'
|
||||
).length
|
||||
failWithName('SyntaxError100')
|
||||
await vi.advanceTimersByTimeAsync(0)
|
||||
|
||||
expect(
|
||||
breadcrumbs.filter((crumb) => crumb.name === 'lazy_chunk_recovery_exhausted')
|
||||
).toHaveLength(before)
|
||||
})
|
||||
|
||||
it('leaves no guard behind that would block a later document from recovering', async () => {
|
||||
installBreadcrumbSink()
|
||||
const settled = loadLazyWithRetry(() => Promise.reject(CORRUPT_CHUNK_ERROR()), {
|
||||
|
||||
@@ -100,26 +100,13 @@ function clearChunkReloadGuard(): void {
|
||||
const MAX_RELOAD_REQUESTS_PER_DOCUMENT = 2
|
||||
let reloadRequestsThisDocument = 0
|
||||
let reloadRequestInFlight = false
|
||||
const MAX_RECORDED_EXHAUSTION_KEYS = 128
|
||||
// One breadcrumb per reloadKey + outcome + error name (unkeyed call sites share
|
||||
// 'unknown'): a corrupt shared chunk can fail dozens of sibling imports at once,
|
||||
// which would otherwise flush the 30-entry breadcrumb ring.
|
||||
const recordedExhaustionKeys = new Set<string>()
|
||||
|
||||
export function getRecordedExhaustionKeyCountForTest(): number {
|
||||
return recordedExhaustionKeys.size
|
||||
}
|
||||
|
||||
export function resetLazyChunkReloadRequestsForTest(): void {
|
||||
reloadRequestsThisDocument = 0
|
||||
reloadRequestInFlight = false
|
||||
recordedExhaustionKeys.clear()
|
||||
}
|
||||
|
||||
type ReloadBreadcrumbName =
|
||||
| 'lazy_chunk_reload'
|
||||
| 'lazy_chunk_reload_vetoed'
|
||||
| 'lazy_chunk_recovery_exhausted'
|
||||
type ReloadBreadcrumbName = 'lazy_chunk_reload' | 'lazy_chunk_reload_vetoed'
|
||||
|
||||
function recordReloadBreadcrumb(
|
||||
name: ReloadBreadcrumbName,
|
||||
@@ -143,54 +130,13 @@ function recordReloadBreadcrumb(
|
||||
|
||||
const wait = (ms: number): Promise<void> => new Promise((resolve) => setTimeout(resolve, ms))
|
||||
|
||||
/**
|
||||
* Names a corrupt-chunk failure that recovery can no longer act on, so the error
|
||||
* boundary contains it instead of filing a react-error-boundary crash. Genuine
|
||||
* logic bugs are not dynamic-import failures and stay raw so they keep reporting.
|
||||
*/
|
||||
function exhaustedRecoveryFailure(
|
||||
lastError: unknown,
|
||||
reloadKey: string,
|
||||
failureMessage: string,
|
||||
outcome: string,
|
||||
// Set when the caller already recorded a vetoed breadcrumb for this outcome;
|
||||
// the 30-entry ring is the only diagnostic left, so it must not self-evict.
|
||||
alreadyRecorded = false
|
||||
): unknown {
|
||||
if (!isKnownDynamicImportFailure(lastError)) {
|
||||
return lastError
|
||||
}
|
||||
const failureName = lastError instanceof Error ? lastError.name : 'unknown'
|
||||
const crumbKey = `${reloadKey}:${outcome}:${failureName}`
|
||||
if (!alreadyRecorded && !recordedExhaustionKeys.has(crumbKey)) {
|
||||
// error.name is library-controlled, so bound the set the way the breadcrumb
|
||||
// and renderer-error key stores do: evict oldest-first, never wholesale, so
|
||||
// an overflow cannot re-open the whole set to a repeat burst.
|
||||
while (recordedExhaustionKeys.size >= MAX_RECORDED_EXHAUSTION_KEYS) {
|
||||
const oldestKey = recordedExhaustionKeys.values().next().value
|
||||
if (oldestKey === undefined) {
|
||||
break
|
||||
}
|
||||
recordedExhaustionKeys.delete(oldestKey)
|
||||
}
|
||||
recordedExhaustionKeys.add(crumbKey)
|
||||
recordReloadBreadcrumb('lazy_chunk_recovery_exhausted', reloadKey, failureMessage, outcome)
|
||||
}
|
||||
return new LazyChunkLoadError(lastError, reloadKey)
|
||||
/** Recovery is spent, so name the failure in the one way the boundary can contain. */
|
||||
function containedChunkFailure(lastError: unknown, reloadKey: string): unknown {
|
||||
return isKnownDynamicImportFailure(lastError)
|
||||
? new LazyChunkLoadError(lastError, reloadKey)
|
||||
: lastError
|
||||
}
|
||||
|
||||
// Module scope: re-evaluating these literals would allocate fresh RegExp objects
|
||||
// on every classification.
|
||||
const DYNAMIC_IMPORT_FAILURE_PATTERNS = [
|
||||
/failed to fetch dynamically imported module/i,
|
||||
/error loading dynamically imported module/i,
|
||||
/importing a module script failed/i,
|
||||
/failed to load module script/i,
|
||||
/loading chunk .+ failed/i,
|
||||
/unexpected token/i,
|
||||
/unexpected end of (input|script|json)/i
|
||||
]
|
||||
|
||||
function isKnownDynamicImportFailure(error: unknown): boolean {
|
||||
if (!(error instanceof Error)) {
|
||||
return false
|
||||
@@ -213,7 +159,15 @@ function isKnownDynamicImportFailure(error: unknown): boolean {
|
||||
return true
|
||||
}
|
||||
|
||||
return DYNAMIC_IMPORT_FAILURE_PATTERNS.some((pattern) => pattern.test(error.message))
|
||||
return [
|
||||
/failed to fetch dynamically imported module/i,
|
||||
/error loading dynamically imported module/i,
|
||||
/importing a module script failed/i,
|
||||
/failed to load module script/i,
|
||||
/loading chunk .+ failed/i,
|
||||
/unexpected token/i,
|
||||
/unexpected end of (input|script|json)/i
|
||||
].some((pattern) => pattern.test(error.message))
|
||||
}
|
||||
|
||||
export async function loadLazyWithRetry<T extends AnyComponent>(
|
||||
@@ -263,36 +217,32 @@ export async function loadLazyWithRetry<T extends AnyComponent>(
|
||||
}
|
||||
// The reload was this document's last recovery step for this chunk, whether it
|
||||
// was refused outright or simply never navigated.
|
||||
throw exhaustedRecoveryFailure(lastError, reloadKey, failureMessage, outcome, true)
|
||||
throw containedChunkFailure(lastError, reloadKey)
|
||||
}
|
||||
|
||||
if (reloadGuardState === 'reload-landed') {
|
||||
throw exhaustedRecoveryFailure(lastError, reloadKey, failureMessage, 'reload-landed')
|
||||
throw containedChunkFailure(lastError, reloadKey)
|
||||
}
|
||||
|
||||
if (reloadGuardState === 'reload-not-landed') {
|
||||
if (reloadRequestInFlight) {
|
||||
// A sibling chunk failing under a pending reload cannot request its own.
|
||||
throw exhaustedRecoveryFailure(lastError, reloadKey, failureMessage, 'reload-in-flight')
|
||||
// A sibling failing under a pending reload must not clear the guard, and an
|
||||
// unrelated failure must not clear a guard it did not set.
|
||||
if (!reloadRequestInFlight && isKnownDynamicImportFailure(lastError)) {
|
||||
// Record the veto beside the resulting report before the ring can evict it.
|
||||
clearChunkReloadGuard()
|
||||
recordReloadBreadcrumb(
|
||||
'lazy_chunk_reload_vetoed',
|
||||
reloadKey,
|
||||
failureMessage,
|
||||
'guard-not-landed'
|
||||
)
|
||||
}
|
||||
if (!isKnownDynamicImportFailure(lastError)) {
|
||||
// An unrelated failure must not clear a guard it did not set.
|
||||
throw lastError
|
||||
}
|
||||
// Record the veto beside the resulting report before the ring can evict it.
|
||||
clearChunkReloadGuard()
|
||||
recordReloadBreadcrumb(
|
||||
'lazy_chunk_reload_vetoed',
|
||||
reloadKey,
|
||||
failureMessage,
|
||||
'guard-not-landed'
|
||||
)
|
||||
throw exhaustedRecoveryFailure(lastError, reloadKey, failureMessage, 'guard-not-landed', true)
|
||||
throw containedChunkFailure(lastError, reloadKey)
|
||||
}
|
||||
|
||||
if (reloadGuardState === 'not-attempted') {
|
||||
// The per-document reload cap is spent; further failures cannot recover.
|
||||
throw exhaustedRecoveryFailure(lastError, reloadKey, failureMessage, 'reload-cap-reached')
|
||||
throw containedChunkFailure(lastError, reloadKey)
|
||||
}
|
||||
|
||||
// No window or no usable storage: recovery was never attempted, so preserve
|
||||
|
||||
Reference in New Issue
Block a user