fix(crash-reporting): coalesce repeated renderer error breadcrumbs (#8800)

* fix(crash-reporting): coalesce repeated renderer error breadcrumbs

The crash-breadcrumb ring holds only 30 entries, and renderer breadcrumbs
arrived via the plain uncoalesced path. A repeating renderer error — like the
ResizeObserver and SSH-rejection storms in #8260 (50+ identical events) —
flushes the entire ring in seconds, erasing the pre-crash trail exactly when a
crash report is about to snapshot it.

Route renderer_error and renderer_unhandled_rejection breadcrumbs through the
existing recordCoalescedCrashBreadcrumb, keyed by name plus message prefix
with a 30s window. Repeats collapse into one entry carrying
suppressedSinceLast, so a storm shows up as "error X fired N times" alongside
29 slots of surrounding context instead of 30 identical copies.

Other breadcrumb names keep the plain path; telemetry trace spans are
unchanged. Renderer breadcrumb routing tests move to a focused
crash-reporting-renderer-breadcrumbs.test.ts (the main suite is at the
max-lines ceiling).

* fix(crash-reporting): coalesce durable renderer traces

* fix: harden renderer crash guards

* fix(crash-reporting): preserve distinct error sources
This commit is contained in:
Brennan Benson
2026-07-15 14:30:33 -07:00
committed by GitHub
parent 0690be5656
commit 8a7cd84bda
9 changed files with 405 additions and 97 deletions
@@ -55,27 +55,30 @@ describe('crash breadcrumb store', () => {
vi.useFakeTimers()
vi.setSystemTime(new Date('2026-05-20T12:00:00.000Z'))
recordCoalescedCrashBreadcrumb({
const first = recordCoalescedCrashBreadcrumb({
name: 'agent_state_changed',
data: { agentType: 'claude', state: 'working' },
coalesceKey: 'agent:claude:working',
minIntervalMs: 30_000
})
vi.advanceTimersByTime(1_000)
recordCoalescedCrashBreadcrumb({
const suppressed = recordCoalescedCrashBreadcrumb({
name: 'agent_state_changed',
data: { agentType: 'claude', state: 'working' },
coalesceKey: 'agent:claude:working',
minIntervalMs: 30_000
})
vi.advanceTimersByTime(30_000)
recordCoalescedCrashBreadcrumb({
const resumed = recordCoalescedCrashBreadcrumb({
name: 'agent_state_changed',
data: { agentType: 'claude', state: 'working' },
coalesceKey: 'agent:claude:working',
minIntervalMs: 30_000
})
expect(first).toEqual({ suppressedSinceLast: 0 })
expect(suppressed).toBeUndefined()
expect(resumed).toEqual({ suppressedSinceLast: 1 })
expect(getCrashBreadcrumbSnapshot().map((entry) => entry.data)).toEqual([
{ agentType: 'claude', state: 'working' },
{ agentType: 'claude', state: 'working', suppressedSinceLast: 1 }
@@ -41,12 +41,12 @@ export function recordCoalescedCrashBreadcrumb({
data?: CrashReportBreadcrumbData
coalesceKey: string
minIntervalMs: number
}): void {
}): { suppressedSinceLast: number } | undefined {
const now = Date.now()
const previous = coalescedBreadcrumbs.get(coalesceKey)
if (previous && now - previous.recordedAt < minIntervalMs) {
previous.suppressed += 1
return
return undefined
}
// Drop entries past their suppression window (they can no longer coalesce
@@ -66,10 +66,9 @@ export function recordCoalescedCrashBreadcrumb({
}
coalescedBreadcrumbs.delete(oldest.value)
}
recordCrashBreadcrumb(
name,
previous?.suppressed ? { ...data, suppressedSinceLast: previous.suppressed } : data
)
const suppressedSinceLast = previous?.suppressed ?? 0
recordCrashBreadcrumb(name, suppressedSinceLast > 0 ? { ...data, suppressedSinceLast } : data)
return { suppressedSinceLast }
}
export function getCrashBreadcrumbSnapshot(): CrashReportBreadcrumb[] {
@@ -0,0 +1,255 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
const {
listeners,
recordCoalescedCrashBreadcrumbMock,
recordCrashBreadcrumbMock,
spanEndMock,
startSpanMock
} = vi.hoisted(() => {
const spanEndMock = vi.fn()
return {
listeners: new Map<string, (_event: unknown, args?: unknown) => void>(),
recordCoalescedCrashBreadcrumbMock: vi.fn(),
recordCrashBreadcrumbMock: vi.fn(),
spanEndMock,
startSpanMock: vi.fn(() => ({
traceId: 'trace-id',
spanId: 'span-id',
setAttribute: vi.fn(),
addEvent: vi.fn(),
fail: vi.fn(),
interrupt: vi.fn(),
end: spanEndMock
}))
}
})
vi.mock('electron', () => ({
app: { getVersion: () => '1.2.3-test' },
clipboard: { writeText: vi.fn() },
ipcMain: {
removeHandler: vi.fn(),
handle: vi.fn(),
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: (...args: unknown[]) =>
recordCoalescedCrashBreadcrumbMock(...args),
recordCrashBreadcrumb: (...args: unknown[]) => recordCrashBreadcrumbMock(...args)
}))
vi.mock('../observability', () => ({
collectDiagnosticBundle: vi.fn(),
getDiagnosticsStatus: vi.fn()
}))
vi.mock('../observability/diagnostic-upload-endpoint', () => ({
resolveDiagnosticOrcaChannel: vi.fn()
}))
vi.mock('../observability/tracer', () => ({
startSpan: startSpanMock
}))
import { registerCrashReportingHandlers } from './crash-reporting'
function registerHandlersWithStubStore(): void {
registerCrashReportingHandlers({
getLatestPending: vi.fn(),
getById: vi.fn(),
dismiss: vi.fn(),
markSent: vi.fn(),
listRecent: vi.fn(),
record: vi.fn(),
formatDiagnosticText: vi.fn()
} as never)
}
function emitRendererBreadcrumb(args: unknown): void {
listeners.get('crashReports:recordBreadcrumb')?.(null, args)
}
describe('renderer breadcrumb IPC routing', () => {
beforeEach(() => {
listeners.clear()
recordCoalescedCrashBreadcrumbMock.mockReset()
recordCoalescedCrashBreadcrumbMock.mockReturnValue({ suppressedSinceLast: 0 })
recordCrashBreadcrumbMock.mockReset()
startSpanMock.mockClear()
spanEndMock.mockClear()
registerHandlersWithStubStore()
})
it('sanitizes and coalesces renderer error breadcrumbs', () => {
emitRendererBreadcrumb({
name: 'renderer_error',
data: {
message: 'boom',
count: 2,
ok: true,
empty: null,
badNumber: Number.POSITIVE_INFINITY,
object: { ignored: true }
}
})
expect(recordCoalescedCrashBreadcrumbMock).toHaveBeenCalledWith({
name: 'renderer_error',
data: { message: 'boom', count: 2, ok: true, empty: null },
coalesceKey: expect.stringContaining('boom'),
minIntervalMs: 30_000
})
expect(recordCrashBreadcrumbMock).not.toHaveBeenCalled()
expect(startSpanMock).toHaveBeenCalledWith('renderer.breadcrumb', {
attributes: {
kind: 'crash-breadcrumb',
'breadcrumb.name': 'renderer_error',
'breadcrumb.data': { message: 'boom', count: 2, ok: true, empty: null }
}
})
expect(spanEndMock).toHaveBeenCalledTimes(1)
})
it('coalesces renderer rejection breadcrumbs by reason message', () => {
emitRendererBreadcrumb({
name: 'renderer_unhandled_rejection',
data: { reasonType: 'string', reasonMessage: 'Remote connection dropped/reconnecting' }
})
expect(recordCoalescedCrashBreadcrumbMock).toHaveBeenCalledWith({
name: 'renderer_unhandled_rejection',
data: { reasonType: 'string', reasonMessage: 'Remote connection dropped/reconnecting' },
coalesceKey: expect.stringContaining('Remote connection dropped/reconnecting'),
minIntervalMs: 30_000
})
expect(recordCrashBreadcrumbMock).not.toHaveBeenCalled()
})
it('keeps the full sanitized message in the coalesce key', () => {
const message = `${'same-prefix-'.repeat(12)}distinct-tail`
emitRendererBreadcrumb({ name: 'renderer_error', data: { message } })
expect(recordCoalescedCrashBreadcrumbMock).toHaveBeenCalledWith({
name: 'renderer_error',
data: { message },
coalesceKey: expect.stringContaining(message),
minIntervalMs: 30_000
})
})
it('does not coalesce same-message errors from different source sites', () => {
emitRendererBreadcrumb({
name: 'renderer_error',
data: { message: 'boom', filename: 'file:///first.js', lineno: 10, colno: 2 }
})
emitRendererBreadcrumb({
name: 'renderer_error',
data: { message: 'boom', filename: 'file:///second.js', lineno: 20, colno: 4 }
})
const [first, second] = recordCoalescedCrashBreadcrumbMock.mock.calls.map(
([args]) => args.coalesceKey
)
expect(first).not.toBe(second)
})
it('does not coalesce same-message rejections from different stacks', () => {
emitRendererBreadcrumb({
name: 'renderer_unhandled_rejection',
data: { reasonMessage: 'boom', reasonStack: 'Error: boom\n at first' }
})
emitRendererBreadcrumb({
name: 'renderer_unhandled_rejection',
data: { reasonMessage: 'boom', reasonStack: 'Error: boom\n at second' }
})
const [first, second] = recordCoalescedCrashBreadcrumbMock.mock.calls.map(
([args]) => args.coalesceKey
)
expect(first).not.toBe(second)
})
it('records message-less errors without coalescing unrelated failures', () => {
emitRendererBreadcrumb({
name: 'renderer_unhandled_rejection',
data: { reasonType: 'Object' }
})
expect(recordCrashBreadcrumbMock).toHaveBeenCalledWith('renderer_unhandled_rejection', {
reasonType: 'Object'
})
expect(recordCoalescedCrashBreadcrumbMock).not.toHaveBeenCalled()
expect(startSpanMock).toHaveBeenCalledTimes(1)
})
it('uses the Error object message when an error event has no message', () => {
emitRendererBreadcrumb({
name: 'renderer_error',
data: { message: '', errorMessage: 'fallback failure' }
})
expect(recordCoalescedCrashBreadcrumbMock).toHaveBeenCalledWith({
name: 'renderer_error',
data: { message: '', errorMessage: 'fallback failure' },
coalesceKey: expect.stringContaining('fallback failure'),
minIntervalMs: 30_000
})
})
it('does not emit durable trace spans for errors suppressed by coalescing', () => {
recordCoalescedCrashBreadcrumbMock
.mockReturnValueOnce({ suppressedSinceLast: 0 })
.mockReturnValue(undefined)
for (let index = 0; index < 1_000; index += 1) {
emitRendererBreadcrumb({ name: 'renderer_error', data: { message: 'storm' } })
}
expect(recordCoalescedCrashBreadcrumbMock).toHaveBeenCalledTimes(1_000)
expect(startSpanMock).toHaveBeenCalledTimes(1)
expect(spanEndMock).toHaveBeenCalledTimes(1)
})
it('includes the suppressed count when durable tracing resumes', () => {
recordCoalescedCrashBreadcrumbMock.mockReturnValueOnce({ suppressedSinceLast: 999 })
emitRendererBreadcrumb({ name: 'renderer_error', data: { message: 'storm' } })
expect(startSpanMock).toHaveBeenCalledWith('renderer.breadcrumb', {
attributes: {
kind: 'crash-breadcrumb',
'breadcrumb.name': 'renderer_error',
'breadcrumb.data': { message: 'storm', suppressedSinceLast: 999 }
}
})
})
it('records non-error renderer breadcrumbs without coalescing', () => {
emitRendererBreadcrumb({ name: 'renderer_bootstrap_started', data: { dev: true } })
expect(recordCrashBreadcrumbMock).toHaveBeenCalledWith('renderer_bootstrap_started', {
dev: true
})
expect(recordCoalescedCrashBreadcrumbMock).not.toHaveBeenCalled()
})
it('ignores renderer breadcrumbs without a string name', () => {
emitRendererBreadcrumb({ name: 123, data: { message: 'boom' } })
expect(recordCrashBreadcrumbMock).not.toHaveBeenCalled()
expect(recordCoalescedCrashBreadcrumbMock).not.toHaveBeenCalled()
expect(startSpanMock).not.toHaveBeenCalled()
})
})
+2 -64
View File
@@ -57,6 +57,8 @@ vi.mock('./feedback', () => ({
vi.mock('../crash-reporting/crash-breadcrumb-store', () => ({
getCrashBreadcrumbSnapshot: vi.fn(() => []),
// Renderer breadcrumb routing is covered in crash-reporting-renderer-breadcrumbs.test.ts.
recordCoalescedCrashBreadcrumb: vi.fn(),
recordCrashBreadcrumb: (...args: unknown[]) => recordCrashBreadcrumbMock(...args)
}))
@@ -782,68 +784,4 @@ describe('registerCrashReportingHandlers', () => {
expect(recordMock).toHaveBeenCalledTimes(261)
})
it('records sanitized renderer breadcrumbs', () => {
registerCrashReportingHandlers({
getLatestPending: vi.fn(),
getById: vi.fn(),
dismiss: vi.fn(),
markSent: vi.fn(),
listRecent: vi.fn(),
record: vi.fn(),
formatDiagnosticText: vi.fn()
} as never)
listeners.get('crashReports:recordBreadcrumb')?.(null, {
name: 'renderer_error',
data: {
message: 'boom',
count: 2,
ok: true,
empty: null,
badNumber: Number.POSITIVE_INFINITY,
object: { ignored: true }
}
})
expect(recordCrashBreadcrumbMock).toHaveBeenCalledWith('renderer_error', {
message: 'boom',
count: 2,
ok: true,
empty: null
})
expect(startSpanMock).toHaveBeenCalledWith('renderer.breadcrumb', {
attributes: {
kind: 'crash-breadcrumb',
'breadcrumb.name': 'renderer_error',
'breadcrumb.data': {
message: 'boom',
count: 2,
ok: true,
empty: null
}
}
})
expect(spanEndMock).toHaveBeenCalledTimes(1)
})
it('ignores renderer breadcrumbs without a string name', () => {
registerCrashReportingHandlers({
getLatestPending: vi.fn(),
getById: vi.fn(),
dismiss: vi.fn(),
markSent: vi.fn(),
listRecent: vi.fn(),
record: vi.fn(),
formatDiagnosticText: vi.fn()
} as never)
listeners.get('crashReports:recordBreadcrumb')?.(null, {
name: 123,
data: { message: 'boom' }
})
expect(recordCrashBreadcrumbMock).not.toHaveBeenCalled()
expect(startSpanMock).not.toHaveBeenCalled()
})
})
+74 -2
View File
@@ -19,6 +19,7 @@ import { submitFeedback } from './feedback'
import type { CrashReportStore } from '../crash-reporting/crash-report-store'
import {
getCrashBreadcrumbSnapshot,
recordCoalescedCrashBreadcrumb,
recordCrashBreadcrumb
} from '../crash-reporting/crash-breadcrumb-store'
import { startSpan } from '../observability/tracer'
@@ -319,6 +320,52 @@ function buildUncapturedCrashReportText(
)
}
// Why: a repeating renderer error (e.g. a ResizeObserver or SSH-rejection
// storm, #8260) can flush the whole fixed-size breadcrumb ring in seconds,
// erasing the pre-crash trail. Coalesce repeats into one entry that carries a
// suppressed count instead.
const COALESCED_RENDERER_ERROR_BREADCRUMB_NAMES = new Set([
'renderer_error',
'renderer_unhandled_rejection'
])
const RENDERER_ERROR_BREADCRUMB_COALESCE_MS = 30_000
function rendererErrorBreadcrumbCoalesceKey(
name: string,
data: CrashReportBreadcrumbData | undefined
): string | undefined {
const primaryMessage = name === 'renderer_error' ? data?.message : data?.reasonMessage
const fallbackMessage = name === 'renderer_error' ? data?.errorMessage : undefined
const message =
typeof primaryMessage === 'string' && primaryMessage.length > 0
? primaryMessage
: typeof fallbackMessage === 'string' && fallbackMessage.length > 0
? fallbackMessage
: undefined
// Why: message-less failures have no stable identity, so grouping them could
// erase unrelated crash evidence. Sanitization already caps messages at 240 chars.
if (!message) {
return undefined
}
// Why: common messages such as "Script error" or "Cannot read properties"
// can come from unrelated sites. Include sanitized source evidence so one
// failure cannot suppress the breadcrumb for another.
const sourceIdentity =
name === 'renderer_error'
? [
data?.errorStack,
data?.filename,
data?.lineno,
data?.colno,
data?.errorType,
data?.errorName,
data?.errorMessage
]
: [data?.reasonStack, data?.reasonType, data?.reasonName]
return JSON.stringify([name, message, ...sourceIdentity])
}
export function registerCrashReportingHandlers(store: CrashReportStore): void {
ipcMain.removeHandler('crashReports:getLatestPending')
ipcMain.handle('crashReports:getLatestPending', () => getLatestPendingReport(store))
@@ -346,8 +393,33 @@ export function registerCrashReportingHandlers(store: CrashReportStore): void {
return
}
const data = sanitizeRendererBreadcrumbData(args.data)
recordCrashBreadcrumb(args.name, data)
recordRendererBreadcrumbTrace(args.name, data)
if (COALESCED_RENDERER_ERROR_BREADCRUMB_NAMES.has(args.name)) {
const coalesceKey = rendererErrorBreadcrumbCoalesceKey(args.name, data)
if (!coalesceKey) {
recordCrashBreadcrumb(args.name, data)
recordRendererBreadcrumbTrace(args.name, data)
return
}
const coalesceResult = recordCoalescedCrashBreadcrumb({
name: args.name,
data,
coalesceKey,
minIntervalMs: RENDERER_ERROR_BREADCRUMB_COALESCE_MS
})
// Why: tracing every suppressed duplicate would preserve the same
// serialization and disk churn that breadcrumb coalescing removes.
if (coalesceResult) {
recordRendererBreadcrumbTrace(
args.name,
coalesceResult.suppressedSinceLast > 0
? { ...data, suppressedSinceLast: coalesceResult.suppressedSinceLast }
: data
)
}
} else {
recordCrashBreadcrumb(args.name, data)
recordRendererBreadcrumbTrace(args.name, data)
}
}
)
@@ -1232,6 +1232,16 @@ describe('createFilePathLinkProvider range bounds', () => {
expect(shellPathExists).toHaveBeenCalled()
})
it('does not invoke the xterm callback twice when the callback throws', async () => {
const { provider } = createProviderSetup([makeBufferLine('CLAUDE.md')])
const callback = vi.fn(() => {
throw new Error('terminal was disposed')
})
provider.provideLinks(1, callback)
await vi.waitFor(() => expect(callback).toHaveBeenCalledTimes(1))
})
it('shows switch and external-open hint for known worktree root hover', async () => {
setPlatform('Macintosh')
storeState.worktreesByRepo = {
@@ -215,29 +215,33 @@ export function createFilePathLinkProvider(
)
)
)
.then((resolvedLinks) => {
const latestFingerprints = new Set(
buildCandidateLogicalLinesForBufferPosition(buffer, bufferLineNumber).map(
(logicalLine) => logicalLine.fingerprint
.then(
(resolvedLinks) => {
const latestFingerprints = new Set(
buildCandidateLogicalLinesForBufferPosition(buffer, bufferLineNumber).map(
(logicalLine) => logicalLine.fingerprint
)
)
)
const providedLinks = resolvedLinks.filter(
(link): link is ProvidedFileLink => link !== null
)
const links = preferLongestNonOverlappingLinks(providedLinks)
.filter(({ logicalLine }) => latestFingerprints.has(logicalLine.fingerprint))
.map(({ link }) => link)
if (providedLinks.length > 0 && links.length === 0) {
return
const providedLinks = resolvedLinks.filter(
(link): link is ProvidedFileLink => link !== null
)
const links = preferLongestNonOverlappingLinks(providedLinks)
.filter(({ logicalLine }) => latestFingerprints.has(logicalLine.fingerprint))
.map(({ link }) => link)
if (providedLinks.length > 0 && links.length === 0) {
return
}
callback(links.length > 0 ? links : undefined)
},
() => {
// Why: remote probes reject during SSH teardown; using the rejection
// arm avoids treating a consumer callback failure as a probe failure.
callback(undefined)
}
callback(links.length > 0 ? links : undefined)
})
)
.catch(() => {
// Why: remote path-existence probes reject with "Remote connection
// dropped/reconnecting" during SSH teardown. Without a catch the
// rejected Promise.all is unhandled and the crash-breadcrumb buffer
// retains it, growing the renderer heap until it crashes (#8260).
callback(undefined)
// Link discovery is best-effort; a stale xterm callback must not
// recreate the unhandled rejection this path is meant to contain.
})
}
}
+24 -1
View File
@@ -125,11 +125,34 @@ describe('renderer crash diagnostics', () => {
message: 'ResizeObserver loop completed with undelivered notifications.',
preventDefault
})
listeners.get('error')?.[0]?.({
message: 'ResizeObserver loop limit exceeded',
preventDefault
})
expect(preventDefault).toHaveBeenCalledTimes(1)
expect(preventDefault).toHaveBeenCalledTimes(2)
expect(recordBreadcrumbMock).not.toHaveBeenCalled()
})
it('records application errors that only mention a ResizeObserver loop', () => {
diagnostics.installRendererCrashDiagnostics()
recordBreadcrumbMock.mockClear()
const preventDefault = vi.fn()
listeners.get('error')?.[0]?.({
message: 'ResizeObserver loop failed while rendering the terminal',
preventDefault
})
expect(preventDefault).not.toHaveBeenCalled()
expect(recordBreadcrumbMock).toHaveBeenCalledWith({
name: 'renderer_error',
data: expect.objectContaining({
message: 'ResizeObserver loop failed while rendering the terminal'
})
})
})
it('disposes global listeners and the memory interval', () => {
diagnostics.installRendererCrashDiagnostics()
+5 -1
View File
@@ -67,7 +67,11 @@ function recordRendererError(event: ErrorEvent): void {
// Why: "ResizeObserver loop completed" is a benign, self-resolving Chromium
// quirk. Recording it fills the breadcrumb buffer and inflates the error
// count without diagnostic value, contributing to renderer heap growth (#8260).
if (/ResizeObserver loop/i.test(event.message)) {
if (
/^ResizeObserver loop (?:limit exceeded|completed with undelivered notifications)\.?$/i.test(
event.message
)
) {
event.preventDefault()
return
}