Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃悰 BUG: requests from workers to DOs (in miniflare) broken in wrangler 3.46+ #5658

Closed
jesseditson opened this issue Apr 19, 2024 · 4 comments 路 Fixed by #5669
Closed

馃悰 BUG: requests from workers to DOs (in miniflare) broken in wrangler 3.46+ #5658

jesseditson opened this issue Apr 19, 2024 · 4 comments 路 Fixed by #5669
Labels
bug Something that isn't working regression Break in existing functionality as a result of a recent change wrangler Relating to the Wrangler CLI tool

Comments

@jesseditson
Copy link

Which Cloudflare product(s) does this pertain to?

Wrangler core, Miniflare

What version(s) of the tool(s) are you using?

3.51.2 [wrangler]

What version of Node are you using?

v18.19.0

What operating system and version are you using?

Mac OS Sonoma 14.4.1

Describe the Bug

Expected behavior

The behavior I expect is:

  • I can bind a durable object that runs under one worker (worker A) to another worker (worker B)
  • From worker B, I can create a durable object stub and perform a request on worker A's DO.

Observed behavior

Instead, when I make a request from worker B, the request does not reach the DO class, and instead I see an exception:

SyntaxError: "undefined" is not valid JSON
    at async Object.fetch (file:///Users/jesseditson/Desktop/cloudflare-test/node_modules/miniflare/dist/src/workers/core/entry.worker.js:954:22)

Steps to reproduce

I've posted a minimal repro repo here: https://github.com/jesseditson/wrangler-do-error-repro

To set it up, clone the repo then run npm install.

To reproduce the issue, run (in two seperate terminals)

npm run do
npm run api

Then visit http://localhost:8777/test in a browser or run curl http://localhost:8777/test

Investigation

Looking at the actual entry.worker.js, it looks like some headers are auto-injected - when I log the request path and headers, the offending call has:

path: /__WRANGLER_EXTERNAL_DURABLE_OBJECTS_WORKER

Headers(6) {
  'accept-encoding' => 'br, gzip',
  'host' => 'localhost:8787',
  'x-miniflare-durable-object-cf-blob' => 'undefined',
  'x-miniflare-durable-object-id' => '899153785ebdad0ee7fe9208aae7ed48307ff327588dddf7e767a75daee36277',
  'x-miniflare-durable-object-name' => 'TestDO',
  'x-miniflare-durable-object-url' => 'http://do/hello',
  [immutable]: false
}

The x-miniflare-durable-object-cf-blob appears to be the issue, and sure enough when I add a line before the request:

request.headers.delete("x-miniflare-durable-object-cf-blob")

things work as expected.

Resolution

I've verified that this is a regression by running npm i --save-dev wrangler@3.45 in the posted repro, which then no longer reproduces this error.

I have not yet looked into the patches merged between .45 and .51, but it also seems possible that the error exists outside this repo, as I can't grep any reads of the x-miniflare-durable-object-cf-blob header.

Please provide a link to a minimal reproduction

https://github.com/jesseditson/wrangler-do-error-repro

Please provide any relevant error logs

No response

@jesseditson jesseditson added the bug Something that isn't working label Apr 19, 2024
@jesseditson jesseditson changed the title 馃悰 BUG: requests from workers to DOs (in miniflare) broken in wrangler 3.50+ 馃悰 BUG: requests from workers to DOs (in miniflare) broken in wrangler 3.46+ Apr 19, 2024
@jesseditson
Copy link
Author

jesseditson commented Apr 19, 2024

I've further narrowed this to 3.46.0 and verified that 3.45.0 does not reproduce the issue.

@jesseditson
Copy link
Author

Looking at the release diff, there's really only one related change that is mentioned both under patch and minor: #5215

This seems a likely candidate but unfortunately is quite large. At first blush the changes to cfBlobHeader look relevant. Seems probable that the header was previously unset when cf was undefined and is now set to the string "undefined", or something along those lines.

@dario-piotrowicz
Copy link
Member

dario-piotrowicz commented Apr 20, 2024

Thanks for the issue @jesseditson 馃槃 (and thanks a lot for the high quality minimal reproduction!!! that's always super helpful and appreciated! 鉂わ笍)

This is something that I've already noticed in the context of getPlatformProxy: #5620

I've just opened #5669 to address this, I've tried it with your minimal reproduction and it does seem to be fixing the issue, but if you want please feel free to also try the PR's prerelease and let me know if it works for you 馃檪

@dario-piotrowicz dario-piotrowicz added wrangler Relating to the Wrangler CLI tool regression Break in existing functionality as a result of a recent change labels Apr 20, 2024
@jesseditson
Copy link
Author

Awesome, thanks for your work here! Appreciate you and the CF team!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something that isn't working regression Break in existing functionality as a result of a recent change wrangler Relating to the Wrangler CLI tool
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants