Skip to content

Merge queue: TabErrors.test.ts leaks a late console.warn and fails the test check with a fully green suite #14646

Description

@christian-byrne

Intermittently evicts PRs from the merge queue. Hit #14145 today; the same signature evicted #14144 earlier.

What the failure looks like

Every test passes — 1092 files, 14,905 passed, 8 skipped — and the check still fails:

⎯⎯⎯ Unhandled Errors ⎯⎯⎯
Vitest caught 2 unhandled errors during the test run.

EnvironmentTeardownError: [vitest-worker]: Closing rpc while "onUserConsoleLog" was pending
This error originated in "src/components/rightSidePanel/errors/TabErrors.test.ts"

 Test Files  1092 passed (1092)
      Tests  14905 passed | 8 skipped (14913)
     Errors  2 errors

Vitest exits non-zero on unhandled errors regardless of test results, so the merge-queue test check fails and the PR is dropped — while the PR's own status checks still read green from an earlier run. That mismatch is what makes it confusing to diagnose: mergeStateStatus is CLEAN, every check is SUCCESS, and the PR silently leaves the queue.

Mechanism

src/components/rightSidePanel/errors/useErrorGroups.ts:475-495:

const results = await Promise.allSettled(
  toResolve.map(async (n) => ({
    type: n.type,
    packId: (await inferPackFromNodeName.call(n.type))?.id ?? null
  }))
)
if (cancelled) return
...
    console.warn('Failed to resolve pack ID:', r.reason)

TabErrors.test.ts mounts the component but never unmounts it, so nothing sets cancelled. When inferPackFromNodeName rejects, the allSettled promise can settle after the test file has finished and the worker's rpc is closing — and the console.warn is what trips onUserConsoleLog.

Timing-dependent, which is why it doesn't reproduce locally: the CI run takes ~724s against ~64s on a dev machine, so the window is far wider under load.

Fixes, cheapest first

  1. Mock inferPackFromNodeName in the test so no rejection is possible — most targeted, no production change.
  2. Unmount in an afterEach so the composable's cleanup sets cancelled — fixes the class rather than the instance, since any late log from this component would do the same.
  3. Optionally drop the console.warn to a non-console channel; a failed pack lookup is already reflected in the resolved map.

I'd do (1) and (2) together.

Why it matters beyond this feature

This is a silent merge-queue eviction with a fully green PR. Anyone hitting it will re-enqueue, get lucky or not, and have no signal pointing at the cause — the PR shows nothing wrong. Worth fixing rather than tolerating.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions