fix(core): re-land asyncWrapper + promise-core error-path fixes onto 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>

Author: 3 people

lib/effects.js

@@ -50,8 +50,10 @@ function within(context, fn) {
             return recorder.promise().then(() => res)
           })
           .catch(e => {
+            finishHelpers()
             finalize()
             recorder.throw(e)
+            return recorder.promise()
           })
       }
 
@@ -203,7 +205,7 @@ async function retryTo(callback, maxTries, pollInterval = 200) {
   const sessionName = 'retryTo'
 
   return new Promise((done, reject) => {
-    let tries = 1
+    let tries = 0
 
     function handleRetryException(err) {
       recorder.throw(err)
@@ -216,7 +218,7 @@ async function retryTo(callback, maxTries, pollInterval = 200) {
       try {
         await callback(tries)
       } catch (err) {
-        handleRetryException(err)
+        recorder.throw(err)
       }
 
       // Call done if no errors
@@ -228,7 +230,7 @@ async function retryTo(callback, maxTries, pollInterval = 200) {
       // Catch errors and retry
       recorder.session.catch(err => {
         recorder.session.restore(`${sessionName} ${tries}`)
-        if (tries <= maxTries) {
+        if (tries < maxTries) {
           output.debug(`Error ${err}... Retrying`)
           recorder.add(`${sessionName} ${tries}`, () => setTimeout(tryBlock, pollInterval))
         } else {

lib/mocha/asyncWrapper.js

@@ -197,23 +197,31 @@ export function setup(suite) {
   return function (done) {
     const doneFn = makeDoneCallableOnce(done)
     recorder.startUnlessRunning()
-    import('./test.js').then(testModule => {
-      const { enhanceMochaTest } = testModule.default || testModule
-      event.emit(event.test.before, enhanceMochaTest(suite?.ctx?.currentTest ?? suite?.currentTest))
-      recorder.add(() => doneFn())
-    })
+    import('./test.js')
+      .then(testModule => {
+        const { enhanceMochaTest } = testModule.default || testModule
+        event.emit(event.test.before, enhanceMochaTest(suite?.ctx?.currentTest ?? suite?.currentTest))
+        recorder.add(() => doneFn())
+      })
+      .catch(err => {
+        doneFn(err)
+      })
   }
 }
 
 export function teardown(suite) {
   return function (done) {
     const doneFn = makeDoneCallableOnce(done)
     recorder.startUnlessRunning()
-    import('./test.js').then(testModule => {
-      const { enhanceMochaTest } = testModule.default || testModule
-      event.emit(event.test.after, enhanceMochaTest(suite?.ctx?.currentTest ?? suite?.currentTest))
-      recorder.add(() => doneFn())
-    })
+    import('./test.js')
+      .then(testModule => {
+        const { enhanceMochaTest } = testModule.default || testModule
+        event.emit(event.test.after, enhanceMochaTest(suite?.ctx?.currentTest ?? suite?.currentTest))
+        recorder.add(() => doneFn())
+      })
+      .catch(err => {
+        doneFn(err)
+      })
   }
 }
 

lib/session.js

@@ -109,6 +109,7 @@ function session(sessionName, config, fn) {
             output.stepShift = 0
             session.restoreVars(sessionName)
             event.dispatcher.removeListener(event.step.after, addContextToStep)
+            recorder.add('restore session on error', () => recorder.session.restore(`session:${sessionName}`))
             recorder.throw(e)
             return recorder.promise()
           })
@@ -124,6 +125,7 @@ function session(sessionName, config, fn) {
           session.restoreVars(sessionName)
           output.stepShift = 0
           event.dispatcher.removeListener(event.step.after, addContextToStep)
+          recorder.session.restore(`session:${sessionName}`)
           throw e
         })
       }

test/unit/mocha/asyncWrapper_test.js

@@ -230,4 +230,43 @@ describe('AsyncWrapper', () => {
       expect(suiteTests[0].err, 'the suite test got the hook error attached').to.be.instanceof(Error)
     })
   })
+
+  describe('setup/teardown import hardening (regression)', () => {
+    beforeEach(() => recorder.start())
+
+    it('setup(): a throwing then-body calls done with the error instead of hanging', async () => {
+      const suite = {
+        ctx: {
+          get currentTest() {
+            throw new Error('setup-boom')
+          },
+        },
+      }
+      const { count, arg } = await runHook(setup(suite), 1000)
+      expect(count).to.equal(1)
+      expect(arg).to.be.instanceof(Error)
+      expect(arg.message).to.equal('setup-boom')
+    })
+
+    it('teardown(): a throwing then-body calls done with the error instead of hanging', async () => {
+      const suite = {
+        ctx: {
+          get currentTest() {
+            throw new Error('teardown-boom')
+          },
+        },
+      }
+      const { count, arg } = await runHook(teardown(suite), 1000)
+      expect(count).to.equal(1)
+      expect(arg).to.be.instanceof(Error)
+      expect(arg.message).to.equal('teardown-boom')
+    })
+
+    it('setup(): happy path calls done with no error', async () => {
+      const suite = { ctx: { currentTest: { title: 'a sample test' } } }
+      const { count, arg } = await runHook(setup(suite), 1000)
+      expect(count).to.equal(1)
+      expect(arg, 'done called with no error').to.be.undefined
+    })
+  })
 })

test/unit/session_composition_test.js

@@ -94,31 +94,28 @@ describe('promise-core composition (characterization)', () => {
       expect(recorder.getCurrentSessionId()).to.equal(null)
     })
 
-    it('FINDING: async callback error leaks the session id and never calls recorder.session.restore', async () => {
+    it('async callback error surfaces the error and restores the session id', async () => {
       session('asyncerr', async () => {
         throw new Error('boom')
       })
-      let rejected
-      await settles(recorder.promise()).catch(e => (rejected = e))
-      // characterized behavior, see plans/001-findings.md
-      expect(rejected, 'outer chain rejects').to.be.instanceof(Error)
-      expect(rejected.message).to.equal('boom')
-      expect(recorder.getCurrentSessionId(), 'session id leaks').to.equal('session:asyncerr')
+      const err = await drain()
+      expect(err, 'outer chain rejects').to.be.instanceof(Error)
+      expect(err.message).to.equal('boom')
+      expect(helper.calls.restoreVars, 'restoreVars called on error').to.be.greaterThan(0)
+      expect(recorder.getCurrentSessionId(), 'session id restored on error').to.equal(null)
     })
 
-    it('FINDING: sync callback whose queued task throws restores vars but leaks the session id', async () => {
+    it('sync callback whose queued task throws surfaces the error and restores the session id', async () => {
       session('syncerr', () => {
         recorder.add(() => {
           throw new Error('boomsync')
         })
       })
       const err = await drain()
-      // The finally recorder.catch re-throws before finalize() runs
-      // recorder.session.restore, so the session id leaks. See plans/001-findings.md
       expect(err, 'error surfaces after draining').to.be.instanceof(Error)
       expect(err.message).to.equal('boomsync')
-      expect(helper.calls.restoreVars, 'restoreVars called by the finally handler').to.equal(1)
-      expect(recorder.getCurrentSessionId(), 'session id leaks').to.equal('session:syncerr')
+      expect(helper.calls.restoreVars, 'restoreVars called by the finally handler').to.be.greaterThan(0)
+      expect(recorder.getCurrentSessionId(), 'session id restored on error').to.equal(null)
     })
   })
 
@@ -134,22 +131,40 @@ describe('promise-core composition (characterization)', () => {
       expect(inside).to.equal(true)
     })
 
-    it('FINDING: async callback error skips _withinEnd and detaches the error onto a trailing task', async () => {
+    it('async callback error runs _withinEnd and propagates the error', async () => {
       within('ctx', async () => {
         throw new Error('boomwithin')
       })
       const err = await drain()
-      // The error is only visible after draining trailing tasks because within()'s
-      // async catch omits `return recorder.promise()`. See plans/001-findings.md
-      expect(err, 'error surfaces after draining').to.be.instanceof(Error)
+      expect(err, 'error surfaces').to.be.instanceof(Error)
       expect(err.message).to.equal('boomwithin')
       expect(helper.calls.withinBegin, '_withinBegin ran').to.be.greaterThan(0)
-      expect(helper.calls.withinEnd, '_withinEnd skipped on error').to.equal(0)
+      expect(helper.calls.withinEnd, '_withinEnd runs on error').to.be.greaterThan(0)
     })
   })
 
   describe('retryTo()', () => {
-    it('FINDING: a synchronously-throwing callback rejects but keeps retrying past the rejection', async () => {
+    it('retries a throwing callback and resolves when a later attempt succeeds', async () => {
+      let firstTries
+      let calls = 0
+      let rejected = null
+      await settles(
+        retryTo(
+          tries => {
+            if (firstTries === undefined) firstTries = tries
+            calls++
+            if (tries < 3) throw new Error('not yet')
+          },
+          5,
+          20,
+        ),
+      ).catch(e => (rejected = e))
+      expect(rejected, 'resolves once an attempt succeeds (no premature reject)').to.equal(null)
+      expect(firstTries, 'first attempt receives tries === 1').to.equal(1)
+      expect(calls, 'ran until the succeeding attempt').to.equal(3)
+    })
+
+    it('a callback that always throws rejects only after exhausting maxTries', async () => {
       let firstTries
       let calls = 0
       let rejected
@@ -165,15 +180,11 @@ describe('promise-core composition (characterization)', () => {
         ),
         3000,
       ).catch(e => (rejected = e))
-      await new Promise(r => setTimeout(r, 300))
-      const finalCalls = calls
-      await new Promise(r => setTimeout(r, 300))
-      // characterized behavior, see plans/001-findings.md
-      expect(rejected, 'retryTo rejects with the real error').to.be.instanceof(Error)
+      await new Promise(r => setTimeout(r, 200))
+      expect(rejected, 'rejects with the real error').to.be.instanceof(Error)
       expect(rejected.message).to.equal('always')
-      expect(firstTries, 'first attempt receives tries === 2 (tries starts at 1, incremented before callback)').to.equal(2)
-      expect(calls, 'retrying continues past the first rejection').to.be.greaterThan(1)
-      expect(calls, 'retries stop once drained (no perpetual loop)').to.equal(finalCalls)
+      expect(firstTries, 'first attempt receives tries === 1').to.equal(1)
+      expect(calls, 'retried exactly maxTries times').to.equal(3)
     })
 
     it('retries via recorder failures then resolves', async () => {