Skip to content

fix: avoid TypeError when timeout timer fires after test resolution (#90)#91

Merged
thetutlage merged 1 commit into
japa:10.xfrom
mjn298:fix/timer-race-issue-90
Apr 28, 2026
Merged

fix: avoid TypeError when timeout timer fires after test resolution (#90)#91
thetutlage merged 1 commit into
japa:10.xfrom
mjn298:fix/timer-race-issue-90

Conversation

@mjn298
Copy link
Copy Markdown
Contributor

@mjn298 mjn298 commented Apr 27, 2026

Fixes #90.

Problem

Under event-loop contention, the TestRunner emits unhandled TypeError: Cannot read properties of undefined (reading 'reject') exceptions, failing CI even when every test passes.

Root cause

#createTimeoutTimer and #resetTimer install a setTimeout callback that dereferences this.#timeout!.reject at execution time rather than capture time. When Node dispatches a queued timer callback after the test has already resolved and #clearTimer() has set this.#timeout = undefined, the !. non-null assertion is wrong and the callback throws.

```ts
// src/test/runner.ts
timer: setTimeout(() => this.#timeout!.reject(this.#createError('Test timeout')), duration),
// ^^ #timeout may be undefined by the time this runs
```

Fix

Replace !. with ?. in both call sites. The callback safely becomes a no-op once #clearTimer has nulled the timeout — which is the correct behavior, since the outer promise has already resolved or rejected.

```diff

  • timer: setTimeout(() => this.#timeout!.reject(this.#createError('Test timeout')), duration),
  • timer: setTimeout(() => this.#timeout?.reject(this.#createError('Test timeout')), duration),
    ```

Two character changes total across #createTimeoutTimer and #resetTimer.

Test

Added a deterministic regression test in tests/test/execute.spec.ts. It stubs globalThis.setTimeout to capture the timer callback (so the timer never fires naturally), runs a test that resolves immediately, then invokes the captured callback after #clearTimer has run. Before the fix this throws the exact TypeError from the issue; after the fix it's a no-op.

Verified locally:

  • Without the fix → new test fails with TypeError: Cannot read properties of undefined (reading 'reject')
  • With the fix → full suite green (170/170)
  • Lint clean

Under event-loop contention the timer callback installed by
TestRunner.#createTimeoutTimer / #resetTimer can be queued and
dispatched by Node after #clearTimer has nulled #timeout. The
callback then dereferences this.#timeout!.reject and throws
"Cannot read properties of undefined (reading 'reject')" as an
unhandled TypeError, failing CI even when every test passed.

Replace the non-null assertion with optional chaining so the
callback becomes a no-op once the timeout has been cleared, which
is the correct behavior when the test has already resolved or
rejected the outer promise.

Adds a regression test that stubs setTimeout to capture the timer
callback, lets the runner finish and clear the timer, then invokes
the captured callback to deterministically reproduce the race.

Closes japa#90
@thetutlage thetutlage merged commit b00b477 into japa:10.x Apr 28, 2026
4 checks passed
@thetutlage
Copy link
Copy Markdown
Collaborator

Thanks 👍🏽

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.

Race condition in TestRunner timeout timer: this.#timeout!.reject fires against nulled #timeout under event-loop contention

3 participants