mirror of
https://github.com/stablyai/orca.git
synced 2026-10-20 08:03:28 +00:00
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 <help@stably.ai>
This commit is contained in:
@@ -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<CrashReportRecord[]> {
|
||||
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<string | null> {
|
||||
const reports = await this.readReports()
|
||||
const report = reports.find((candidate) => candidate.id === id)
|
||||
|
||||
@@ -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<CrashReportStore, 'markRendererCrashesAutoRecovered'>
|
||||
|
||||
/**
|
||||
* 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
|
||||
}
|
||||
@@ -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')
|
||||
}
|
||||
})
|
||||
|
||||
@@ -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<string, (_event: unknown, args?: unknown) => unknown>(),
|
||||
listeners: new Map<string, (_event: unknown, args?: unknown) => 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<CrashReportStore> {
|
||||
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<CrashReportRecord | null> {
|
||||
return (await handlers.get('crashReports:getLatestPending')?.(null)) as CrashReportRecord | null
|
||||
}
|
||||
|
||||
async function getLatestReport(): Promise<CrashReportRecord | null> {
|
||||
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' })
|
||||
})
|
||||
})
|
||||
@@ -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<string>()
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user