Skip to content

Debug remote binding issues#13028

Draft
edmundhung wants to merge 1 commit intomainfrom
edmundhung/http-batch-remote-bindings
Draft

Debug remote binding issues#13028
edmundhung wants to merge 1 commit intomainfrom
edmundhung/http-batch-remote-bindings

Conversation

@edmundhung
Copy link
Member

Fixes #[insert GH or internal issue link(s)].

Describe your change...


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

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

@changeset-bot
Copy link

changeset-bot bot commented Mar 23, 2026

⚠️ No Changeset found

Latest commit: 6f76e9d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

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

@github-project-automation github-project-automation bot moved this to Untriaged in workers-sdk Mar 23, 2026
@ask-bonk
Copy link
Contributor

ask-bonk bot commented Mar 23, 2026

I see. The makeFetch function uses the remoteProxyConnectionString directly (line 50) and puts MF-Binding in the headers, not the URL search params. Meanwhile makeRemoteProxyStub adds MF-Binding to the URL search params (line 65). The isJSRPCBinding check on the server only looks at URL search params, so the "raw" fetch path (which puts MF-Binding only in headers) will correctly NOT match isJSRPCBinding. The routing logic is sound.

Now let me verify there's no case where a raw fetch path could accidentally have MF-Binding in the query string. The makeFetch function on line 50 does fetch(remoteProxyConnectionString, req) — it uses the connection string directly as the URL without modifying search params. So the routing change is safe.

The changes look correct and straightforward:

  1. Switch from WebSocket-based RPC to HTTP batch-based RPC (newWebSocketRpcSessionnewHttpBatchRpcSession)
  2. Remove the protocol conversion to wss:/ws: since HTTP batch doesn't need WebSocket
  3. Simplify isJSRPCBinding to not require the Upgrade header check since HTTP batch doesn't use WebSocket upgrades
  4. Update the comment to reflect both transport options

The @ts-expect-error is a minor concern but it's already cast to unknown right after, making it a pragmatic workaround for the deep type instantiation issue.

LGTM

github run

@pkg-pr-new
Copy link

pkg-pr-new bot commented Mar 23, 2026

create-cloudflare

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

@cloudflare/kv-asset-handler

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

miniflare

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

@cloudflare/pages-shared

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

@cloudflare/unenv-preset

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

@cloudflare/vite-plugin

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

@cloudflare/vitest-pool-workers

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

@cloudflare/workers-editor-shared

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

wrangler

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

commit: 6f76e9d

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Untriaged

Development

Successfully merging this pull request may close these issues.

2 participants