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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 BUG: does Miniflare.dispatchFetch handle redirect Response from worker script? #5018

Closed
hi-ogawa opened this issue Feb 15, 2024 · 2 comments · Fixed by #5191
Closed
Assignees
Labels
bug Something that isn't working

Comments

@hi-ogawa
Copy link

hi-ogawa commented Feb 15, 2024

Which Cloudflare product(s) does this pertain to?

Miniflare

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

miniflare@3.20240129.2

What version of Node are you using?

20.11.1

What operating system and version are you using?

Linux 6.7 Arch Linux

Describe the Bug

Observed behavior

It appears that redirect response (e.g. 302 status) causes Error: redirect count exceeded regardless of location header.

Expected behavior

Preferably it returns response from worker script as is with 30x status and location header, instead of internally undici resolving redirection. I think this is preferred if framework author wants to run their server code on Miniflare by proxying request via dispatchFetch.

Steps to reproduce

I added a reproduction in this repository https://github.com/hi-ogawa/reproductions/tree/main/miniflare-redirect Please let me know if it is concise enough.

(additional context)

I'm currently testing running ViteRuntime to run Vite framework server side code inside Workerd/Miniflare. Here a few relevant links:

It's mostly working, but I just noticed that react-router/remix's server side redirection isn't working and it appears that it's hitting this bug/limitation of Miniflare.dispatchFetch.

Please provide a link to a minimal reproduction

https://github.com/hi-ogawa/reproductions/tree/main/miniflare-redirect

Please provide any relevant error logs

I also wrote this in readme of the reproduction repo, but I'll copy-paste it here:

Reveal log
$ node main.js https://example.local/redirect
[workerd] request.url = https://example.local/redirect
[workerd] request.url = https://example.local/redirect
[workerd] request.url = https://example.local/redirect
[workerd] request.url = https://example.local/redirect
[workerd] request.url = https://example.local/redirect
[workerd] request.url = https://example.local/redirect
[workerd] request.url = https://example.local/redirect
[workerd] request.url = https://example.local/redirect
[workerd] request.url = https://example.local/redirect
[workerd] request.url = https://example.local/redirect
[workerd] request.url = https://example.local/redirect
[workerd] request.url = https://example.local/redirect
[workerd] request.url = https://example.local/redirect
[workerd] request.url = https://example.local/redirect
[workerd] request.url = https://example.local/redirect
[workerd] request.url = https://example.local/redirect
[workerd] request.url = https://example.local/redirect
[workerd] request.url = https://example.local/redirect
[workerd] request.url = https://example.local/redirect
[workerd] request.url = https://example.local/redirect
[workerd] request.url = https://example.local/redirect
/home/hiroshi/code/personal/reproductions/miniflare-redirect/node_modules/.pnpm/undici@5.28.3/node_modules/undici/index.js:112
        Error.captureStackTrace(err, this)
              ^

TypeError: fetch failed
    at fetch (/home/hiroshi/code/personal/reproductions/miniflare-redirect/node_modules/.pnpm/undici@5.28.3/node_modules/undici/index.js:112:15)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async fetch2 (/home/hiroshi/code/personal/reproductions/miniflare-redirect/node_modules/.pnpm/miniflare@3.20240129.2/node_modules/miniflare/dist/src/index.js:4128:20)
    at async Miniflare.dispatchFetch (/home/hiroshi/code/personal/reproductions/miniflare-redirect/node_modules/.pnpm/miniflare@3.20240129.2/node_modules/miniflare/dist/src/index.js:8617:22)
    at async file:///home/hiroshi/code/personal/reproductions/miniflare-redirect/main.js:22:18 {
  cause: Error: redirect count exceeded
      at makeNetworkError (/home/hiroshi/code/personal/reproductions/miniflare-redirect/node_modules/.pnpm/undici@5.28.3/node_modules/undici/lib/fetch/response.js:352:9)
      at httpRedirectFetch (/home/hiroshi/code/personal/reproductions/miniflare-redirect/node_modules/.pnpm/undici@5.28.3/node_modules/undici/lib/fetch/index.js:1141:28)
      at httpFetch (/home/hiroshi/code/personal/reproductions/miniflare-redirect/node_modules/.pnpm/undici@5.28.3/node_modules/undici/lib/fetch/index.js:1090:24)
      at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
      at async /home/hiroshi/code/personal/reproductions/miniflare-redirect/node_modules/.pnpm/undici@5.28.3/node_modules/undici/lib/fetch/index.js:600:16
      at async mainFetch (/home/hiroshi/code/personal/reproductions/miniflare-redirect/node_modules/.pnpm/undici@5.28.3/node_modules/undici/lib/fetch/index.js:584:16)
      at async httpFetch (/home/hiroshi/code/personal/reproductions/miniflare-redirect/node_modules/.pnpm/undici@5.28.3/node_modules/undici/lib/fetch/index.js:1090:18)
      at async /home/hiroshi/code/personal/reproductions/miniflare-redirect/node_modules/.pnpm/undici@5.28.3/node_modules/undici/lib/fetch/index.js:600:16
      at async mainFetch (/home/hiroshi/code/personal/reproductions/miniflare-redirect/node_modules/.pnpm/undici@5.28.3/node_modules/undici/lib/fetch/index.js:584:16)
      at async httpFetch (/home/hiroshi/code/personal/reproductions/miniflare-redirect/node_modules/.pnpm/undici@5.28.3/node_modules/undici/lib/fetch/index.js:1090:18)
}
@hi-ogawa hi-ogawa added the bug Something that isn't working label Feb 15, 2024
@github-project-automation github-project-automation bot moved this to Untriaged in workers-sdk Feb 15, 2024
@hi-ogawa
Copy link
Author

hi-ogawa commented Feb 15, 2024

With the help of @frandiox, I found that using dispatchFetch(url, { redirect: "manual" }) is an appropriate for my use case hi-ogawa/reproductions@8d2f9ee.

I'm happy to close this issue, but maybe this should be still considered a bug (or unfriendly error message) for the default { redirect: "follow" } case. I'll keep this opened for now, but feel free to triage the issue as you like. Thanks!

@mrbbot mrbbot self-assigned this Feb 16, 2024
@mrbbot mrbbot moved this from Untriaged to Backlog in workers-sdk Feb 16, 2024
@mrbbot
Copy link
Contributor

mrbbot commented Mar 7, 2024

Hey! 👋 Thanks for reporting this, and for the super-helpful reproduction. There were a couple issues here, which should be fixed by #5191. 🎉

@mrbbot mrbbot moved this from Backlog to In Review in workers-sdk Mar 7, 2024
@github-project-automation github-project-automation bot moved this from In Review to Done in workers-sdk Mar 8, 2024
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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants