test(crash-reporting): drive the dropped-withdrawal case through a real failing amend

The stub attachAttribution threw, but collectLateSiblingAttributions never calls it, so the failed-write path it named was never exercised. Go through correlateChildProcessDeath with an attachDetails that rejects once and assert the stored document ends up without the label.
2026-09-28 08:02:43 +00:00 · 2026-09-10 22:34:59 -07:00
parent 265ab8e790
commit cb00ebb8bc
1 changed files with 90 additions and 75 deletions
@@ -15,6 +15,10 @@ import { CrashReportStore } from './crash-report-store'
 import { clearCrashBreadcrumbsForTest } from './crash-breadcrumb-store'
 import { ProcessGoneDedupe } from './process-gone-dedupe'
 import { recordProcessGoneCrash, type ProcessGoneCrashEvent } from './process-gone-recorder'
+import {
+  correlateChildProcessDeath,
+  trackRendererSiblingAttribution
+} from './process-gone-sibling-attribution'
 import {
  collectLateSiblingAttributions,
  resetProcessGoneSiblingCorrelationForTest,
@@ -22,11 +26,13 @@ import {
  type ChildProcessDeath
 } from './process-gone-sibling-correlation'
 import { _resetTracerForTests } from '../observability/tracer'
+import type { CrashReportCreateInput } from '../../shared/crash-reporting'

 // Field shape: crash report 11a9d459 (v1.4.199, win32 10.0.19045), renderer
 // crashed/-2147483645 (0x80000003) with a GPU process crash-looping at -17ms / +14ms / +61ms.
 const BREAKPOINT_EXIT = -2_147_483_645
 const GONE_AT = 1_789_021_889_062
+const CONCURRENT = 'concurrent-process-deaths'

 const noMinidump = async () => null
 const originalPlatform = process.platform
@@ -65,11 +71,55 @@ function rendererCrash(): ProcessGoneCrashEvent {
  }
 }

+function gpuDeath(at: number): ChildProcessDeath {
+  return {
+    at,
+    processType: 'GPU',
+    serviceName: 'GPU',
+    reason: 'crashed',
+    exitCode: BREAKPOINT_EXIT
+  }
+}
+
+function rendererReportInput(details: Record<string, unknown>): CrashReportCreateInput {
+  return {
+    source: 'renderer',
+    processType: 'renderer',
+    reason: 'crashed',
+    exitCode: BREAKPOINT_EXIT,
+    appVersion: '1.4.199-test',
+    platform: 'win32',
+    osRelease: '10.0.19045',
+    arch: 'x64',
+    electronVersion: '38.0.0',
+    chromeVersion: '140.0.0.0',
+    details
+  }
+}
+
+let directory: string
+let store: CrashReportStore
+
+beforeEach(async () => {
+  Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
+  directory = await fs.mkdtemp(path.join(os.tmpdir(), 'orca-sibling-attr-'))
+  store = new CrashReportStore(path.join(directory, 'crash-reports.json'))
+  resetProcessGoneSiblingCorrelationForTest()
+  clearCrashBreadcrumbsForTest()
+  _resetTracerForTests()
+})
+
+afterEach(async () => {
+  Object.defineProperty(process, 'platform', { configurable: true, value: originalPlatform })
+  vi.restoreAllMocks()
+  resetProcessGoneSiblingCorrelationForTest()
+  clearCrashBreadcrumbsForTest()
+  _resetTracerForTests()
+  await fs.rm(directory, { recursive: true, force: true })
+})
+
 /** The amend is fire-and-forget behind two real file writes, so poll the stored document. */
