Skip to content

a second in-process build of a directory whose file throws at import reports no error, so the differentials compare error parity by luck #2368

Description

@lex00

What

Under vitest, the second and every later build()/discover() of a source directory containing a file that throws while being imported reports no error at all. The first build in the process reports it. Nothing about the source changed between the two.

Found by #2347's adversarial corpus entry, whose nullish-property-read.ts is a file that folds to a refusal and then throws a TypeError when the run path imports it (the #2328 shape).

Reproduction

examples/fold-adversarial/src/nullish-property-read.ts is the fixture. Two discover() calls on examples/fold-adversarial/src, same options, same process:

discover #1: sourceFiles=9 entities=8 errors=1
discover #2: sourceFiles=9 entities=8 errors=0

and the import underneath, called three times after that first failure:

importModule #1: RESOLVED keys=["nullishRead"]
importModule #2: RESOLVED keys=["nullishRead"]
importModule #3: RESOLVED keys=["nullishRead"]

The same module under plain Node ESM, three imports:

node import #1: THREW Cannot read properties of undefined (reading 'k')
node import #2: THREW Cannot read properties of undefined (reading 'k')
node import #3: THREW Cannot read properties of undefined (reading 'k')

Node caches the evaluation failure and re-throws it forever, which is what the spec requires. Vitest's module runner caches the module as evaluated and hands later importers a namespace carrying whichever bindings were initialized before the throw — nullishRead is present, undefined. importModule (packages/core/src/discovery/import.ts) is a plain await import(path), so it has nothing to notice.

Why it matters

It is invisible to a real chant build, which imports a directory once per process. It is not invisible to the six differentials in examples/, every one of which builds the same directory two or three times in one process and compares the results. For an entry with a throwing file:

  • whichever build runs first carries the error, and every build after it reports none;
  • so normalizeErrors(a) vs normalizeErrors(b) compares ["…DiscoveryError…"] against [] or [] against [] depending purely on the order the suite happens to build in;
  • and the isolation retry that exists precisely to rule out cross-build state bleed does not fire, because it is triggered by output drift and a file that throws contributes no output to drift.

json-boundary-differential.test.ts fails on the fixture for exactly this reason (JSON-vs-run error drift … expected [] to deeply equal [ Array(1) ]); fold-differential.test.ts passes on it only because its classification probe build absorbs the error before the comparison starts, which is luck, not a property.

This is the same family as chant#1032 — build() is not safe to call twice on the same directory in one process — and the same family as the propagate() in-place mutation the differentials already document. It is a new member of it, on a path nothing had exercised, because until #2347 no corpus entry had a file that throws.

Workaround in place

#2347 widens the isolation-retry trigger in fold-differential.test.ts and json-boundary-differential.test.ts from "outputs drifted" to "outputs or errors drifted" (errorsEqual, examples/differential-corpus.ts), so a mismatch of this kind is retried with vi.resetModules() and both sides are compared on first-build footing. examples/fold-adversarial/fold-adversarial.test.ts asserts the fixture's fold-vs-run error parity directly, with an explicit reset around every build, rather than trusting the shared harness's ordering.

That is a workaround, not a fix: the underlying non-idempotency is still there, and any suite that builds a directory twice without isolating modules will still lose the error.

Do

Decide whether chant wants importModule to be robust to it — recording the failure per file and replaying it, so a second discover() in one process reports what the first did regardless of the runner underneath — or whether the differentials owning the isolation is the right layer for it. Either way the current state is that error parity is a claim the harness can only make when it isolates, and nothing says so.

Refs #2347, #2328, #1032.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions