fix(browser): wait for orchestrator readiness before resolving browser sessions

[soconnor-seeq]

Description

I diagnosed an intermittent failure in our CI runs (about one in five) to a timing issue in vitest. The tests would not fail — vitest would hang until the CI job was killed after a while.

Resolves (do you need an issue for a bug fix?)

I figured out (with the help of Codex) hanging behaviour was caused when the WS upgrade happened (so the connected() event was triggered on the session), but the orchestrator wasn't created and assigned to getBrowserState().orchestrator yet.

The fix was to introduce a "ready" event in the IframeOrchestrator's constructor to signal that the websocket connection is ready. In the session only after both the ready + connected events are seen does the promise returned from createSession resolve.

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

• It's really useful if your PR references an issue where it is discussed ahead of time. If the feature is substantial or introduces breaking changes without a discussion, PR might be closed.
• Ideally, include a test that fails without this PR but passes with it.
• Please, don't make changes to pnpm-lock.yaml unless you introduce a new test example.
• Please check Allow edits by maintainers to make review process faster. Note that this option is not available for repositories that are owned by Github organizations.

Tests

• Run the tests with pnpm test:ci.

Documentation

• If you introduce new functionality, document it. You can run documentation with pnpm run docs command.

Changesets

• Changes in changelog are generated from PR name. Please, make sure that it explains your changes in an understandable manner. Please, prefix changeset messages with feat:, fix:, perf:, docs:, or chore:.

AI assisted: Codex

[soconnor-seeq]

Notes:

Ideally, include a test that fails without this PR but passes with it.

I can't (or couldn't think of how to) test this bug as it is a fairly specific timing issue. It would require somehow mocking Playwright, etc so that the connected event fired before / after the orchestrator being created + assigned to getBrowserState().orchestrator. I added unit test that simulate the behaviour in the session code.

It's really useful if your PR references an issue where it is discussed ahead of time. If the feature is substantial or introduces breaking changes without a discussion, PR might be closed.

Because this is just a bug fix is an issue needed?

---

I was also able to reproduce this (intermittently) on my local machine by using Linux's ulimit -n 302 && pnpm test (that 302 is probably very repo + OS specific mind you). No other resource pressure seemed to work to re-create it: nice, ionice, tasket, 10 concurrent runs, cgroup with CPU and memory limits, etc.

[sheremet-va]

Did you try the fixed version? Does it work for you?

[pkg-pr-new]

@vitest/browser

npm i https://pkg.pr.new/@vitest/browser@10397

@vitest/browser-playwright

npm i https://pkg.pr.new/@vitest/browser-playwright@10397

@vitest/browser-preview

npm i https://pkg.pr.new/@vitest/browser-preview@10397

@vitest/browser-webdriverio

npm i https://pkg.pr.new/@vitest/browser-webdriverio@10397

@vitest/coverage-istanbul

npm i https://pkg.pr.new/@vitest/coverage-istanbul@10397

@vitest/coverage-v8

npm i https://pkg.pr.new/@vitest/coverage-v8@10397

@vitest/expect

npm i https://pkg.pr.new/@vitest/expect@10397

@vitest/mocker

npm i https://pkg.pr.new/@vitest/mocker@10397

@vitest/pretty-format

npm i https://pkg.pr.new/@vitest/pretty-format@10397

@vitest/runner

npm i https://pkg.pr.new/@vitest/runner@10397

@vitest/snapshot

npm i https://pkg.pr.new/@vitest/snapshot@10397

@vitest/spy

npm i https://pkg.pr.new/@vitest/spy@10397

@vitest/ui

npm i https://pkg.pr.new/@vitest/ui@10397

@vitest/utils

npm i https://pkg.pr.new/@vitest/utils@10397

vitest

npm i https://pkg.pr.new/vitest@10397

@vitest/web-worker

npm i https://pkg.pr.new/@vitest/web-worker@10397

commit: 79b23eb

[netlify]

✅ Deploy Preview for vitest-dev ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	79b23eb
🔍 Latest deploy log	https://app.netlify.com/projects/vitest-dev/deploys/6a188db5b553bc000867f3f6
😎 Deploy Preview	https://deploy-preview-10397--vitest-dev.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[soconnor-seeq]

Did you try the fixed version? Does it work for you?

If by "try the fixed version" you mean the latest beta release of v5.0.0-beta.3 then yes I did finally get around to testing that beta version.

I tried upgrading to v5.0.0-beta.3 but it still hangs. It looks like the source of the hang is different then what this fix is solving though. It's likely caused by this sequence of events:

1. orchestrator.ts sends prepare as soon as iframe.onload fires
2. tester.ts only starts listening for BroadcastChannel messages after its module finishes evaluating
3. iframe.onload can happen before that listener exists. So the first prepare message can be lost then the orchestrator waits forever for response:prepare.

I'm going to create a new PR for the above race condition = hanging vitest bug I found. But I still think there this PR's fix for a different race condition could still surface again w/o this it also being fixed.

[sheremet-va]

I meant try the fix that you implemented here. It is obviously not merged, so testing a beta version is meaningless for your use case. You can use the link from the pkg.pr.new or manually applying a fix

[soconnor-seeq]

I meant try the fix that you implemented here.

Ah gotcha — now I see what you are saying. I didn't know that there were built versions per-PR for testing.

Originally I noticed this bug (CI hanging w/ vitest) using v4.1.2. I then figured out where the bug was by altering the v4.1.2 version locally by adding logging and re-running CI till a hanging-failure surfaced again.

After figuring out where in code the hanging was coming from I made these changes on the v5 branch — built it locally — and tested that fixed-with-these-changes vitest in CI and it passed†

† but sometimes it did fail but I then narrowed it down to another hanging scenario I detailed in my comment above.