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

next.359 - load fetch POST => Request with GET/HEAD method cannot have body #5383

Closed
jycouet opened this issue Jul 6, 2022 · 9 comments · Fixed by #5420 or #5422
Closed

next.359 - load fetch POST => Request with GET/HEAD method cannot have body #5383

jycouet opened this issue Jul 6, 2022 · 9 comments · Fixed by #5420 or #5422

Comments

@jycouet
Copy link
Contributor

jycouet commented Jul 6, 2022

Describe the bug

In a load function of a page, doing a fetch to http://localhost:3000/api will throw an error:

Request with GET/HEAD method cannot have body.

Here is the code:

    const resultPOST = await event.fetch("http://localhost:3000/api", {
      method: "POST",
      headers: {
        "Content-Type": "application/json",
      },
      body: JSON.stringify({
        hello: "World",
      }),
    });
    const jsonPOST = await resultPOST.json();

Info

I stripped down the reproduction to http://localhost:3000/api URL.
But my use case will be to target other APIs not only locals one like '/api'

Reproduction

Here is the stackblitz: https://stackblitz.com/fork/github/jycouet/sk-359-post
(fetch issue is happening, but the log is different than locally!)

Here is the minimal repo: https://github.com/jycouet/sk-359-post
With the few step in git from npx create to the issue.

Logs

Request with GET/HEAD method cannot have body.
TypeError: Request with GET/HEAD method cannot have body.
    at new Request (file:///home/jycouet/udev/tmp/sk-359-post/node_modules/@sveltejs/kit/dist/node/polyfills.js:9104:14)
    at Agent$1.fetch (file:///home/jycouet/udev/tmp/sk-359-post/node_modules/@sveltejs/kit/dist/node/polyfills.js:10280:22)
    at fetch (file:///home/jycouet/udev/tmp/sk-359-post/node_modules/@sveltejs/kit/dist/node/polyfills.js:12200:22)
    at Object.fetch (file:///home/jycouet/udev/tmp/sk-359-post/.svelte-kit/runtime/server/index.js:2293:51)
    at load (index.svelte:17:35)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async load_node (file:///home/jycouet/udev/tmp/sk-359-post/.svelte-kit/runtime/server/index.js:2394:12)
    at async respond$1 (file:///home/jycouet/udev/tmp/sk-359-post/.svelte-kit/runtime/server/index.js:2778:15)
    at async resolve (file:///home/jycouet/udev/tmp/sk-359-post/.svelte-kit/runtime/server/index.js:3291:11)

System Info

System:
    OS: Linux 5.4 Ubuntu 20.04.2 LTS (Focal Fossa)
    CPU: (8) x64 11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz
    Memory: 1.24 GB / 7.69 GB
    Container: Yes
    Shell: 5.0.17 - /bin/bash
  Binaries:
    Node: 16.15.1 - ~/.volta/tools/image/node/16.15.1/bin/node
    Yarn: 1.22.17 - ~/.volta/tools/image/yarn/1.22.17/bin/yarn
    npm: 8.3.0 - ~/.volta/tools/image/npm/8.3.0/bin/npm
  npmPackages:
    @sveltejs/adapter-auto: next => 1.0.0-next.53 
    @sveltejs/kit: 1.0.0-next.359 => 1.0.0-next.359 
    svelte: ^3.44.0 => 3.48.0 
    vite: 2.9.13 => 2.9.13

Severity

blocking an upgrade

Additional Information

It's a blocker for our Library Houdini that was working well before next.359.
We have an issue open here: HoudiniGraphql/houdini#374

@Nickersoft
Copy link

I can confirm I'm facing the same issue (urql-graphql/urql#2533). Did a little bit of digging, and it looks like #5117 introduced this bug, as the logic behind fetch requests differs slightly between node-fetch and unidici (see here and here).

I'm tempted to open an issue on unidici's side, but am not sure if there's anything SvelteKit is doing to trigger this error on its side. @Rich-Harris, any thoughts since you picked up the unidici work?

@GauBen
Copy link
Contributor

GauBen commented Jul 6, 2022

I thought it would be easy to patch in SvelteKit, but I was wrong.

I found a simple workaround for now:

If you're using Node < 16, you might try import fetch from 'node-fetch'; export const externalFetch = fetch; instead.

@jycouet
Copy link
Contributor Author

jycouet commented Jul 6, 2022

Thx for looking at it.

test 1

  • I moved to node v16.7.0
  • added export const externalFetch = fetch; in src/hooks.ts

Now, the all page is saying: Invalid request body (I don't arrive to my manual fetch)
(I don't know how to pass --experimental-fetch to the cmd vite dev)

test 2

  • I moved to node v18.4.0 (where fetch is not experimental anymore)
  • kept export const externalFetch = fetch; in src/hooks.ts
  • I'm still having Request with GET/HEAD method cannot have body.

info

Maybe it's better to wait the the undici release + integration in sveltekit then to debug workarounds.

Again thx, keep us posted 🙏

@GauBen
Copy link
Contributor

GauBen commented Jul 6, 2022

You can also try patching node_modules/@sveltejs/kit/assets/server/index.js:2304 with:

const external_request = new Request(requested, /** @type {RequestInit} */ (opts));
response = await options.hooks.externalFetch.call(null, external_request.url, external_request);

and restart the dev server.

@jycouet
Copy link
Contributor Author

jycouet commented Jul 6, 2022

That's working 👍

@jycouet
Copy link
Contributor Author

jycouet commented Jul 8, 2022

@GauBen, any idea when this fix will make it to SvelteKit?
It's still not working in .361

@GauBen
Copy link
Contributor

GauBen commented Jul 8, 2022

Unfortunately I'm not a maintainer of Svelkit nor Undici. I pinged the maintainer of Undici to incite them to release the patch to npm.

Once Undici is released, you won't need an update from SvelteKit for it to work. You could use yarn up 'undici' -R or a similar command to upgrade nested dependencies.

@GauBen
Copy link
Contributor

GauBen commented Jul 8, 2022

Well, that was fast nodejs/undici#1529 (comment)

@jycouet
Copy link
Contributor Author

jycouet commented Jul 8, 2022

Great news, thx 👌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants