[wrangler] Extract remote bindings into a package#14720
Conversation
🦋 Changeset detectedLatest commit: 79808aa 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 |
|
✅ All changesets look good |
|
I'm Bonk, and I've done a quick review of your PR. Extracts the remote bindings implementation into a private One actionable issue: 1. Unused import will fail lint — This PR removes the import { isNonInteractiveOrCI } from "@cloudflare/workers-utils";
(applied to lines 9-10, dropping the unused import and keeping the Nit (non-blocking): |
|
Posted the review as a top-level PR comment on #14720 (the inline suggestion returned a 422 because the unused-import line falls outside the diff hunk, so I fell back to a PR comment per the instructions). Summary of findings:
Everything else (the logger singleton init timing, |
55ca99a to
235afa1
Compare
|
Codeowners approval required for this PR:
Show detailed file reviewers |
@cloudflare/autoconfig
@cloudflare/config
create-cloudflare
@cloudflare/deploy-helpers
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-auth
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
commit: |
What is the goal here? It feels a bit weird to be making these copies (even if they are narrower). This is non-trivial code and any bug in one is likely to be in the other, making an additional maintenance burden. Was there no way to just share the code? |
I initially tried that, but pulling the entirety of DevEnv outside of Wrangler proved quite complex, and ended up with similarly hard to review diffs. The goal here is to pull out out remote bindings as a distinct thing from DevEnv, so that we can use remote bindings in Vite and Vitest without depending on Wrangler. One aspect here is that I don't know that these codebases will necessarily evolve in the same direction. We may end up with remote bindings being powered by some more dedicated system in the future, and we may end up with the remote aspect of DevEnv being deprecated at some point. |
workers-devprod
left a comment
There was a problem hiding this comment.
Codeowners reviews satisfied
…oller.ts Co-authored-by: James Opstad <13586373+jamesopstad@users.noreply.github.com>
- Drop the empty `WranglerLog` wrapper — `ProxyControllerLogger` now extends Miniflare's `Log` directly - De-wrangler-ify the extracted package — `ProxyController` log prefix and the tail WebSocket `User-Agent` now say `remote-bindings` rather than `wrangler` - Move runtime dependencies out of `devDependencies` into `dependencies`, matching the convention in other packages
138abc1 to
79808aa
Compare
…artup `startRemoteProxySession` only unwrapped `RemoteSessionAuthenticationError` at the error event's direct `cause`, so a `UserError` surfaced asynchronously (e.g. the Miniflare user-error case) fell through to being wrapped in a generic `Error`. That made Wrangler treat it as an unexpected internal error (bug prompt + Sentry report) rather than a clean user-facing error. Surface a directly-emitted `UserError` verbatim, and walk the error event's cause chain for a `RemoteSessionAuthenticationError` (mirroring the original `findRemoteSessionAuthError`). Generic API errors — which also extend `UserError` via `APIError` — still fall through to the "Failed to start the remote proxy session" wrapper. Adds regression tests covering each case.
79808aa to
2b3ea8b
Compare
Extract the remote bindings implementation from Wrangler into the private
@cloudflare/remote-bindingspackage while preserving Wrangler's public API through logger-injecting wrappers.This also narrows the copied development runtime to the functionality remote bindings use, makes the package directly importable as ESM, registers a workers.dev subdomain when required for preview sessions, and retains existing proxy sessions when bindings are updated.
A picture of a cute animal (not mandatory, but encouraged)