ardatan / whatwg-node

Helper packages to create platform agnostic applications and libraries without worrying about the lack of WHATWG support in Node.js
MIT License
158 stars 31 forks source link

Issue on version 0.4.8 #52

Closed eavendano closed 2 years ago

eavendano commented 2 years ago

GET without body

Hello @ardatan! I'm checking the new version 0.4.8 on a Node server. When I did the update from 0.4.7 to 0.4.8 I got a new issue. I got the following code:

  const query = graphql.print(document);
  // console.info('Query: ', query);
  // console.info('Variables: ', variables);
  const fetchResult = await crossFetch.fetch(process.env.GRAPHQL_SERVER, {
    method: 'POST',
    headers: {
      'Content-Type': 'application/json',
      ...context?.headers
    },
    body: JSON.stringify({ query, variables })
  });
  return fetchResult.json();
};

But now when I run the server I have the message showing in my log:

\node_modules\undici\lib\fetch\request.js:437
      throw new TypeError('Request with GET/HEAD method cannot have body.')
            ^

TypeError: Request with GET/HEAD method cannot have body.

On my code I have not changed a single line of code and I just get that error. Here is the stack trace:

\node_modules\undici\lib\fetch\request.js:437:13
\node_modules\undici\lib\fetch\index.js:114:21
\node_modules\undici\index.js:90:22
\node_modules\cross-undici-fetch\dist\create-node-ponyfill.js:130:16
\node_modules\cross-undici-fetch\dist\create-node-ponyfill.js:125:18
server.js:39:40 <-- This is my code on the line: const fetchResult = await crossFetch.fetch(process.env.GRAPHQL_SERVER, 

Is this a bug or is there something I need to do after the upgrade?

Destreyf commented 2 years ago

I ran into a similar issue, this appears to be tied to the cross-undici-fetch package, but instead with graphql-tools.

@saihaj over at https://github.com/dotansimha/graphql-code-generator/issues/8012#issuecomment-1172754938 identified the dependency causing the problem.

You can modify your package.json to get around the issue, for yarn you can add the following: Copied from @tylermenezes here https://github.com/dotansimha/graphql-code-generator/issues/8012#issuecomment-1172725253

  "resolutions": {
    "undici": "5.5.1"
  }

For npm (requires npm 8.3 or higher) you can do the following, this is how I fixed it for the time being.

{
  ...
  "dependencies": ...
  "devDependencies": ...
  "overrides": {
    "undici": "5.5.1"
   }
}

Hopefully this helps you out as a workaround until undici is fixed.

eavendano commented 2 years ago

Awesome! It worked for me.

Destreyf commented 2 years ago

@eavendano I don't know that I would have closed this as the problem still exists and all my workaround does is downgrades the version for now. This will still affect everyone using this until they fix the dependency in this package or the upstream undici fixes it's issue.

SimenB commented 2 years ago

The issue is with cross-undici-fetch's usage of undici.

import { fetch } from 'cross-undici-fetch';

const res = await fetch('https://jsonplaceholder.typicode.com/posts', {
  method: 'POST',
  body: JSON.stringify({
    title: 'foo',
    body: 'bar',
    userId: 1,
  }),
  headers: {
    'Content-type': 'application/json; charset=UTF-8',
  },
});
console.log(await res.text());

This fails with the error from OP. Replace import { fetch } from 'cross-undici-fetch'; with import { fetch } from 'undici'; and it works fine.

ardatan commented 2 years ago

Thanks for the issue! We're on it right now!

ardatan commented 2 years ago

Please see https://github.com/dotansimha/graphql-code-generator/issues/8012#issuecomment-1173608427

eavendano commented 2 years ago

Great news! Thanks a log @ardatan! Amazing job my friend.

SimenB commented 2 years ago

Opened #53 unpinning again, btw