Skip to content

fix(miniflare): Create WebSocketPair after fetching target WebSocket#13737

Merged
petebacondarwin merged 3 commits into
mainfrom
rfigueira/race-condition
May 1, 2026
Merged

fix(miniflare): Create WebSocketPair after fetching target WebSocket#13737
petebacondarwin merged 3 commits into
mainfrom
rfigueira/race-condition

Conversation

@ruifigueira

@ruifigueira ruifigueira commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

Move WebSocketPair creation and server.accept() to after the fetch call to ensure the target WebSocket is established before creating the proxy pair.

Extends and builds upon #13734


  • Tests
    • Tests included/updated
    • Automated tests not possible - manual testing has been completed as follows:
    • Additional testing not necessary because: it's already being tested
  • Public documentation
    • Cloudflare docs PR(s):
    • Documentation not necessary because: it's a patch

A picture of a cute animal (not mandatory, but encouraged)


Open in Devin Review

@changeset-bot

changeset-bot Bot commented Apr 29, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 0f5e8d5

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-project-automation github-project-automation Bot moved this to Untriaged in workers-sdk Apr 29, 2026
@workers-devprod workers-devprod requested review from a team and penalosa and removed request for a team April 29, 2026 17:31
@workers-devprod

workers-devprod commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

Codeowners approval required for this PR:

  • ✅ @cloudflare/wrangler
Show detailed file reviewers

@github-actions

github-actions Bot commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

Changeset Review

File: .changeset/fix-miniflare-browser-rendering-windows-race.md

Review Checklist

Criterion Status
Version Type ✅ Patch is appropriate for bug fix
Changelog Quality ✅ Detailed description explaining the race condition, root cause, and fix
Markdown Headers ✅ No h1/h2/h3 headers
Analytics ✅ N/A - no analytics changes
Dependabot ✅ N/A - not a dependency update
Experimental Features ✅ N/A - bug fix for existing feature

Notes

  • Version type: Patch is correct for a bug fix addressing a race condition
  • Description quality: Excellent - clearly explains:
    • The specific problem (race condition on Windows when Chrome hasn't started accepting connections)
    • Root cause (returning endpoint before socket was ready)
    • The fix (probing /json/version with retry/backoff)
    • Additional safety net (retrying transient connection failures)

✅ All changesets look good

@ask-bonk

ask-bonk Bot commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

After thorough review of all the substantive changes:

  1. WebSocket race fix - Correct ordering fix, moving pair creation after fetch.
  2. Browser readiness probe - Sound exponential backoff with timeout.
  3. Fetch retry wrapper - Correct retry logic with proper error classification.
  4. Hyperdrive proxy skip - Valid optimization, IPv6 handled correctly.
  5. Hyperdrive proxy error handling - Properly fixes race during server.listen.
  6. Node.js version bump - Consistent across all packages and CI.
  7. Tests - Thorough coverage of new Hyperdrive behavior.

No logic bugs, security issues, backward compatibility violations, or incorrect API behavior found.

LGTM

github run

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

⚠️ 1 issue in files not directly in the diff

⚠️ AGENTS.md "Requirements" section not updated after Node.js minimum version bump (AGENTS.md:85)

The PR changes the minimum Node.js version from 20 to 22 across all package.json engines fields, the root volta config, bin/wrangler.js, and wrangler-banner.ts, but does not update AGENTS.md which still states Node.js >= 20 under Development Guidelines > Requirements (AGENTS.md:85). A developer or agent following AGENTS.md would attempt to use Node.js 20, only to be rejected by the engines constraint in package.json:57 ("node": ">=22.0.0").

View 5 additional findings in Devin Review.

Open in Devin Review

@pkg-pr-new

pkg-pr-new Bot commented Apr 29, 2026

Copy link
Copy Markdown
create-cloudflare

npm i https://pkg.pr.new/create-cloudflare@13737

@cloudflare/kv-asset-handler

npm i https://pkg.pr.new/@cloudflare/kv-asset-handler@13737

miniflare

npm i https://pkg.pr.new/miniflare@13737

@cloudflare/pages-shared

npm i https://pkg.pr.new/@cloudflare/pages-shared@13737

@cloudflare/unenv-preset

npm i https://pkg.pr.new/@cloudflare/unenv-preset@13737

@cloudflare/vite-plugin

npm i https://pkg.pr.new/@cloudflare/vite-plugin@13737

@cloudflare/vitest-pool-workers

npm i https://pkg.pr.new/@cloudflare/vitest-pool-workers@13737

@cloudflare/workers-editor-shared

npm i https://pkg.pr.new/@cloudflare/workers-editor-shared@13737

@cloudflare/workers-utils

npm i https://pkg.pr.new/@cloudflare/workers-utils@13737

wrangler

npm i https://pkg.pr.new/wrangler@13737

commit: 0f5e8d5

@ruifigueira ruifigueira force-pushed the rfigueira/race-condition branch 4 times, most recently from 1a88c28 to fd81c1d Compare April 29, 2026 20:30
@petebacondarwin petebacondarwin marked this pull request as draft April 29, 2026 20:47
@ruifigueira ruifigueira marked this pull request as ready for review April 29, 2026 21:35
@petebacondarwin petebacondarwin force-pushed the rfigueira/race-condition branch from fd81c1d to 807fc66 Compare May 1, 2026 08:46

@workers-devprod workers-devprod left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Codeowners reviews satisfied

@github-project-automation github-project-automation Bot moved this from Untriaged to Approved in workers-sdk May 1, 2026
petebacondarwin and others added 3 commits May 1, 2026 10:31
Wait for Chrome's DevTools port to actually accept connections before
returning from launchBrowser, and retry transient connection failures
when the binding worker reaches Chrome. Fixes flaky ConnectEx (#1225)
and WSARecv (#64) errors observed when running miniflare browser
rendering tests on Windows CI.
…in browser rendering

Move WebSocketPair creation and server.accept() to after the fetch call to ensure the target WebSocket is established before creating the proxy pair.
@petebacondarwin petebacondarwin force-pushed the rfigueira/race-condition branch from 807fc66 to 0f5e8d5 Compare May 1, 2026 09:31
@petebacondarwin petebacondarwin removed the request for review from penalosa May 1, 2026 09:34
@petebacondarwin petebacondarwin removed request for a team May 1, 2026 09:35
@petebacondarwin petebacondarwin merged commit bb27219 into main May 1, 2026
53 checks passed
@petebacondarwin petebacondarwin deleted the rfigueira/race-condition branch May 1, 2026 09:47
@github-project-automation github-project-automation Bot moved this from Approved to Done in workers-sdk May 1, 2026
@petebacondarwin petebacondarwin added ci-flake Applied to PRs addressing CI flakiness product:browser Relating to Cloudflare Browser run: https://developers.cloudflare.com/browser-run/ labels May 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-flake Applied to PRs addressing CI flakiness product:browser Relating to Cloudflare Browser run: https://developers.cloudflare.com/browser-run/

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants