Repository navigation
[wrangler] Fail with an error instead of hanging under Bun - #15846
petebacondarwin merged 15 commits into
Conversation
ðĶ Changeset detectedLatest commit: 617272b The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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 |
@cloudflare/autoconfig
@cloudflare/build-output-utils
@cloudflare/codemods
@cloudflare/config
@cloudflare/containers-shared
create-cloudflare
@cloudflare/deploy-helpers
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-functions
@cloudflare/pages-shared
@cloudflare/runtime-types
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-plugin
@cloudflare/workers-auth
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
commit: |
|
#15906 changed this PR's premise. Under bun, |
|
Codeowners approval required for this PR:
Show detailed file reviewers
|
âĶ doors The guard in cloudflare#15845 ran inside listen(), so under Bun it also refused server.getWorker().fetch(), which never goes through the ProxyWorker and worked before. Move the check to server.fetch(), and have the ProxyWorker answer with a 503 once a control request arrives without its payload, so requests to the listen() URL fail loudly instead of queueing forever.
âĶer the harness under simulated Bun A request already forwarded to the UserWorker sits in neither queue when an empty control request drains them, and its retry is requeued asynchronously once the old connection fails. Route both requeue sites through requeueForRetry(), which answers with the same 503 while controlPayloadMissing is set instead of waiting for a play that cannot come. Add a createTestHarness() test that simulates Bun under Node (isBun() true, proxy control requests stripped of cf) and asserts listen() and getWorker().fetch() work while server.fetch() throws and the listen() URL answers 503.
âĶ15906 routes getWorker() through the dispatcher too
f874268 to
4712825
Compare
âĶorkflows-shared Both are HeadersTimeoutError / waitUntil timeouts on the Windows runners. The same fixture ran 17/17 on Windows at this branch's previous head, and an unrelated PR failed it with the same HeadersTimeoutError.
|
Codeowners approval required for this PR:
Show detailed file reviewers |
|
This pull request was not reviewed automatically because its author does not have write access to this repository. A maintainer with write access can start a review by commenting |
âĶy after updateBindings DevEnv only logs a failed `play` send and never re-emits it, so an error event that arrives while startup waits for `play` is a separate failure (authentication, the remote preview). Startup now reports it as itself even under Bun, and keeps the Bun-specific error for a `play` that was not delivered. ProxyController records that send failure in `latestSendError`, since the event bus never carried it; the test mock emitted it as an error event, which the real DevEnv does not, so the mock now matches DevEnv. updateBindings now waits for the new `play` to be acknowledged, as startup does, instead of only for the message mutex to drain. The changeset lists the Vite and Vitest plugins, which bundle the private @cloudflare/remote-bindings package.
Wrangler's rule is undici's fetch, never the global one. Wrangler bundles undici, so its fetch and dispatcher work under Bun (only Miniflare's runtime `undici` import resolves to Bun's dispatcher-less shim). Measured on Bun 1.4.4 with undici 7.29.1 inlined by esbuild: an Agent with rejectUnauthorized false reaches a self-signed HTTPS server (204), and the same request without it fails with DEPTH_ZERO_SELF_SIGNED_CERT.
âĶcribes an older message A non-204 answer, or a ProxyWorker that was not ready, returned false without touching latestSendError, so a failure left over from an earlier message was reported as the cause of a later rejected `play`. Each send now sets latestSendError to its own outcome: cleared on a 204, a status error on any other answer.
petebacondarwin
left a comment
There was a problem hiding this comment.
Please can you clean up the PR description here?
We don't need to know the history of changes to the PR. Instead a short clear description of what this PR achieves and why.
In general we don't land PRs to improve Bun compatibility - any compatibility issue is deemed on Bun to fix.
In this case it appears that yo are just trying to provide better error messages when Bun fails. We could consider this although as it stands the PR is quite complicated and are a loathe to add such logic just for Bun.
Each Bun failure now gets one check at its entry point: the harness's dispatchFetch() helper throws a UserError, the ProxyWorker answers with a 503 when ProxyController passes it the Bun message as a binding, and startRemoteProxySession() throws a UserError. This drops the play-delivery tracking in both ProxyController copies, the ProxyWorker's missing-payload state, the undici fallback request and the harness timeout.
|
Ah for sure, so sorry about the wordiness on the description before - I'd assumed it'd gotten a bit lost as the PR morphed. Also totally understand with regards to not going out of our way to write this repo around Bun! Iâve rewritten the description and cut the change to error messages only: one isBun() check (Darioâs helper from #15845) wherever a request would otherwise hang. The diff went from 963 lines across 16 files to 177 across 10 (617272b). Happy to drop the remote-bindings check too if youâd rather keep this to the test harness! |
workers-devprod
left a comment
There was a problem hiding this comment.
Codeowners reviews satisfied
## pnpm-workspace.yaml (default) ## Dependency Updates | Package | From | To | Type | | --- | --- | --- | --- | | `@cloudflare/vitest-plugin` | 1.4.5 | 1.4.6 | patch | | `@cloudflare/workers-types` | 5.20261019.1 | 5.20261021.1 | minor | | `miniflare` | 5.20261019.0-beta | 5.20261019.1-beta | prerelease | | `wrangler` | 4.151.2 | 4.151.3 | patch | ## Release Notes <details> <summary><b>@<!---->cloudflare/vitest-plugin</b> (1.4.5 â 1.4.6)</summary> ### Patch Changes - [#15708](cloudflare/workers-sdk#15708) [`3464abc`](cloudflare/workers-sdk@3464abc) Thanks [@<!---->samstarling](https://github.com/samstarling)! - Dispose Workflow step results and introspection modifiers after use Release RPC resources deterministically during local Workflow execution and introspection. This prevents undisposed RPC warnings and requests being cancelled after their execution context has ended. Live step results retain their original data shape, including typed-array offsets and backing buffers. Callbacks that finish after a step times out also release their results, cancelling unused streams without delaying retries. Disposing an introspector still aborts the instance if modifier acquisition failed, without introducing a teardown error. Non-stream results containing function-valued array properties are rejected according to the serialisable-output contract, instead of silently discarding those properties during storage normalisation. - [#15846](cloudflare/workers-sdk#15846) [`a88cac4`](cloudflare/workers-sdk@a88cac4) Thanks [@<!---->oddharsh](https://github.com/oddharsh)! - Fail with an error instead of hanging when local dev requests can't work under Bun Bun's `fetch()` ignores the undici `dispatcher` that Miniflare routes requests through (oven-sh/bun#39247). Under Bun, `createTestHarness()`'s `server.fetch()` and `server.getWorker()`'s `fetch()`, `email()` and `scheduled()` now throw a `UserError`, requests to the dev server URL get a 503 naming the cause, and starting remote bindings throws a `UserError`. Each of these used to hang with no error. `server.getWorker().getEnv()` and `getExport()` keep working under Bun. - Updated dependencies [[`4508172`](cloudflare/workers-sdk@4508172), [`61fc739`](https://gi âĶ[full notes](https://github.com/cloudflare/workers-sdk/releases/tag/%40cloudflare/vitest-plugin%401.4.6) </details> <details> <summary><b>miniflare</b> (5.20261019.0-beta â 5.20261019.1-beta)</summary> ### Patch Changes - [#15708](cloudflare/workers-sdk#15708) [`3464abc`](cloudflare/workers-sdk@3464abc) Thanks [@<!---->samstarling](https://github.com/samstarling)! - Dispose Workflow step results and introspection modifiers after use Release RPC resources deterministically during local Workflow execution and introspection. This prevents undisposed RPC warnings and requests being cancelled after their execution context has ended. Live step results retain their original data shape, including typed-array offsets and backing buffers. Callbacks that finish after a step times out also release their results, cancelling unused streams without delaying retries. Disposing an introspector still aborts the instance if modifier acquisition failed, without introducing a teardown error. Non-stream results containing function-valued array properties are rejected according to the serialisable-output contract, instead of silently discarding those properties during storage normalisation. </details> <details> <summary><b>wrangler</b> (4.151.2 â 4.151.3)</summary> ### Patch Changes - [#15611](cloudflare/workers-sdk#15611) [`4508172`](cloudflare/workers-sdk@4508172) Thanks [@<!---->taylorlee](https://github.com/taylorlee)! - Allow `wrangler dev` to start module Workers without a default export Previously, `wrangler dev` failed to start module Workers that exported only named entrypoints, such as RPC services, unless an unused default export was added. These Workers now start normally in local development without that workaround. - [#16370](cloudflare/workers-sdk#16370) [`61fc739`](cloudflare/workers-sdk@61fc739) Thanks [@<!---->WalshyDev](https://github.com/WalshyDev)! - Fetch the remote config diff from the configured compliance region When a Worker was last deployed from the dashboard, `wrangler deploy` and `wrangler versions upload` download its remote configuration to compare with the local one. These requests ignored the `compliance_region` field in the Wrangler configuration file and only honoured the `CLOUDFLARE_COMPLIANCE_REGION` environment variable. A FedRAMP High Worker configured through the config file would send them to `api.cloudflare.com` instead of `api.fed.cloudflare.com`, and the deploy would fail with authentication errors. They now use the same compliance region as the rest of the deploy. - [#15846](cloudflare/workers-sdk#15846) [`a88cac4`](cloudflare/workers-sdk@a88cac4) Thanks [@<!---->oddharsh](https://github.com/oddharsh)! - Fail with an error instead of hanging when local dev requests can't work under Bun Bun's `fetch()` ignores the undici `dispatcher` that Miniflare routes requests through (oven-sh/bun#39247). Under Bun, `createTestHarness()`'s `server.fetch()` and `server.getWorker()`'s `fetch()`, `email()` and `scheduled()` now throw a `UserError`, requests to th âĶ[full notes](https://github.com/cloudflare/workers-sdk/releases/tag/wrangler%404.151.3) </details> --- *This PR was auto-generated by [catalog-update-action](https://github.com/brandhaug/catalog-update-action).* Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Fixes #15717.
Under Bun, Wrangler's local dev requests hang with no error, because Bun's
fetch()ignores the undicidispatcherthat Miniflare routes them through (oven-sh/bun#39247). The real fix belongs in Bun. This PR adds oneisBun()check at each place a user would otherwise wait forever, and each check throws or responds with an error that names the Bun issue:createTestHarness(): thedispatchFetch()helper behindserver.fetch()andgetWorker()'sfetch(),email()andscheduled()throws aUserError.getEnv()andgetExport()don't dispatch, so they keep working.ProxyControllerpasses the error text to the ProxyWorker as a binding, and the ProxyWorker answers with a 503 instead of queueing the request.startRemoteProxySession()throws aUserErrorbefore starting the session.This builds on #15845 and includes its commit under its original author. #15845 fails in
listen(), which also blocksgetEnv().isBun(), and fails on a 200 without the ProxyWorker change. On Bun 1.4.4 with this branch built,getEnv()works, bothfetch()calls throw theUserError, and the dev server URL answers 503; Node is unchanged. Remote bindings were not run against a live session under Bun.server.fetch()JSDoc describes the Bun behaviourNote
This is a contribution from an AI agent: Claude Code, running Claude Opus 5.5. It was written on instructions from @oddharsh and under their supervision, and @oddharsh reviewed the change and this description before submission.