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

PATCH method doesn't work with fetch #51336

Closed
fxdave opened this issue Jan 2, 2024 · 9 comments
Closed

PATCH method doesn't work with fetch #51336

fxdave opened this issue Jan 2, 2024 · 9 comments

Comments

@fxdave
Copy link

fxdave commented Jan 2, 2024

Version

v21.5.0

Platform

Linux dbiro-pc 6.6.7-arch1-1 #1 SMP PREEMPT_DYNAMIC Thu, 14 Dec 2023 03:45:42 +0000 x86_64 GNU/Linux

Subsystem

No response

What steps will reproduce the bug?

  1. Create a server with an endpoint that responds with a json
  2. Create a client with this:
fetch("http://localhost:8080/endpoint",{
  body: '{"segments":["put"],"argument":{}}',
  method: 'put',
  headers: { 'Content-Type': 'application/json', Accept: 'application/json' }
})
  1. The response.json() will throw an error because the body is empty

How often does it reproduce? Is there a required condition?

always

What is the expected behavior? Why is that the expected behavior?

It should get the json

What do you see instead?

It returns Bad Request error and the body is empty

Additional information

GET, DELETE, POST, PUT work. Only PATCH has this problem. Using the node-fetch instead of the built-in version solves the problem.

@aduh95
Copy link
Contributor

aduh95 commented Jan 2, 2024

  1. Create a server with an endpoint that responds with a json

I'd be helpful if your code snippet could include one, so we can run a repro and confirm the issue.

@fxdave
Copy link
Author

fxdave commented Jan 2, 2024

@aduh95
I created a branch that uses nodejs's fetch.

git clone https://github.com/fxdave/cuple.git
cd cuple
git checkout use-nodejs-fetch
npm i
npm run test

@aduh95
Copy link
Contributor

aduh95 commented Jan 2, 2024

A minimal repro should not include downloading any code from the internet, so no GitHub repo, no npm deps. Here's what I can do from the issue description:

require("node:http")
  .createServer((_, res) => {
    res.setHeader("Content-Type", "application/json");
    res.end("{}");
  })
  .unref()
  .listen(8080, () => {
    fetch("http://localhost:8080/endpoint", {
      body: '{"segments":["put"],"argument":{}}',
      method: "put",
      headers: {
        "Content-Type": "application/json",
        Accept: "application/json",
      },
    }).then(res => res.json()).then(console.log, console.error);
  });

Running the above code, I don't see any error.

@fxdave
Copy link
Author

fxdave commented Jan 2, 2024

@aduh95 Thanks for the example! Please replace put with patch and you will see the error:
image

@aduh95
Copy link
Contributor

aduh95 commented Jan 2, 2024

FWIW trying with the following code:

require("node:http")
  .createServer((req, res) => {
    res.setHeader('Access-Control-Allow-Origin', '*');
    res.setHeader('Access-Control-Allow-Methods', '*');
    res.setHeader('Access-Control-Allow-Headers', '*');
    if (req.method === 'OPTIONS') {
      res.writeHead(204);
      res.end()
      return
    }
    res.setHeader("Content-Type", "application/json");
    res.end("{}");
  })
  .listen(8080, () => {
    fetch("http://localhost:8080/endpoint", {
      body: '{"segments":["put"],"argument":{}}',
      method: "patch",
      headers: {
        "Content-Type": "application/json",
        Accept: "application/json",
      },
    }).then(res => res.json()).then(console.log, console.error);
  });                                              

Then visiting about:blank and pasting the fetch call from the browser DevTools console fails on Chromium and Firefox, but works on Safari. Maybe it's an undefined behavior in the HTML spec? /cc @nodejs/undici

@KhafraDev
Copy link
Member

patch isn't normalized to PATCH as per the spec. Replacing it fixes the issue.

@KhafraDev
Copy link
Member

@fxdave
Copy link
Author

fxdave commented Jan 2, 2024

Oh interesting! Thanks @aduh95, @KhafraDev !

@Aissaoui-Saber
Copy link

try to uppercase patch to PATCH

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

No branches or pull requests

4 participants