Skip to content

Stop session stack pollution on repeated teardown starts - #5771

Open
DenysKuchma wants to merge 2 commits into
4.xfrom
fix/recorder-session-restore
Open

DenysKuchma wants to merge 2 commits into
4.xfrom
fix/recorder-session-restore

Conversation

@DenysKuchma

Copy link
Copy Markdown
Collaborator

Summary

Fixes #5769.
In run-workers --by pool, a Before hook failing after loginAs() left the 'teardown' recorder session unbalanced. Every subsequent failure re-started it, pushing stale promises onto the shared session stack (oldPromises), until session.restore() chained a promise onto itself — "Chaining cycle detected" rejections grew exponentially and the worker died of OOM with OK | 0 passed

@gololdf1sh

Copy link
Copy Markdown
Contributor

Thanks! Works for the #5769 repro. But with retry: { Before: 1 } in the config (we use { Scenario: 1, Before: 1 }) it still breaks in --by pool: the failed Before error keeps coming back as UNHANDLED REJECTION (16 → 1,957 → 133K …), the worker runs out of memory, and the summary says OK | 0 passed.

Repro: https://github.com/gololdf1sh/codeceptjs-5769-repro (npm install && npm test)

npm test output (trimmed)
     1 [Worker 01] Suite 10 --
    16 [Worker 1] UNHANDLED REJECTION: element (#missing) still not present on page after 1 sec
…
     1 [Worker 01] Suite 07 --
  1957 [Worker 1] UNHANDLED REJECTION: element (#missing) still not present on page after 1 sec
…
     1 [Worker 01] Suite 04 --
133505 [Worker 1] UNHANDLED REJECTION: element (#missing) still not present on page after 1 sec
…
     1 [Worker 01]   FAIL  | 11 passed, 11 failed, 6 failedHooks   // 2s
…
     1 [Main] Worker Error: Error [ERR_WORKER_OUT_OF_MEMORY]: Worker terminated due to reaching memory limit: JS heap out of memory
     1   OK  | 0 passed   // 59s

@DenysKuchma

Copy link
Copy Markdown
Collaborator Author

@gololdf1sh done

@DavertMik

Copy link
Copy Markdown
Contributor

👷‍♀️ Is it done
Partially. Prevent worker crashes after authenticated setup failures.

The added cleanup is skipped with retry: { Before: 1 }, leaving the reported retry case unaddressed. This is a static review; scenarios were not executed.

🦕 Backwards Compatibility
Existing configurations remain valid, but nested execution scopes are affected by the regression below.

🌋 Merge Risks
Nested scopes restore the wrong parent — authors nesting within blocks lose the outer execution boundary in lib/recorder.js.

🔬 What must be verified
What if a QA engineer retries failing authenticated setup once — retry completes; remaining pooled tests execute; failures are reported.
What if a QA engineer runs repeated authenticated setup failures — all suites execute without memory exhaustion or false success.
What if a QA engineer’s saved login expires during setup — authentication recovers; subsequent failures remain isolated.

This branch has not been deployed

No deployments
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.

run-workers --by pool: failing Before after auth loginAs() hangs the worker; remaining tests never run

3 participants