Skip to content

test(core): characterize promise-core error and hang paths#5622

Merged
DavertMik merged 1 commit into
4.xfrom
advisor/001-promise-core-characterization
Jun 13, 2026
Merged

test(core): characterize promise-core error and hang paths#5622
DavertMik merged 1 commit into
4.xfrom
advisor/001-promise-core-characterization

Conversation

@DavertMik

@DavertMik DavertMik commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

What

Browser-free characterization tests for the promise-composition core (lib/recorder.js, lib/session.js, lib/effects.js, lib/mocha/asyncWrapper.js). These pin down the error paths and settle-guarantees so the core can be fixed or refactored safely. No lib/ code is changed — current behavior, including known-bad behavior, is frozen.

This is plan 001 of the promise-core hardening series; it produces the regression gate that plans 003/004 build on.

Tests added

  • test/unit/recorder_test.js (+8): errHandler/catch() routing and stop(), catchWithoutStop terminal-vs-normal errors, throw() after ignoreErr(), nested-session id semantics, unbalanced session restore, and task timeout (both the TimeoutError rejection and the no-late-fire success case).
  • test/unit/session_composition_test.js (new, 8): session() / within() / retryTo() / hopeThat() composition. Uses a fake helper registered through the real container, a settles()/drain() harness that turns deadlocks into fast named failures, and hermetic per-test isolation of the recorder singleton.
  • test/unit/mocha/asyncWrapper_test.js (+4): test() lifecycle (queued-step failure, synchronous throw, test.throws match) and a rejecting injected() before-hook.

Characterized divergences (frozen, not fixed)

The suite documents these latent behaviors (all verified byte-identical to 3.x — latent, not 4.x regressions) for the follow-up fix plan:

  1. recorder.retries is never cleared by reset() → leaks retry config across runs.
  2. A stopped recorder turns the next add()-driven op into a silent hang (the highest-severity finding).
  3. session() async error leaks the session id (never calls recorder.session.restore).
  4. session() sync queued-throw restores vars but still leaks the session id.
  5. within() async error skips _withinEnd and detaches the error onto a trailing task (invisible to a caller awaiting within()).
  6. retryTo() keeps retrying after it has already rejected; first attempt receives tries === 2.
  7. Nested-session restore tasks execute in promise-chain order, not lexical order.

Verification

  • npx mocha test/unit/recorder_test.js test/unit/effects_test.js test/unit/session_composition_test.js test/unit/mocha/asyncWrapper_test.js --exit → 40 passing (stable across repeated runs)
  • npm run test:unit → 747 passed, 0 failed, 11 skipped
  • npm run lint → clean
  • git status shows no lib/ changes

Notes for reviewers

  • Hang detection uses an explicit settles() race, not mocha timeouts, so a stalled chain fails with a named error instead of a generic timeout.
  • These tests intentionally freeze current behavior. When a fix lands (e.g. session restore on error), the matching assertion must be updated in the same PR.

🤖 Generated with Claude Code
@claude
test(core): characterize promise-core error and hang paths
5f51d3e 
Adds browser-free characterization tests that pin down the error and
settle-guarantees of the promise composition core, so it can be fixed or
refactored safely. No lib/ code is changed — current behavior (including
known-bad behavior) is frozen.

- recorder_test.js: errHandler/catch routing, catchWithoutStop terminal
  vs normal errors, ignoreErr, nested-session id semantics, unbalanced
  session restore, and task timeout (success + failure).
- session_composition_test.js (new): session()/within()/retryTo()/hopeThat()
  composition with a fake helper registered through the real container, a
  settles()/drain() harness that turns deadlocks into fast named failures,
  and hermetic per-test isolation of the recorder singleton.
- mocha/asyncWrapper_test.js: test() lifecycle (queued-step failure, sync
  throw, test.throws pass) and a rejecting injected() before-hook.

Characterized divergences (session-id leak on error, within skipping
_withinEnd, retryTo retrying past rejection, recorder.retries leaking across
runs, stopped-recorder hang) are documented for the follow-up fix plan.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@DavertMik DavertMik force-pushed the advisor/001-promise-core-characterization branch from b595cec to 5f51d3e Compare June 13, 2026 10:39
This was referenced Jun 13, 2026
Merged
Merged
@DavertMik DavertMik merged commit a615e9f into 4.x Jun 13, 2026
13 checks passed
DavertMik pushed a commit that referenced this pull request Jun 13, 2026
@claude
fix(core): restore sessions and propagate errors on session/within/re…
dca7626 
…tryTo error paths

Fixes four of the latent error-path divergences characterized in #5622, by
making the error paths symmetric with the success paths. The characterization
assertions are updated to the corrected behavior in the same commit.

