diff --git a/lib/mocha/asyncWrapper.js b/lib/mocha/asyncWrapper.js index 1457a5ce1..bc11f2495 100644 --- a/lib/mocha/asyncWrapper.js +++ b/lib/mocha/asyncWrapper.js @@ -54,7 +54,10 @@ export function test(test) { test.fn = function (done) { const doneFn = makeDoneCallableOnce(done) + let recoveryStarted = false recorder.errHandler(err => { + if (recoveryStarted) return + recoveryStarted = true recorder.session.start('teardown') recorder.cleanAsyncErr() if (test.throws) { @@ -114,7 +117,13 @@ export function test(test) { export function injected(fn, suite, hookName) { return function (done) { const doneFn = makeDoneCallableOnce(done) + const opts = suite.opts || {} + const retries = opts[`retry${ucfirst(hookName)}`] || 0 + let retryPending = retries > 0 + let recoveryStarted = false const errHandler = err => { + if (retryPending || recoveryStarted) return + recoveryStarted = true recorder.session.start('teardown') recorder.cleanAsyncErr() if (['before', 'beforeSuite'].includes(hookName)) { @@ -149,9 +158,6 @@ export function injected(fn, suite, hookName) { }) } - const opts = suite.opts || {} - const retries = opts[`retry${ucfirst(hookName)}`] || 0 - const currentTest = hookName === 'before' || hookName === 'after' ? suite?.ctx?.currentTest : null promiseRetry( @@ -165,8 +171,13 @@ export function injected(fn, suite, hookName) { retry(err) } finally { if (number < retries) { + // drain the failed chain so in-flight steps (sessions, listeners) + // finish before the next attempt starts queueing new ones + await recorder.promise().catch(() => {}) recorder.stop() recorder.start() + } else { + retryPending = false } } }, diff --git a/lib/plugin/auth.js b/lib/plugin/auth.js index 7e89c0635..e487017b5 100644 --- a/lib/plugin/auth.js +++ b/lib/plugin/auth.js @@ -303,6 +303,7 @@ export default function (config) { section.start() + const queueId = recorder.getQueueId() const cookies = store[`${name}_session`] const shouldAwait = isAsyncFunction(userSession.login) || isAsyncFunction(userSession.restore) || isAsyncFunction(userSession.check) @@ -338,6 +339,7 @@ export default function (config) { } section.end() recorder.session.catch(err => { + if (recorder.getQueueId() !== queueId) return output.debug(`Failed auto login for ${name} due to ${err}`) output.debug('Logging in again') recorder.session.start('auto login') @@ -355,6 +357,7 @@ export default function (config) { }) }) recorder.add(() => { + if (recorder.getQueueId() !== queueId) return recorder.session.restore('check login') }) diff --git a/lib/recorder.js b/lib/recorder.js index a83e0baa1..38893a1fd 100644 --- a/lib/recorder.js +++ b/lib/recorder.js @@ -89,6 +89,8 @@ export default { */ reset() { if (promise && running) this.catch() + else swallow(promise) + oldPromises.forEach(swallow) queueId++ sessionId = null sessionStack = [] // Clear the session stack @@ -125,6 +127,16 @@ export default { * @inner */ start(name) { + if (sessionId === name) { + // re-starting the same session abandons the current (broken) chain + // without polluting the session stack + debug(`${currentQueue()}Session <${name}> already started, abandoning its chain`) + tasks.push('--->') + this.running = true + swallow(promise) + promise = Promise.resolve() + return + } if (sessionId) { debug(`${currentQueue()}Session already started as ${sessionId}, nesting session ${name}`) // Push current session to stack instead of restoring it @@ -136,7 +148,7 @@ export default { } debug(`${currentQueue()}Starting <${name}> session`) tasks.push('--->') - oldPromises.push(promise) + oldPromises.push(swallow(promise)) this.running = true sessionId = name promise = Promise.resolve() @@ -151,7 +163,8 @@ export default { debug(`${currentQueue()}Finalize <${name}> session`) this.running = false this.catch(errFn) - promise = promise.then(() => oldPromises.pop()) + const restored = oldPromises.pop() + if (restored && restored !== promise) promise = promise.then(() => restored) // Restore parent session from stack if available if (sessionStack.length > 0) { @@ -426,6 +439,11 @@ export default { }, } +function swallow(p) { + if (p) p.catch(() => {}) + return p +} + function getTimeoutPromise(timeoutMs, taskName) { let timer if (timeoutMs) debug(`Timing out in ${timeoutMs}ms`) diff --git a/test/unit/recorder_test.js b/test/unit/recorder_test.js index 04096ee6d..87c492250 100644 --- a/test/unit/recorder_test.js +++ b/test/unit/recorder_test.js @@ -28,6 +28,76 @@ describe('Recorder', () => { recorder.add(() => (order += 'b')) return recorder.promise().then(() => expect(order).is.equal('acdb')) }) + + it('re-starting the same session name does not pollute the session stack', () => { + let order = '' + recorder.add(() => (order += 'a')) + recorder.add(() => { + recorder.session.start('teardown') + recorder.add(() => (order += 'c')) + recorder.session.start('teardown') // leaked teardown re-start: no sessionStack/oldPromises push + recorder.add(() => (order += 'd')) + }) + recorder.add(() => recorder.session.restore('teardown')) + recorder.add(() => (order += 'b')) + // steps already attached to the live chain still run; a broken one would be abandoned + return recorder.promise().then(() => expect(order).is.equal('acdb')) + }) + + it('does not chain a promise onto itself when restoring an unbalanced stack', async () => { + const unhandled = [] + const onUnhandled = e => unhandled.push(e) + process.on('unhandledRejection', onUnhandled) + try { + recorder.session.start('s1') + recorder.add(() => 'step') + recorder.session.restore('s1') + recorder.session.restore('extra') // nothing left to pop, must not create a cycle + await recorder.promise() + await new Promise(r => setTimeout(r, 20)) + expect(unhandled).to.have.length(0) + } finally { + process.removeListener('unhandledRejection', onUnhandled) + } + }) + + it('does not report unhandled rejections for chains abandoned on session re-start', async () => { + const unhandled = [] + const onUnhandled = e => unhandled.push(e) + process.on('unhandledRejection', onUnhandled) + try { + recorder.add('failing step', () => { + throw new Error('boom') + }) + recorder.session.start('teardown') + recorder.session.start('teardown') // abandons the chain holding the failing step + recorder.add(() => 'recovered') + await recorder.promise() + await new Promise(r => setTimeout(r, 20)) + expect(unhandled).to.have.length(0) + } finally { + process.removeListener('unhandledRejection', onUnhandled) + } + }) + + it('does not report unhandled rejections for chains discarded by reset', async () => { + const unhandled = [] + const onUnhandled = e => unhandled.push(e) + process.on('unhandledRejection', onUnhandled) + try { + recorder.add('failing step', () => { + throw new Error('boom') + }) + recorder.stop() + recorder.start() + recorder.add(() => 'fresh') + await recorder.promise() + await new Promise(r => setTimeout(r, 20)) + expect(unhandled).to.have.length(0) + } finally { + process.removeListener('unhandledRejection', onUnhandled) + } + }) }) describe('#add', () => {