Skip to content

Fix recorder: stop mutating shared defaultRetryOptions#5572

Merged
DavertMik merged 1 commit into
codeceptjs:4.xfrom
gololdf1sh:fix/recorder-default-retry-options-mutation
May 18, 2026
Merged

Fix recorder: stop mutating shared defaultRetryOptions#5572
DavertMik merged 1 commit into
codeceptjs:4.xfrom
gololdf1sh:fix/recorder-default-retry-options-mutation

Conversation

@gololdf1sh
Copy link
Copy Markdown

@gololdf1sh gololdf1sh commented May 18, 2026

Problem

In lib/recorder.js:220:

return promiseRetry(Object.assign(defaultRetryOptions, retryOpts), (retry, number) => { ... })

Object.assign(target, source) mutates target and returns the same reference. By passing the module-level defaultRetryOptions as the target, every custom option (minTimeout, factor, maxTimeout, retries) gets merged into the shared defaults and persists for the lifetime of the process.

Subsequent recorder.retry() calls inherit those leaked values, even when they explicitly omit them.

Reproduction

// Scenario 1: explicit custom backoff
Scenario('s1', ({ I }) => {
  I.retry({ retries: 3, minTimeout: 1000, factor: 1.5 }).seeElement('[data-test-id="non-existent"]');
});

// Scenario 2: expects default minTimeout (~150ms)
Scenario('s2', ({ I }) => {
  I.retry(3).seeElement('[data-test-id="non-existent"]');
});

Instrumentation around the Object.assign call:

Scenario 1:
BEFORE: defaultRetryOptions={"retries":0,"minTimeout":150,"maxTimeout":10000}
AFTER:  defaultRetryOptions={"retries":3,"minTimeout":1000,"factor":1.5,...}  sameRef=true

Scenario 2:
BEFORE: defaultRetryOptions={"retries":2,"minTimeout":1000,...}  ← leaked from S1

Scenario 2 fails in ~5s instead of the expected ~700ms because it inherits minTimeout=1000 from the previous scenario.

Fix

Pass a fresh {} as the first arg so a new merged object is produced each call without touching the shared defaults:

-return promiseRetry(Object.assign(defaultRetryOptions, retryOpts), (retry, number) => {
+return promiseRetry(Object.assign({}, defaultRetryOptions, retryOpts), (retry, number) => {

Repro project

gololdf1sh/codeceptjs-retry-test.

`Object.assign(defaultRetryOptions, retryOpts)` returned and used the
shared module-level `defaultRetryOptions` object as the merge target,
so any custom retry option (e.g. `minTimeout`, `factor`) persisted into
the defaults for every later `recorder.retry()` call.

Subsequent steps and tests then silently inherited the previous run's
overrides. A test that explicitly set `minTimeout=1000` poisoned the
defaults for the rest of the session — later steps used 1000ms even
when no `minTimeout` was passed.

Pass a fresh `{}` as the first arg to `Object.assign` so a new merged
options object is produced each call without touching the shared
defaults.
@DavertMik DavertMik merged commit 4ef2af2 into codeceptjs:4.x May 18, 2026
10 checks passed
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.

2 participants