-async function rendererDetails(
-  store: CrashReportStore,
-  expectedSiblingCount: number
-): Promise<Record<string, unknown>> {
+async function rendererDetails(expectedSiblingCount: number): Promise<Record<string, unknown>> {
  return vi.waitFor(async () => {
    // By source, not position: a recorded child report would otherwise head the list and
    // make this poll time out instead of fail.
@@ -80,27 +130,6 @@ async function rendererDetails(
 }

 describe('sibling attribution withdrawal', () => {
-  let directory: string
-  let store: CrashReportStore
-
-  beforeEach(async () => {
-    Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
-    directory = await fs.mkdtemp(path.join(os.tmpdir(), 'orca-sibling-attr-'))
-    store = new CrashReportStore(path.join(directory, 'crash-reports.json'))
-    resetProcessGoneSiblingCorrelationForTest()
-    clearCrashBreadcrumbsForTest()
-    _resetTracerForTests()
-  })
-
-  afterEach(async () => {
-    Object.defineProperty(process, 'platform', { configurable: true, value: originalPlatform })
-    vi.restoreAllMocks()
-    resetProcessGoneSiblingCorrelationForTest()
-    clearCrashBreadcrumbsForTest()
-    _resetTracerForTests()
-    await fs.rm(directory, { recursive: true, force: true })
-  })
-
  it('drops crashAttribution once a repeating sibling identity disqualifies it', async () => {
    const dedupe = new ProcessGoneDedupe()
    const now = vi.spyOn(Date, 'now')
@@ -118,7 +147,7 @@ describe('sibling attribution withdrawal', () => {
    now.mockReturnValue(GONE_AT + 61)
    recordProcessGoneCrash(store, gpuChildCrash(), dedupe, noMinidump)

-    const details = await rendererDetails(store, 3)
+    const details = await rendererDetails(3)
    expect(details.siblingProcessDeathRepeats).toBe(2)
    expect(details.siblingProcessDeaths).toBe('GPU +14ms, GPU -17ms, GPU +61ms')
    expect(details).not.toHaveProperty('crashAttribution')
@@ -135,25 +164,15 @@ describe('sibling attribution withdrawal', () => {
    now.mockReturnValue(GONE_AT + 14)
    recordProcessGoneCrash(store, networkServiceCrash(), dedupe, noMinidump)

-    const details = await rendererDetails(store, 2)
+    const details = await rendererDetails(2)
    expect(details.siblingProcessDeathRepeats).toBeUndefined()
-    expect(details.crashAttribution).toBe('concurrent-process-deaths')
+    expect(details.crashAttribution).toBe(CONCURRENT)
  })

  it('withdraws only the disproved key and leaves the rest of the report intact', async () => {
-    const recorded = await store.record({
-      source: 'renderer',
-      processType: 'renderer',
-      reason: 'crashed',
-      exitCode: BREAKPOINT_EXIT,
-      appVersion: '1.4.199-test',
-      platform: 'win32',
-      osRelease: '10.0.19045',
-      arch: 'x64',
-      electronVersion: '38.0.0',
-      chromeVersion: '140.0.0.0',
-      details: { crashAttribution: 'concurrent-process-deaths', minidumpStatus: 'captured' }
-    })
+    const recorded = await store.record(
+      rendererReportInput({ crashAttribution: CONCURRENT, minidumpStatus: 'captured' })
+    )

    const amended = await store.attachDetails(recorded.id, {
      crashAttribution: null,
@@ -171,46 +190,42 @@ describe('sibling attribution withdrawal', () => {
 })

 /**
- * The amend is fire-and-forget: `attachAttribution` logs a failed write and moves on. A
- * withdrawal that never reached the file has to stay owed, or the Windows retry ladder
+ * The amend is fire-and-forget: `siblingAttributionAttacher` logs a failed write and moves
+ * on. A withdrawal that never reached the file has to stay owed, or the Windows retry ladder
 * exhausting once (EPERM/EBUSY) ships the stale label the amend existed to take back.
 */
 describe('a withdrawal amend whose write never lands', () => {
-  const gpuDeath = (at: number): ChildProcessDeath => ({
-    at,
-    processType: 'GPU',
-    serviceName: 'GPU',
-    reason: 'crashed',
-    exitCode: BREAKPOINT_EXIT
-  })
+  it('re-emits it on the next late sibling until the report loses the label', async () => {
+    vi.spyOn(console, 'error').mockImplementation(() => {})
+    const recorded = await store.record(
+      rendererReportInput({ crashAttribution: CONCURRENT, siblingProcessDeathCount: 1 })
+    )
+    const attachDetails = vi
+      .fn<(reportId: string, details: Record<string, unknown>) => Promise<unknown>>()
+      .mockRejectedValueOnce(new Error('EBUSY'))
+      .mockImplementation((reportId, details) => store.attachDetails(reportId, details))

-  beforeEach(() => {
-    Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
-    resetProcessGoneSiblingCorrelationForTest()
-  })
-
-  afterEach(() => {
-    Object.defineProperty(process, 'platform', { configurable: true, value: originalPlatform })
-    resetProcessGoneSiblingCorrelationForTest()
-  })
-
-  it('re-emits it on the next late sibling instead of reporting it withdrawn', () => {
-    trackRendererCrashReport(
-      {
-        at: GONE_AT,
-        reason: 'crashed',
-        exitCode: BREAKPOINT_EXIT,
-        attachAttribution: () => {
-          throw new Error('the amend is fire-and-forget; this write is dropped')
-        }
-      },
-      [gpuDeath(GONE_AT - 17)]
+    trackRendererSiblingAttribution(
+      { source: 'renderer', reason: 'crashed', exitCode: BREAKPOINT_EXIT },
+      GONE_AT,
+      [gpuDeath(GONE_AT - 17)],
+      attachDetails,
+      Promise.resolve(recorded),
+      { source: 'renderer', processType: 'renderer', reason: 'crashed', exitCode: BREAKPOINT_EXIT }
    )

-    const [dropped] = collectLateSiblingAttributions(gpuDeath(GONE_AT + 14))
-    expect(dropped.attribution.crashAttribution).toBeNull()
-    const [retried] = collectLateSiblingAttributions(gpuDeath(GONE_AT + 61))
-    expect(retried.attribution.crashAttribution).toBeNull()
+    correlateChildProcessDeath(gpuDeath(GONE_AT + 14))
+    correlateChildProcessDeath(gpuDeath(GONE_AT + 61))
+
+    await vi.waitFor(async () => {
+      const [persisted] = await store.listRecent()
+      expect(persisted.details).not.toHaveProperty('crashAttribution')
+    })
+    // Both amends carry it: the first was dropped, so the second still owes the withdrawal.
+    expect(attachDetails.mock.calls.map(([, details]) => details.crashAttribution)).toEqual([
+      null,
+      null
+    ])
  })

  it('never withdraws a label the report was never given', () => {