From 99f81c844b30af388fbe5f6269736e5dad0dba13 Mon Sep 17 00:00:00 2001 From: Jaromir Obr Date: Wed, 30 Sep 2026 10:04:07 +0200 Subject: [PATCH] fix(recorder): use retry options of the rule matching the error recorder.add() picked one retry config per task by sorting this.retries in place with a boolean comparator. V8 always selected the Playwright/Puppeteer helper's conditional config, so retryFailedStep's factor/minTimeout/maxTimeout were ignored; JavaScriptCore (Bun) alternated the winner between tasks. Select the retry options from the rule that matches the first error (a rule without `when` still takes precedence) and stop mutating this.retries. Fixes #5723 Co-Authored-By: Claude Opus 5.5 --- lib/recorder.js | 37 +++++++++++++++++++------------------ test/unit/recorder_test.js | 22 ++++++++++++++++++++++ 2 files changed, 41 insertions(+), 18 deletions(-) diff --git a/lib/recorder.js b/lib/recorder.js index 2f55ee093..a83e0baa1 100644 --- a/lib/recorder.js +++ b/lib/recorder.js @@ -202,34 +202,35 @@ export default { debug(chalk.gray(`${currentQueue()} Queued | ${taskName}`)) return (promise = Promise.resolve(promise).then(res => { - // prefer options for non-conditional retries - const retryOpts = this.retries - .sort((r1, r2) => r1.when && !r2.when) - .slice(-1) - .pop() - // no retries or unnamed tasks debug(`${currentQueue()} Running | ${taskName} | Timeout: ${timeout || 'None'}`) - if (retryOpts) debug(`${currentQueue()} Retry opts`, JSON.stringify(retryOpts)) - if (!retryOpts || !taskName || !retry) { + const runTask = () => { const [promise, timer] = getTimeoutPromise(timeout, taskName) return Promise.race([promise, Promise.resolve(res).then(fn)]).finally(() => clearTimeout(timer)) } + // no retries or unnamed tasks + if (!this.retries.length || !taskName || !retry) return runTask() + const retryRules = this.retries.slice().reverse() - return promiseRetry(Object.assign({}, defaultRetryOptions, retryOpts), (retry, number) => { - if (number > 1) output.log(`${currentQueue()}Retrying... Attempt #${number}`) - const [promise, timer] = getTimeoutPromise(timeout, taskName) - return Promise.race([promise, Promise.resolve(res).then(fn)]) - .finally(() => clearTimeout(timer)) - .catch(err => { + // prefer options for non-conditional retries, otherwise use the latest rule matching the error + const findRetryOpts = err => retryRules.find(r => !r.when) || retryRules.find(r => r.when(err)) + + return runTask().catch(firstErr => { + if (ignoredErrs.includes(firstErr)) return + const retryOpts = findRetryOpts(firstErr) + if (!retryOpts) throw firstErr + debug(`${currentQueue()} Retry opts`, JSON.stringify(retryOpts)) + + return promiseRetry(Object.assign({}, defaultRetryOptions, retryOpts), (retry, number) => { + if (number === 1) return retry(firstErr) + output.log(`${currentQueue()}Retrying... Attempt #${number}`) + return runTask().catch(err => { if (ignoredErrs.includes(err)) return - for (const retryObj of retryRules) { - if (!retryObj.when) return retry(err) - if (retryObj.when && retryObj.when(err)) return retry(err) - } + if (findRetryOpts(err)) return retry(err) throw err }) + }) }) })) }, diff --git a/test/unit/recorder_test.js b/test/unit/recorder_test.js index 26f02a71e..04096ee6d 100644 --- a/test/unit/recorder_test.js +++ b/test/unit/recorder_test.js @@ -144,6 +144,28 @@ describe('Recorder', () => { expect(attempts[1] - attempts[0]).to.be.lessThan(500, 'second retry must use default minTimeout, not leaked 800ms') }) + it('should use timing opts of the retry rule matching the error', async function () { + this.timeout(5000) + + recorder.retries = [] + const attempts = [] + recorder.retry({ retries: 1, minTimeout: 600, factor: 1, when: err => err.message === 'not found' }) + recorder.retry({ retries: 1, minTimeout: 10, factor: 1, when: err => err.message.includes('context') }) + recorder.add( + () => { + attempts.push(Date.now()) + if (attempts.length < 2) throw new Error('not found') + }, + undefined, + undefined, + true, + ) + await recorder.promise() + + expect(attempts).to.have.length(2) + expect(attempts[1] - attempts[0]).to.be.greaterThan(500) + }) + it('should prefer opts for non-when retry when possible', () => { let counter = 0 const errorText = 'noerror'