- within() async error (lib/effects.js): the catch now calls finishHelpers()
  (so helpers' _withinEnd runs on error, not just success) and returns
  recorder.promise() (so the error propagates through within() instead of being
  detached onto a trailing task that a caller awaiting within() never sees).

- session() async error (lib/session.js): schedules a recorder task that runs
  recorder.session.restore so the recorder session is restored on error (it
  was only restored on success, leaking the session id). The existing
  restoreVars/listener cleanup is kept as-is — switching to finalize()'s real
  restoreVars() closes the browser context under BROWSER_RESTART=session.

- session() sync error (lib/session.js): the finally recorder.catch now calls
  recorder.session.restore before re-throwing (the only place that runs on a
  rejected chain).

- retryTo() (lib/effects.js): a thrown callback no longer reject()s the outer
  promise prematurely — it routes through recorder.throw so the retry logic
  owns the outcome (retry, or reject once maxTries is exhausted). A callback
  that throws then succeeds on a later attempt now resolves instead of
  rejecting. tries now starts at 1 on the first attempt (was 2); the retry
  count is preserved (tries < maxTries).

Verified: unit 748/0, runner 273/0 (incl. all retryFailedStep/rerun tests),
acceptance within/session/els green under both BROWSER_RESTART=browser and
=session.

Not changed here (would be unsafe or out of scope, see PR):
recorder.retries clearing (retryFailedStep depends on it), the stopped-recorder
no-op contract, and nested cross-level restore ordering.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
DavertMik added a commit that referenced this pull request Jun 13, 2026
@DavertMik @claude
fix(core): restore sessions and propagate errors on session/within/re…
974e699 
…tryTo error paths (#5633)

Fixes four of the latent error-path divergences characterized in #5622, by
making the error paths symmetric with the success paths. The characterization
assertions are updated to the corrected behavior in the same commit.

- within() async error (lib/effects.js): the catch now calls finishHelpers()
  (so helpers' _withinEnd runs on error, not just success) and returns
  recorder.promise() (so the error propagates through within() instead of being
  detached onto a trailing task that a caller awaiting within() never sees).

- session() async error (lib/session.js): schedules a recorder task that runs
  recorder.session.restore so the recorder session is restored on error (it
  was only restored on success, leaking the session id). The existing
  restoreVars/listener cleanup is kept as-is — switching to finalize()'s real
  restoreVars() closes the browser context under BROWSER_RESTART=session.

- session() sync error (lib/session.js): the finally recorder.catch now calls
  recorder.session.restore before re-throwing (the only place that runs on a
  rejected chain).

- retryTo() (lib/effects.js): a thrown callback no longer reject()s the outer
  promise prematurely — it routes through recorder.throw so the retry logic
  owns the outcome (retry, or reject once maxTries is exhausted). A callback
  that throws then succeeds on a later attempt now resolves instead of
  rejecting. tries now starts at 1 on the first attempt (was 2); the retry
  count is preserved (tries < maxTries).

Verified: unit 748/0, runner 273/0 (incl. all retryFailedStep/rerun tests),
acceptance within/session/els green under both BROWSER_RESTART=browser and
=session.

Not changed here (would be unsafe or out of scope, see PR):
recorder.retries clearing (retryFailedStep depends on it), the stopped-recorder
no-op contract, and nested cross-level restore ordering.

Co-authored-by: DavertMik <davert@testomat.io>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
@DavertMik DavertMik mentioned this pull request Jun 14, 2026
Merged
DavertMik added a commit that referenced this pull request Jun 14, 2026
@DavertMik @claude
fix(core): re-land asyncWrapper + promise-core error-path fixes onto …
eba4a62 
…4.x (#5624, #5633) (#5637)

* fix(mocha): fail fast when test hook import chain rejects

In 4.x the per-test setup()/teardown() hooks load ./test.js via a dynamic
import() with no .catch(). If that import rejects, or the .then body throws
(e.g. enhanceMochaTest on an undefined test, a listener throwing), done() is
never called and the mocha hook hangs forever — the silent-hang failure mode
the 4.0 release is most concerned with.

This mirrors the existing suiteSetup()/suiteTeardown() shape exactly: append a
.catch(err => doneFn(err)) to both import chains. No recorder.errHandler is
added (the per-test handler owns the single errFn slot). makeDoneCallableOnce
already guards against a double done() call.

Adds 3 regression tests: setup() and teardown() with a throwing then-body call
done with the error within 1s instead of hanging, plus a happy-path check.
Reverting the fix makes the two error-path tests hang (verified).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
(cherry picked from commit ac59a40)

* fix(core): restore sessions and propagate errors on session/within/retryTo error paths

Fixes four of the latent error-path divergences characterized in #5622, by
making the error paths symmetric with the success paths. The characterization
assertions are updated to the corrected behavior in the same commit.

- within() async error (lib/effects.js): the catch now calls finishHelpers()
  (so helpers' _withinEnd runs on error, not just success) and returns
  recorder.promise() (so the error propagates through within() instead of being
  detached onto a trailing task that a caller awaiting within() never sees).

- session() async error (lib/session.js): schedules a recorder task that runs
  recorder.session.restore so the recorder session is restored on error (it
  was only restored on success, leaking the session id). The existing
  restoreVars/listener cleanup is kept as-is — switching to finalize()'s real
  restoreVars() closes the browser context under BROWSER_RESTART=session.

- session() sync error (lib/session.js): the finally recorder.catch now calls
  recorder.session.restore before re-throwing (the only place that runs on a
  rejected chain).

- retryTo() (lib/effects.js): a thrown callback no longer reject()s the outer
  promise prematurely — it routes through recorder.throw so the retry logic
  owns the outcome (retry, or reject once maxTries is exhausted). A callback
  that throws then succeeds on a later attempt now resolves instead of
  rejecting. tries now starts at 1 on the first attempt (was 2); the retry
  count is preserved (tries < maxTries).

Verified: unit 748/0, runner 273/0 (incl. all retryFailedStep/rerun tests),
acceptance within/session/els green under both BROWSER_RESTART=browser and
=session.

Not changed here (would be unsafe or out of scope, see PR):
recorder.retries clearing (retryFailedStep depends on it), the stopped-recorder
no-op contract, and nested cross-level restore ordering.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
(cherry picked from commit dca7626)

---------

Co-authored-by: DavertMik <davert@testomat.io>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

1 participant

@DavertMik