From e928bed289b6f85c4a4d73ce63562741e030a28c Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 1 Aug 2026 04:20:44 -0700 Subject: [PATCH] fix(crash-reporting): stop prompting for renderer crashes auto-recovery healed A renderer killed by the OS is recorded as a pending crash report, then the recovery reload brings a new renderer up ~600ms later. That renderer's CrashReportDialog immediately queries getLatestPending and opens the crash prompt for a crash the app already healed (10/10 of the cluster). Key the suppression on observed recovery, not on reason/exit code: arm an outcome check when the recovery reload is issued, and resolve the pending renderer report only once the new renderer emits renderer_bootstrap_rendered. A reload that never brings React back leaves the report pending, so the blank-window case still prompts. The record stays on disk, keeps its breadcrumbs, stays sendable from Help > Report Crash, and is stamped rendererAutoRecovered. Co-authored-by: Orca --- .../crash-reporting/crash-report-store.ts | 29 +++ .../renderer-recovery-crash-outcome.ts | 59 ++++++ src/main/index.ts | 3 + ...eporting-recovered-renderer-prompt.test.ts | 190 ++++++++++++++++++ src/main/ipc/crash-reporting.ts | 9 + 5 files changed, 290 insertions(+) create mode 100644 src/main/crash-reporting/renderer-recovery-crash-outcome.ts create mode 100644 src/main/ipc/crash-reporting-recovered-renderer-prompt.test.ts diff --git a/src/main/crash-reporting/crash-report-store.ts b/src/main/crash-reporting/crash-report-store.ts index 35f9eb5cb48..99280007b79 100644 --- a/src/main/crash-reporting/crash-report-store.ts +++ b/src/main/crash-reporting/crash-report-store.ts @@ -126,6 +126,35 @@ export class CrashReportStore { return this.transitionPending(id, 'dismissed') } + /** + * Resolve pending renderer crashes that an auto-recovery reload healed. + * + * Why: `dismissed` keeps the record sendable from Help > Report Crash while + * taking it out of the startup prompt; the detail flag preserves the reason. + */ + async markRendererCrashesAutoRecovered(notBeforeMs: number): Promise { + return this.withWrite(async (reports) => { + const resolved: CrashReportRecord[] = [] + const nextReports = reports.map((report) => { + if (report.status !== 'pending' || report.source !== 'renderer') { + return report + } + const createdAtMs = Date.parse(report.createdAt) + if (!Number.isFinite(createdAtMs) || createdAtMs < notBeforeMs) { + return report + } + const recovered: CrashReportRecord = { + ...report, + status: 'dismissed', + details: { ...report.details, rendererAutoRecovered: true } + } + resolved.push(recovered) + return recovered + }) + return { reports: nextReports, result: resolved } + }) + } + async formatDiagnosticText(id: string, notes?: string): Promise { const reports = await this.readReports() const report = reports.find((candidate) => candidate.id === id) diff --git a/src/main/crash-reporting/renderer-recovery-crash-outcome.ts b/src/main/crash-reporting/renderer-recovery-crash-outcome.ts new file mode 100644 index 00000000000..20d47b3d330 --- /dev/null +++ b/src/main/crash-reporting/renderer-recovery-crash-outcome.ts @@ -0,0 +1,59 @@ +import type { CrashReportStore } from './crash-report-store' +import { recordDurableCrashBreadcrumb } from './durable-crash-breadcrumb' + +export const RENDERER_BOOTSTRAP_RENDERED_BREADCRUMB = 'renderer_bootstrap_rendered' + +// Why: field traces show crash -> reload -> React bootstrap under 1.5s; this only +// has to survive a loaded machine, and must expire before an unrelated boot. +const RENDERER_RECOVERY_OUTCOME_WINDOW_MS = 30_000 + +let recoveryReloadIssuedAtMs: number | null = null + +export function noteRendererRecoveryReloadIssued(nowMs = Date.now()): void { + recoveryReloadIssuedAtMs = nowMs +} + +function takeRendererRecoveryReloadIssuedAt(nowMs: number): number | null { + const issuedAtMs = recoveryReloadIssuedAtMs + recoveryReloadIssuedAtMs = null + if (issuedAtMs === null || nowMs - issuedAtMs > RENDERER_RECOVERY_OUTCOME_WINDOW_MS) { + return null + } + return issuedAtMs +} + +type RecoveredCrashReportStore = Pick + +/** + * Resolve renderer crash reports that an auto-recovery reload actually healed. + * + * Why: this must stay synchronous up to the store call so the write is queued + * before the recovered renderer's getLatestPending read drains the same chain. + */ +export function resolveRecoveredRendererCrashReports( + store: RecoveredCrashReportStore, + nowMs = Date.now() +): void { + const issuedAtMs = takeRendererRecoveryReloadIssuedAt(nowMs) + if (issuedAtMs === null) { + return + } + void store + .markRendererCrashesAutoRecovered(issuedAtMs - RENDERER_RECOVERY_OUTCOME_WINDOW_MS) + .then((resolved) => { + if (resolved.length === 0) { + return + } + recordDurableCrashBreadcrumb('renderer_crash_auto_recovered', { + resolvedReportCount: resolved.length, + recoveryLatencyMs: nowMs - issuedAtMs + }) + }) + .catch((error) => { + console.error('[crash-reporting] Failed to resolve auto-recovered crash reports:', error) + }) +} + +export function _resetRendererRecoveryOutcomeForTests(): void { + recoveryReloadIssuedAtMs = null +} diff --git a/src/main/index.ts b/src/main/index.ts index a80bea0d965..857211b0111 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -281,6 +281,7 @@ import { type ExpectedTeardownScope } from './crash-reporting/process-gone-classification' import { recordProcessGoneCrash as recordProcessGoneCrashEvent } from './crash-reporting/process-gone-recorder' +import { noteRendererRecoveryReloadIssued } from './crash-reporting/renderer-recovery-crash-outcome' import { advanceSyntheticTitleSpinnerEntries, type SyntheticTitleSpinnerEntry @@ -1246,6 +1247,8 @@ function openMainWindow(): BrowserWindow { // Why: the recovery reload re-fires did-finish-load; flag it so the local-PTY orphan sweep skips that reload (#5787). onBeforeRecoveryReload: (webContentsId) => { markRecoveryReloadInFlight(webContentsId) + // Why: arm the outcome check before the reload so a crash this heals never prompts the user. + noteRendererRecoveryReloadIssued() recordDurableCrashBreadcrumb('renderer_recovery_reload') } }) diff --git a/src/main/ipc/crash-reporting-recovered-renderer-prompt.test.ts b/src/main/ipc/crash-reporting-recovered-renderer-prompt.test.ts new file mode 100644 index 00000000000..7d60d1d6fc7 --- /dev/null +++ b/src/main/ipc/crash-reporting-recovered-renderer-prompt.test.ts @@ -0,0 +1,190 @@ +import fs from 'node:fs/promises' +import os from 'node:os' +import path from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const { handlers, listeners } = vi.hoisted(() => ({ + handlers: new Map unknown>(), + listeners: new Map void>() +})) + +vi.mock('electron', () => ({ + app: { getVersion: () => '1.4.162' }, + clipboard: { writeText: vi.fn() }, + ipcMain: { + removeHandler: vi.fn((channel: string) => handlers.delete(channel)), + handle: vi.fn((channel: string, handler: (_event: unknown, args?: unknown) => unknown) => { + handlers.set(channel, handler) + }), + removeAllListeners: vi.fn((channel: string) => listeners.delete(channel)), + on: vi.fn((channel: string, listener: (_event: unknown, args?: unknown) => void) => { + listeners.set(channel, listener) + }) + } +})) + +vi.mock('./feedback', () => ({ submitFeedback: vi.fn() })) + +vi.mock('../crash-reporting/crash-breadcrumb-store', () => ({ + getCrashBreadcrumbSnapshot: vi.fn(() => []), + recordCoalescedCrashBreadcrumb: vi.fn(() => ({ suppressedSinceLast: 0 })), + recordCrashBreadcrumb: vi.fn() +})) + +vi.mock('../observability', () => ({ + collectDiagnosticBundle: vi.fn(), + getDiagnosticsStatus: vi.fn() +})) + +vi.mock('../observability/diagnostic-upload-endpoint', () => ({ + resolveDiagnosticOrcaChannel: vi.fn() +})) + +vi.mock('../observability/tracer', () => ({ + startSpan: vi.fn(() => ({ + traceId: 'trace-id', + spanId: 'span-id', + setAttribute: vi.fn(), + addEvent: vi.fn(), + fail: vi.fn(), + interrupt: vi.fn(), + end: vi.fn() + })), + flushActiveSink: vi.fn() +})) + +import { CrashReportStore } from '../crash-reporting/crash-report-store' +import { + _resetRendererRecoveryOutcomeForTests, + noteRendererRecoveryReloadIssued, + resolveRecoveredRendererCrashReports +} from '../crash-reporting/renderer-recovery-crash-outcome' +import type { CrashReportCreateInput, CrashReportRecord } from '../../shared/crash-reporting' +import { + _resetRendererErrorReportDedupeForTests, + registerCrashReportingHandlers +} from './crash-reporting' + +const tempDirs: string[] = [] + +async function createStore(): Promise { + const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'orca-recovered-renderer-crash-')) + tempDirs.push(dir) + return new CrashReportStore(path.join(dir, 'crash-reports.json')) +} + +// The cluster-F shape: macOS/Linux SIGKILL, Windows uses different codes on the same path. +function killedRendererCrash(exitCode = 9): CrashReportCreateInput { + return { + source: 'renderer', + processType: 'renderer', + reason: 'killed', + exitCode, + appVersion: '1.4.162', + platform: process.platform, + osRelease: 'test', + arch: process.arch, + electronVersion: '41', + chromeVersion: '141', + details: { processType: 'renderer' } + } +} + +function emitRendererBootstrapRendered(): void { + listeners.get('crashReports:recordBreadcrumb')?.(null, { name: 'renderer_bootstrap_rendered' }) +} + +async function getLatestPending(): Promise { + return (await handlers.get('crashReports:getLatestPending')?.(null)) as CrashReportRecord | null +} + +async function getLatestReport(): Promise { + return (await handlers.get('crashReports:getLatestReport')?.(null)) as CrashReportRecord | null +} + +beforeEach(() => { + handlers.clear() + listeners.clear() + _resetRendererRecoveryOutcomeForTests() + _resetRendererErrorReportDedupeForTests() +}) + +afterEach(async () => { + await Promise.all(tempDirs.splice(0).map((dir) => fs.rm(dir, { recursive: true, force: true }))) +}) + +describe('auto-recovered renderer crash reporting', () => { + it('does not prompt the recovered renderer for a crash the reload healed', async () => { + const store = await createStore() + registerCrashReportingHandlers(store) + await store.record(killedRendererCrash()) + + noteRendererRecoveryReloadIssued() + emitRendererBootstrapRendered() + + await expect(getLatestPending()).resolves.toBeNull() + }) + + it.each([ + ['macOS/Linux SIGKILL', 9], + ['Windows ACCESS_VIOLATION', -1073741819], + ['Windows renderer OOM', -536870904] + ])('suppresses the prompt for %s once recovery is observed', async (_label, exitCode) => { + const store = await createStore() + registerCrashReportingHandlers(store) + await store.record(killedRendererCrash(exitCode)) + + noteRendererRecoveryReloadIssued() + emitRendererBootstrapRendered() + + await expect(getLatestPending()).resolves.toBeNull() + }) + + it('keeps the healed report sendable from Help > Report Crash with its recovery flag', async () => { + const store = await createStore() + registerCrashReportingHandlers(store) + const recorded = await store.record(killedRendererCrash()) + + noteRendererRecoveryReloadIssued() + emitRendererBootstrapRendered() + await getLatestPending() + + const sendable = await getLatestReport() + expect(sendable?.id).toBe(recorded.id) + expect(sendable?.status).toBe('dismissed') + expect(sendable?.details.rendererAutoRecovered).toBe(true) + expect(sendable?.breadcrumbs).toEqual(recorded.breadcrumbs) + }) + + it('still prompts when the renderer booted without an auto-recovery reload', async () => { + const store = await createStore() + registerCrashReportingHandlers(store) + const recorded = await store.record(killedRendererCrash()) + + emitRendererBootstrapRendered() + + await expect(getLatestPending()).resolves.toMatchObject({ id: recorded.id, status: 'pending' }) + }) + + it('still prompts when the recovery reload never brought the renderer back', async () => { + const store = await createStore() + registerCrashReportingHandlers(store) + const recorded = await store.record(killedRendererCrash()) + + noteRendererRecoveryReloadIssued() + + await expect(getLatestPending()).resolves.toMatchObject({ id: recorded.id, status: 'pending' }) + }) + + it('leaves an unresolved crash from a previous session pending', async () => { + const store = await createStore() + registerCrashReportingHandlers(store) + const recorded = await store.record(killedRendererCrash()) + + const oneHourLater = Date.parse(recorded.createdAt) + 3_600_000 + noteRendererRecoveryReloadIssued(oneHourLater) + resolveRecoveredRendererCrashReports(store, oneHourLater) + + await expect(getLatestPending()).resolves.toMatchObject({ id: recorded.id, status: 'pending' }) + }) +}) diff --git a/src/main/ipc/crash-reporting.ts b/src/main/ipc/crash-reporting.ts index 4cdd4bdbbb4..a530615891a 100644 --- a/src/main/ipc/crash-reporting.ts +++ b/src/main/ipc/crash-reporting.ts @@ -33,6 +33,10 @@ import { isClipboardTextWriteTooLargeError } from '../../shared/clipboard-text' import { formatCrashReportCopyText } from '../crash-reporting/crash-report-copy-text' +import { + RENDERER_BOOTSTRAP_RENDERED_BREADCRUMB, + resolveRecoveredRendererCrashReports +} from '../crash-reporting/renderer-recovery-crash-outcome' import { TERMINAL_WEBGL_DIAGNOSTIC_BREADCRUMB } from '../../shared/terminal-webgl-diagnostics' const inFlightSubmissions = new Set() @@ -420,6 +424,11 @@ export function registerCrashReportingHandlers(store: CrashReportStore): void { if (!args || typeof args.name !== 'string') { return } + if (args.name === RENDERER_BOOTSTRAP_RENDERED_BREADCRUMB) { + // Why: the recovered renderer asks for the startup prompt right after + // this breadcrumb, so the healed crash must be resolved first. + resolveRecoveredRendererCrashReports(store) + } const data = sanitizeRendererBreadcrumbData(args.data) if (COALESCED_RENDERER_BREADCRUMB_NAMES.has(args.name)) { const coalesceKey = rendererBreadcrumbCoalesceKey(args.name, data)