[wrangler] Fix races in RemoteProxySession reload and remote tail WebSocket#13867
Open
petebacondarwin wants to merge 5 commits intomainfrom
Open
[wrangler] Fix races in RemoteProxySession reload and remote tail WebSocket#13867petebacondarwin wants to merge 5 commits intomainfrom
petebacondarwin wants to merge 5 commits intomainfrom
Conversation
DevEnv extends EventEmitter and routes events between controllers via its internal `dispatch()` method. Re-emit `reloadComplete` events on the EventEmitter so external callers (in the next commit, `RemoteProxySession.updateBindings`) can wait for a remote upload to finish. Other event types stay internal for now.
… to complete Previously `updateBindings` resolved as soon as `worker.patchConfig` dispatched the configUpdate event, long before the remote worker had been re-uploaded with the new bindings and the local proxy worker had unpaused. Callers issuing requests immediately after `updateBindings` could see flaky failures (typically "WebSocket connection failed" for JSRPC bindings like service bindings or dispatch namespaces) because the local proxy worker was still in its paused state during the reload window. `updateBindings` now subscribes to the `reloadComplete` event before calling `patchConfig`, then awaits either that event (success) or the DevEnv `error` event (non-recoverable failure). After reload it also drains the local proxy worker's runtime-message mutex so the "play" message that resumes the proxy has been delivered, mirroring what `worker.fetch` already does.
…estart The remote-runtime tail-logs WebSocket (`#activeTail` in `RemoteRuntimeController`) is constructed with `signal: this.#abortController.signal`. The `ws` package wires that signal to the underlying HTTP upgrade request via Node's `addAbortSignal()` helper, so when `onBundleStart` aborts the controller to cancel in-flight preview-session operations, the WebSocket's request is destroyed with `AbortError` and emits an `error` event. Only a `message` listener was attached at construction, so the `error` had no handler and propagated as an unhandled exception — visible in e2e runs as a trailing "Unhandled Errors / Uncaught Exception" in the test report. Attach an `error` listener at WebSocket construction that ignores errors (debug-logged), matching the safeguards already present on the `terminate` paths in `#previewToken` and `teardown()`. This covers the window between WebSocket construction and the next `terminate`, plus any transient network errors during normal operation.
🦋 Changeset detectedLatest commit: f1586ac 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 |
workers-devprod
requested review from
a team and
edmundhung
and removed request for
a team
Contributor
|
Codeowners approval required for this PR:
Show detailed file reviewers |
Contributor
|
✅ All changesets look good |
Contributor
|
UnknownError: ProviderInitError |
Contributor
|
@petebacondarwin Bonk workflow failed. Check the logs for details. View workflow run · To retry, trigger Bonk again. |
create-cloudflare
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
commit: |
penalosa
approved these changes
May 8, 2026
workers-devprod
approved these changes
May 8, 2026
Contributor
workers-devprod
left a comment
workers-devprod
left a comment
There was a problem hiding this comment.
Codeowners reviews satisfied
…ng events.once() Address PR feedback to use `node:events`'s `events.once()` helper, which resolves on the named event and rejects on `error`, instead of hand-rolling a Promise with cross-cleanup listeners. Same behavior, fewer lines, matches the existing convention used elsewhere in wrangler (`dev.ts`, `pages/dev.ts`, `api/dev.ts`, `ProxyController.ts`, `check/commands.ts`).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #13831
Cherry-picks four commits from
pbd/agent-memorythat fix underlying race conditions inwrangler dev's remote-binding infrastructure. These contribute to flakiness in theminiflare-remote-resources.test.tse2e suite (and to user-visible flakes inwrangler dev).Changes
RemoteProxySession.updateBindingsreload race — Previously resolved as soon asworker.patchConfigdispatched theconfigUpdateevent, long before the remote worker had been re-uploaded and the local proxy worker had unpaused. Callers issuing requests immediately afterwards saw flaky failures (typically "WebSocket connection failed" for JSRPC bindings like service bindings or dispatch namespaces).updateBindingsnow waits for the nextreloadCompleteevent and drains the local proxy worker's runtime-message mutex before returning. Requires a small foundational change to re-emitreloadCompleteevents externally onDevEnv.Unhandled
AbortErrorfrom remote tail WebSocket — The remote-runtime tail-logs WebSocket (#activeTailinRemoteRuntimeController) was constructed with the sameAbortSignalthatonBundleStartaborts to cancel in-flight preview-session operations. The abort destroyed the WebSocket's underlying upgrade request withAbortError, which had noerrorlistener attached and propagated as an unhandled exception — visible in e2e runs as a trailing "Unhandled Errors / Uncaught Exception" in the test report. We now attach anerrorlistener that ignores errors (debug-logged), matching the safeguards already present on theterminatepaths.The complementary test-side changes (per-test
updateBindings, transient 5xx retries) live onpbd/agent-memoryand will land separately.updateBindingschange is exercised by the pending test refactor onpbd/agent-memory; theAbortErrorsuppression eliminates an unhandled-exception report seen in e2e runs.wrangler dev's remote-bindings runtime with no public API changes.