Skip to content

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

Merged
brennanb2025 merged 4 commits into
mainfrom
fix/coalesce-renderer-error-breadcrumbs
Jul 15, 2026
Merged

brennanb2025 merged 4 commits into
mainfrom
fix/coalesce-renderer-error-breadcrumbs

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Jul 15, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Hardens the crash-breadcrumb pipeline against repeated renderer error storms and closes two edge cases in the #8279 guards.

Problem

The crash-breadcrumb ring holds only 30 events and is snapshotted into crash reports. renderer_error and renderer_unhandled_rejection previously used the uncoalesced path, so a repeating error could evict the useful pre-crash trail and create one durable trace record per occurrence. The local trace sink serializes these records and flushes them to disk.

Fix

  • Route renderer errors and unhandled rejections with a stable message through recordCoalescedCrashBreadcrumb using a 30-second window.
  • Key coalescing by the full sanitized message plus sanitized stack/source/type evidence, so unrelated failures with the same generic message cannot suppress each other.
  • Suppress duplicate durable trace spans with the same decision; the next recorded span carries suppressedSinceLast.
  • Keep state bounded: sanitization caps each identity field and the store caps the map at 128 keys.
  • Leave message-less failures uncoalesced because they do not have a safe identity; use errorMessage when an Error object provides one.
  • Handle SSH path-probe rejection separately from xterm callback failures, preventing a throwing/stale callback from being invoked twice or recreating an unhandled rejection.
  • Suppress only Chromium's exact benign ResizeObserver messages; application errors that merely mention a ResizeObserver loop remain visible.

Performance evidence

  • A deterministic 1,000-event duplicate storm creates one durable trace span instead of 1,000.
  • Suppressed duplicates use the store's O(1) lookup/return path.
  • Coalescing state remains capped at 128 keys; no timers, polling, subprocesses, provider scans, or growing buffers are added.

Terminal reliability contract

  • Invariant (xterm-addon.boundary-containment): rejected remote file probes and throwing/stale xterm callbacks remain pane-scoped, invoke the consumer at most once, and do not escape as unhandled rejections.
  • Failure source: [Bug]: Renderer crashes to white screen on macOS — unhandled rejections accumulate causing memory leak and ResizeObserver loop (v1.4.128–1.4.135) #8260 and the post-merge review of fix: stop renderer heap leak from unhandled terminal teardown rejections (#8260) #8279's SSH teardown guard.
  • Oracle: renderer-unit tests force a rejected path-existence probe and a throwing xterm callback, then assert an empty-link result or exactly one callback invocation. Source-aware breadcrumb tests also prove same-message failures at different sites retain distinct identities.
  • Gate: existing experimental, partial xterm-addon.boundary-containment gate; its focused command passes with the terminal file-link suite added to the run.
  • Provider/platform coverage: the containment code is platform-neutral renderer TypeScript. The SSH/remote rejection shape has direct deterministic coverage; local, daemon, WSL, and mobile/relay PTY ownership and I/O paths are unaffected. No live SSH soak or separate Windows/Linux desktop run is claimed here.
  • Performance budget: one bounded candidate pass and the existing path probes remain; no retry, polling, liveness scan, session listing, startup await, or subprocess work is introduced.
  • Diagnostics: repeated renderer failures retain a bounded suppressed count and source-aware identity in both the in-memory breadcrumb and resumed durable trace.
  • Residual gaps: this does not prove the broader rejection-free SSH/xterm heap growth is fixed; that investigation remains in feat(terminal): trim scrollback of hidden SSH terminals after cold-park delay #8655.

Screenshots

No visual change.

Testing

  • 199 focused crash-reporting, observability, renderer-diagnostics, and terminal-link tests pass after rebasing onto current main.
  • Full node/CLI/web TypeScript check passes.
  • Full lint, switch-exhaustiveness, reliability-gate, max-lines-ratchet, bundled-skill, and localization checks pass.
  • Hosted PR verification passed at the prior head on the repository's required Node 24; the pushed review fix triggers a fresh run.

