fix: add duplex option when sending a body

[codebytere]

Resolves #570

Refs nodejs/node#46221.

As of Node.js v18.13.0, via an update to undici, request.duplex must be set if request.body is ReadableStream or Async Iterables. This PR therefore adds missing duplex option to fetch-wrapper as per spec

I'm not sure if it's possible to test for this since responses are mocked.

Behavior

Before the change?

fetch calls fail with the following type of error:

Uncaught TypeError: RequestInit: duplex option is required when sending a body.
    at new Request (node:internal/deps/undici/undici:7090:19)

After the change?

fetch calls should no longer fail.

Other information

I'm not sure if it's possible to test for this since responses are mocked.

---

Additional info

Pull request checklist

• Tests for the changes have been added (for bug fixes / features)
• Docs have been reviewed and added / updated if needed (for bug fixes / features)
• Added the appropriate label for the given change

Does this introduce a breaking change?

• Yes (Please add the Type: Breaking change label)
• No

Pull request type

Please add the corresponding label for change this PR introduces:

• Bugfix: Type: Bug
• Feature/model/API additions: Type: Feature
• Updates to docs or samples: Type: Documentation
• Dependencies/code cleanup: Type: Maintenance

---

[gr2m]

What is the node version where it starts failing? We should add it to the test matrix to avoid regressions

[gr2m]

we should add a comment about why the duplex option is added here, with a URL that holds more information. Will half always work, or are there different values that need to be set based on different types of request body?

[codebytere]

To my understanding, based on the spec, half should be appropriate to set for all cases in Node.js (it is the also only option available at the moment)

[codebytere]

Oops, I thought i added that to the PR body but i missed it! It was Node.js v18.13.0, via an update to undici

[gr2m]

Thank you so much Shelley!

And thanks a lot for all your great work in Electron and Beyond!

[gr2m]

It was Node.js v18.13.0

In that case our current CI should cover it. Node 18 currently loads 18.14.0

[gr2m]

Hmm do you have a test script that fails for you in Node 18.14?

I tried this code on the main branch

  await request("POST /repos/{owner}/{repo}/issues/{issue_number}/comments", {
    owner: "gr2m",
    repo: "sandbox",
    issue_number: 240,
    body: "Hello World",
    headers: {
      authorization: `token ${process.env.GITHUB_TOKEN}`,
    },
  });

And it worked

[codebytere]

@nickfloyd @gr2m do you know when we can expect a new version? We're having to patch around this locally: electron/electron#38250

[gr2m]

hey sorry we have automated releases, but this pull request was merged with an incorrect commit message. It was before we set commit messages to default to the pull request title. I'll have that sorted out in a moment

[github-actions]

🎉 This PR is included in version 6.2.4 🎉

The release is available on:

• GitHub release
• npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

[gr2m]

all set, very sorry for the delay 😞

[codebytere]

@gr2m no worries - thank you so much!