Not work with Node.js v21 when using https in dev server

hronro commented 9 months ago

Describe the bug

The dev server always responds "Invalid request body"

The original issue can be found at https://github.com/sveltejs/kit/issues/11213.

I believe the problem lies in the incompatibility between the vite dev server and HTTP/2, which is the default protocol when using HTTPS.

While many people only use HTTP on their dev servers to avoid this issue, we have specific cookie strategies that require both our dev server and production server to use HTTPS.

UPDATE: I recently upgraded to SvelteKit 2 (which is using Vite 5) and Node.js v21.5.0, the issue persists with a different error message:

TypeError: Headers.append: ":method" is an invalid header name.
    at webidl.errors.exception (node:internal/deps/undici/undici:1636:14)
    at webidl.errors.invalidArgument (node:internal/deps/undici/undici:1647:28)
    at appendHeader (node:internal/deps/undici/undici:2053:29)
    at fill (node:internal/deps/undici/undici:2039:11)
    at new Request (node:internal/deps/undici/undici:6151:13)

Reproduction

https://github.com/hronro/vite-https-issue

Steps to reproduce

Run pnpm i
Run pnpm dev
Open the page in browser

System Info

System:
    OS: Linux 6.6 Arch Linux
    CPU: (14) x64 AMD Ryzen 7 5800X 8-Core Processor
    Memory: 26.50 GB / 27.41 GB
    Container: Yes
    Shell: 3.7.0 - /usr/bin/fish
  Binaries:
    Node: 21.5.0 - /usr/bin/node
    Yarn: 1.22.21 - /usr/bin/yarn
    npm: 10.2.5 - /usr/bin/npm
    pnpm: 8.13.1 - /usr/bin/pnpm
  npmPackages:
    vite: ^5.0.10 => 5.0.10

Used Package Manager

pnpm

Logs

No response

Validations

[X] Follow our Code of Conduct
[X] Read the Contributing Guidelines.
[X] Read the docs.
[X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
[X] Make sure this is a Vite issue and not a framework-specific issue. For example, if it's a Vue SFC related bug, it should likely be reported to vuejs/core instead.
[X] Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
[X] The provided reproduction is a minimal reproducible example of the bug.

XiSenao commented 9 months ago

I think this is a way to be compatible with Request. https://github.com/sveltejs/kit/pull/3572

hronro commented 9 months ago

@XiSenao Could you please explain more details about this? The PR you linked was merged 2 years ago.

XiSenao commented 9 months ago

I think it may be related to the changes in this logic(node 21.4.0).

function isValidHTTPToken (characters) {
  if (characters.length === 0) {
    return false
  }
  for (let i = 0; i < characters.length; ++i) {
    if (!isTokenCharCode(characters.charCodeAt(i))) {
      return false
    }
  }
  return true
}

https://github.com/nodejs/node/blob/68c8472ed9edbf1d3fbb1e46cb65b678ee52312c/deps/undici/src/lib/fetch/util.js#L140-L150 https://datatracker.ietf.org/doc/html/rfc7230#page-27

It seems that Request is not compatible with some features of H2. Configuring proxy for Vite can downgrade to HTTP/1.1 and also avoid the occurrence of problems. ref https://github.com/vitejs/vite/commit/02cc24f5e7c3219eb0dd77480f9df8208e81c09c I don't know if this issue is important enough to require Vite to be compatible with it. @hronro

bluwy commented 9 months ago

@XiSenao do you know where we could fix to fix the issue? I'm slightly inclined to close this as there had always been issues with odd-numbered Node.js versions and often it's a Node.js bug instead.

XiSenao commented 9 months ago

I missed the notification for this issue😂. Yes, I think Node is still in an unstable stage, and I may not be inclined to be compatible with unstable versions of Node. @bluwy

hi-ogawa commented 9 months ago

FYI, Remix also had a similar story as sveltekit when adapting Vite's http2:

https://github.com/remix-run/remix/issues/7867

Initially I suggested to strip out pseudo headers (:method, etc...) when converting Node's IncomingMessage class to Web Request class:

https://github.com/remix-run/remix/pull/8049

which I thought is necessary since Remix's Request/Response related polyfill (https://github.com/remix-run/web-std-io) didn't accept such headers at that time (in sveltekit case, I guess it was node-fetch polyfill).

Since Remix's polyfill is opt-in, when disabling such polyfill, it's actually working since Node 18/20's global Request (which is essentially undici's Request) was accepting pseudo headers. So, eventually Remix team decided to follow that behavior in their polyfill as well:

https://github.com/remix-run/web-std-io/pull/55

Apparently, Undici "fixed" this behavior in response to this issue and Node 21 is currently rejecting http2 pseudo header:

https://github.com/nodejs/undici/issues/2409

Regardless of whether this strict behavior would land in stable node or not, personally I feel this is a framework concern than Vite itself since framework is the one converting from Node-style IncomingMessage class to Web/fetch-style Request class (also deciding whether polyfill Request or not).

Btw, I made a little script to test node's http2 here https://github.com/hi-ogawa/repro-http2-pseudo-headers/blob/main/repro.mjs

vitejs / vite