AI Review Report

  • Audited error identity, coalescing bounds, trace serialization, SSH teardown, callback settlement, ResizeObserver classification, cleanup, and source-aware diagnostic preservation.
  • Verified macOS/Linux/Windows-neutral behavior; the SSH rejection path has direct regression coverage and no Git/provider behavior changes.
  • Fixed false coalescing for long, message-less, and same-message/different-source failures, duplicate callback settlement, and over-broad benign-error suppression.

Security Audit

  • Renderer data is sanitized and secret/path-redacted before it enters a coalescing key or durable trace.
  • No new IPC capability, network request, filesystem path, credential handling, or persisted unbounded state is introduced.
  • Source-aware keys remain bounded by per-field sanitization and the existing 128-key store cap.

Notes

This is a follow-up to #8279 / #8260. It protects diagnostic context and removes trace-write amplification, but does not claim to solve the rejection-free SSH/xterm heap growth reported later. That residual terminal-memory investigation is tracked separately in #8655.

@coderabbitai

coderabbitai Bot commented Jul 15, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Renderer error and unhandled rejection breadcrumbs now use bounded coalescing keys and a fixed interval, with suppression metadata included in resumed durable tracing. Non-error breadcrumbs retain direct recording. Terminal link handling separates probe rejections from callback failures to avoid duplicate callbacks. ResizeObserver suppression now targets only recognized benign messages, while other related application errors are recorded. Tests cover these behaviors.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly matches the main change: coalescing repeated renderer error breadcrumbs in crash reporting.
Description check ✅ Passed The description covers Summary, Screenshots, Testing, AI Review Report, Security Audit, and Notes, with enough detail for the PR.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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).
@brennanb2025
brennanb2025 force-pushed the fix/coalesce-renderer-error-breadcrumbs branch from 8268f50 to 6608c0e Compare July 15, 2026 08:57
@brennanb2025
brennanb2025 merged commit 8a7cd84 into main Jul 15, 2026
1 check passed
brennanb2025 added a commit that referenced this pull request Aug 15, 2026
Two ways one burst's suppressed repeats were misattributed in forensics,
found across two adversarial review loops on the sibling-correlation work
but pre-dating it (the coalesce machinery shipped in #8800/#5818/#10729):

- Double-claim: a crash report filed mid-window snapshots the ring, folding
  the suppressed repeats into the emitted crumb — but the next emit still
  re-claimed those repeats in its suppressedSinceLast, reporting one burst
  twice across two crumbs.
- Mirror erasure: a fold resolving onto a re-emitted crumb overwrote the
  count that crumb was born carrying, deleting the previous window's repeats.

Track what each crumb has claimed (carried at emit, resolved by folds) so
every repeat is attributed exactly once. And never resolve into an evicted
crumb: when a storm pushes the burst crumb out of the 30-entry ring — or
past the retained-slot snapshot budget — mid-window, folding there would
mark the repeats claimed by evidence no snapshot can see and the burst
would vanish from the record entirely; drop the handle so the next emit
claims them instead.

Semantic note for trace-mirroring consumers: the returned
suppressedSinceLast is now net of already-folded repeats, so exactly-once
holds over the union of trace spans and report snapshots rather than
within the trace stream alone.
brennanb2025 added a commit that referenced this pull request Aug 15, 2026
Two ways one burst's suppressed repeats were misattributed in forensics,
found across two adversarial review loops on the sibling-correlation work
but pre-dating it (the coalesce machinery shipped in #8800/#5818/#10729):

- Double-claim: a crash report filed mid-window snapshots the ring, folding
  the suppressed repeats into the emitted crumb — but the next emit still
  re-claimed those repeats in its suppressedSinceLast, reporting one burst
  twice across two crumbs.
- Mirror erasure: a fold resolving onto a re-emitted crumb overwrote the
  count that crumb was born carrying, deleting the previous window's repeats.

Track what each crumb has claimed (carried at emit, resolved by folds) so
every repeat is attributed exactly once. And never resolve into an evicted
crumb: when a storm pushes the burst crumb out of the 30-entry ring — or
past the retained-slot snapshot budget — mid-window, folding there would
mark the repeats claimed by evidence no snapshot can see and the burst
would vanish from the record entirely; drop the handle so the next emit
claims them instead.

Semantic note for trace-mirroring consumers: the returned
suppressedSinceLast is now net of already-folded repeats, so exactly-once
holds over the union of trace spans and report snapshots rather than
within the trace stream alone.
brennanb2025 added a commit that referenced this pull request Aug 15, 2026
Two ways one burst's suppressed repeats were misattributed in forensics,
found across two adversarial review loops on the sibling-correlation work
but pre-dating it (the coalesce machinery shipped in #8800/#5818/#10729):

- Double-claim: a crash report filed mid-window snapshots the ring, folding
  the suppressed repeats into the emitted crumb — but the next emit still
  re-claimed those repeats in its suppressedSinceLast, reporting one burst
  twice across two crumbs.
- Mirror erasure: a fold resolving onto a re-emitted crumb overwrote the
  count that crumb was born carrying, deleting the previous window's repeats.

Track what each crumb has claimed (carried at emit, resolved by folds) so
every repeat is attributed exactly once. And never resolve into an evicted
crumb: when a storm pushes the burst crumb out of the 30-entry ring — or
past the retained-slot snapshot budget — mid-window, folding there would
mark the repeats claimed by evidence no snapshot can see and the burst
would vanish from the record entirely; drop the handle so the next emit
claims them instead.

Semantic note for trace-mirroring consumers: the returned
suppressedSinceLast is now net of already-folded repeats, so exactly-once
holds over the union of trace spans and report snapshots rather than
within the trace stream alone.
brennanb2025 added a commit that referenced this pull request Aug 15, 2026
Two ways one burst's suppressed repeats were misattributed in forensics,
found across two adversarial review loops on the sibling-correlation work
but pre-dating it (the coalesce machinery shipped in #8800/#5818/#10729):

- Double-claim: a crash report filed mid-window snapshots the ring, folding
  the suppressed repeats into the emitted crumb — but the next emit still
  re-claimed those repeats in its suppressedSinceLast, reporting one burst
  twice across two crumbs.
- Mirror erasure: a fold resolving onto a re-emitted crumb overwrote the
  count that crumb was born carrying, deleting the previous window's repeats.

Track what each crumb has claimed (carried at emit, resolved by folds) so
every repeat is attributed exactly once. And never resolve into an evicted
crumb: when a storm pushes the burst crumb out of the 30-entry ring — or
past the retained-slot snapshot budget — mid-window, folding there would
mark the repeats claimed by evidence no snapshot can see and the burst
would vanish from the record entirely; drop the handle so the next emit
claims them instead.

Semantic note for trace-mirroring consumers: the returned
suppressedSinceLast is now net of already-folded repeats, so exactly-once
holds over the union of trace spans and report snapshots rather than
within the trace stream alone.
brennanb2025 added a commit that referenced this pull request Aug 16, 2026
Two ways one burst's suppressed repeats were misattributed in forensics,
found across two adversarial review loops on the sibling-correlation work
but pre-dating it (the coalesce machinery shipped in #8800/#5818/#10729):

- Double-claim: a crash report filed mid-window snapshots the ring, folding
  the suppressed repeats into the emitted crumb — but the next emit still
  re-claimed those repeats in its suppressedSinceLast, reporting one burst
  twice across two crumbs.
- Mirror erasure: a fold resolving onto a re-emitted crumb overwrote the
  count that crumb was born carrying, deleting the previous window's repeats.

Track what each crumb has claimed (carried at emit, resolved by folds) so
every repeat is attributed exactly once. And never resolve into an evicted
crumb: when a storm pushes the burst crumb out of the 30-entry ring — or
past the retained-slot snapshot budget — mid-window, folding there would
mark the repeats claimed by evidence no snapshot can see and the burst
would vanish from the record entirely; drop the handle so the next emit
claims them instead.

Semantic note for trace-mirroring consumers: the returned
suppressedSinceLast is now net of already-folded repeats, so exactly-once
holds over the union of trace spans and report snapshots rather than
within the trace stream alone.
brennanb2025 added a commit that referenced this pull request Aug 16, 2026
* fix(crash-reporting): attribute coalesced repeats exactly once

Two ways one burst's suppressed repeats were misattributed in forensics,
found across two adversarial review loops on the sibling-correlation work
but pre-dating it (the coalesce machinery shipped in #8800/#5818/#10729):

- Double-claim: a crash report filed mid-window snapshots the ring, folding
  the suppressed repeats into the emitted crumb — but the next emit still
  re-claimed those repeats in its suppressedSinceLast, reporting one burst
  twice across two crumbs.
- Mirror erasure: a fold resolving onto a re-emitted crumb overwrote the
  count that crumb was born carrying, deleting the previous window's repeats.

Track what each crumb has claimed (carried at emit, resolved by folds) so
every repeat is attributed exactly once. And never resolve into an evicted
crumb: when a storm pushes the burst crumb out of the 30-entry ring — or
past the retained-slot snapshot budget — mid-window, folding there would
mark the repeats claimed by evidence no snapshot can see and the burst
would vanish from the record entirely; drop the handle so the next emit
claims them instead.

Semantic note for trace-mirroring consumers: the returned
suppressedSinceLast is now net of already-folded repeats, so exactly-once
holds over the union of trace spans and report snapshots rather than
within the trace stream alone.

* fix(crash-reporting): preserve orphaned repeat debt on cleanup

* fix(crash-reporting): preserve data-less repeat debt

* fix(crash-reporting): make coalescing window monotonic
AmethystLiang pushed a commit that referenced this pull request Aug 17, 2026
* fix(crash-reporting): attribute coalesced repeats exactly once

Two ways one burst's suppressed repeats were misattributed in forensics,
found across two adversarial review loops on the sibling-correlation work
but pre-dating it (the coalesce machinery shipped in #8800/#5818/#10729):

- Double-claim: a crash report filed mid-window snapshots the ring, folding
  the suppressed repeats into the emitted crumb — but the next emit still
  re-claimed those repeats in its suppressedSinceLast, reporting one burst
  twice across two crumbs.
- Mirror erasure: a fold resolving onto a re-emitted crumb overwrote the
  count that crumb was born carrying, deleting the previous window's repeats.

Track what each crumb has claimed (carried at emit, resolved by folds) so
every repeat is attributed exactly once. And never resolve into an evicted
crumb: when a storm pushes the burst crumb out of the 30-entry ring — or
past the retained-slot snapshot budget — mid-window, folding there would
mark the repeats claimed by evidence no snapshot can see and the burst
would vanish from the record entirely; drop the handle so the next emit
claims them instead.

Semantic note for trace-mirroring consumers: the returned
suppressedSinceLast is now net of already-folded repeats, so exactly-once
holds over the union of trace spans and report snapshots rather than
within the trace stream alone.

* fix(crash-reporting): preserve orphaned repeat debt on cleanup

* fix(crash-reporting): preserve data-less repeat debt

* fix(crash-reporting): make coalescing window monotonic
enrick-dev pushed a commit to enrick-dev/orca that referenced this pull request Oct 15, 2026
…tablyai#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 stablyai#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
enrick-dev pushed a commit to enrick-dev/orca that referenced this pull request Oct 15, 2026
…14666)

* fix(crash-reporting): attribute coalesced repeats exactly once

Two ways one burst's suppressed repeats were misattributed in forensics,
found across two adversarial review loops on the sibling-correlation work
but pre-dating it (the coalesce machinery shipped in stablyai#8800/stablyai#5818/stablyai#10729):

- Double-claim: a crash report filed mid-window snapshots the ring, folding
  the suppressed repeats into the emitted crumb — but the next emit still
  re-claimed those repeats in its suppressedSinceLast, reporting one burst
  twice across two crumbs.
- Mirror erasure: a fold resolving onto a re-emitted crumb overwrote the
  count that crumb was born carrying, deleting the previous window's repeats.

Track what each crumb has claimed (carried at emit, resolved by folds) so
every repeat is attributed exactly once. And never resolve into an evicted
crumb: when a storm pushes the burst crumb out of the 30-entry ring — or
past the retained-slot snapshot budget — mid-window, folding there would
mark the repeats claimed by evidence no snapshot can see and the burst
would vanish from the record entirely; drop the handle so the next emit
claims them instead.

Semantic note for trace-mirroring consumers: the returned
suppressedSinceLast is now net of already-folded repeats, so exactly-once
holds over the union of trace spans and report snapshots rather than
within the trace stream alone.

* fix(crash-reporting): preserve orphaned repeat debt on cleanup

* fix(crash-reporting): preserve data-less repeat debt

* fix(crash-reporting): make coalescing window monotonic
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant