Skip to content

Conversation

Samuel-StO
Copy link
Contributor

Playwright Helper: Remove duplicate async initialization causing race conditions

Summary

This PR removes an unnecessary setTimeout(... _init()) call from the Playwright helper constructor that caused _init() to run twice in parallel during framework bootstrap. Initialization is already performed by the CodeceptJS container (container.createHelpers -> helper._init()), so the deferred constructor call was redundant and risky.

Background

After merging PR #5089 (custom locator strategies + selector engine registration improvements), the constructor still contained (or reintroduced) an asynchronous fire‑and‑forget initialization pattern:

setTimeout(() => this._init().catch(console.error), 0)

At runtime this led to two near-simultaneous executions:

  1. Automatic container call: global.createHelpers -> Playwright._init
  2. Deferred constructor invocation via setTimeout

Result: duplicate selector engine registration attempts and potential race conditions (especially if future logic adds non-idempotent side effects).

Problem Details

Stack traces showed two _init executions for a single codecept init / test catalog discovery phase. Although selector engine registration was wrapped in try/catch, this produced noisy logs and increased risk of subtle state bugs (e.g. shared sets like registeredCustomLocatorStrategies).

Changes

  • Removed the deferred setTimeout call from the Playwright helper constructor.
  • Relied solely on the container-managed _init() lifecycle.
  • (No functional behavior loss: _startBrowser() already awaits _init() explicitly.)

Rationale

  • Avoid hidden async side effects inside the constructor.
  • Make initialization order predictable and single-pass.
  • Reduce noise (“already registered”) and eliminate potential race windows.

What This Does NOT Change

  • Public API of the Playwright helper.
  • Custom locator strategy registration logic.
  • Restart/session behavior.
  • Trace/video/HAR setup.

Reproduction Before Fix

  1. Run a script that loads CodeceptJS programmatically (e.g. introspection or test discovery).
  2. Observe duplicated _init stack traces and selector registration attempts.

After Fix

Only one _init call—triggered by the container—appears in traces.

Testing & Safety

  • Manual verification: single _init invocation during bootstrap.
  • Selector engines still registered once; no "already registered" warnings.
  • No timing dependency detected that relied on the deferred microtask/macro task gap.

Potential Follow-Up (Optional)

If future contributions add asynchronous prep stages that must be serialized:

  • Introduce _initialized / _initPromise guards (idempotence) inside _init().
  • Add a small unit/integration test asserting single execution count.

Migration

No action required for end users or plugin authors.

Risk Assessment

Low. The removed code path was redundant; the retained path is the canonical lifecycle hook.

Changelog Entry

Playwright Helper: Remove redundant deferred _init() invocation to prevent duplicate initialization and potential race conditions.


Applicable helpers:

  • Playwright
  • Puppeteer
  • WebDriver
  • REST
  • FileHelper
  • Appium
  • TestCafe

Applicable plugins:

  • allure
  • autoDelay
  • autoLogin
  • customLocator
  • pauseOnFail
  • coverage
  • retryFailedStep
  • screenshotOnFail
  • selenoid
  • stepByStepReport
  • stepTimeout
  • wdio
  • subtitles

Type of change

  • 🔥 Breaking changes
  • 🚀 New functionality
  • 🐛 Bug fix
  • 🧹 Chore
  • 📋 Documentation changes/updates
  • ♨️ Hot fix
  • 🔨 Markdown files fix - not related to source code
  • 💅 Polish code

Checklist:

  • Tests have been added
  • Documentation has been added (Run npm run docs)
  • Lint checking (Run npm run lint)
  • Local tests are passed (Run npm test)

@Samuel-StO
Copy link
Contributor Author

All users running CodeceptJS 3.7.5 with the Playwright helper may see noisy log warnings like:
selectors.register: "__value" selector engine has been already registered

This comes from the helper invoking _init() twice (constructor + container lifecycle). It is harmless (registration is idempotent in practice) but confusing and risks future race conditions if more logic is added. This PR removes the redundant deferred init so only one registration path remains. A quick release would prevent confusion in newly upgraded projects.

@kobenguyent kobenguyent requested a review from Copilot September 23, 2025 13:37
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Removes a redundant asynchronous initialization call from the Playwright helper constructor that was causing duplicate execution of the _init() method. The initialization is already properly handled by the CodeceptJS container lifecycle, making the deferred constructor call unnecessary and potentially problematic.

  • Eliminates duplicate _init() method executions that occurred during framework bootstrap
  • Removes race condition potential between container-managed and constructor-initiated initialization
  • Simplifies initialization flow to rely solely on the standard CodeceptJS container lifecycle

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@Samuel-StO
Copy link
Contributor Author

The fails seems flaky and unrelated to my PR 🤔

@kobenguyent kobenguyent merged commit 6e9ae95 into codeceptjs:3.x Sep 24, 2025
16 of 18 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