undici icon indicating copy to clipboard operation
undici copied to clipboard

`fetch` support for HTTP/2 by default

Open mctrafik opened this issue 4 months ago • 14 comments

What is the problem this feature will solve?

This will get node's implementation of fetch closer to how it behaves in the browser.

What is the feature you are proposing to solve the problem?

Support protocol upgrade similar to how fetch-h2 package does. It's weird to try to use a third-party package that uses node's internal http2 package under the hood.

What alternatives have you considered?

I am now using fetch-h2 package, but much like got-fetch or node-fetch I fear it will fall out of spec and not evolve with the ecosystem.

mctrafik avatar Feb 12 '24 04:02 mctrafik

Duplicate of https://github.com/nodejs/undici/issues/399. AFAICT it has already been implemented.

aduh95 avatar Feb 12 '24 17:02 aduh95

@aduh95 Can you elaborate? Does nodeJs use undici? If yes, in what version is this supported? I'm using Node 20, which is latest LTE version and fetch there doesn't support H2.

mctrafik avatar Feb 13 '24 01:02 mctrafik

Does nodeJs use undici?

Correct (it is also maintained by us) https://github.com/nodejs/node/blob/109ab0a89cdeff5717a2a16edd27bafecc104cf6/doc/contributing/maintaining/maintaining-http.md#L54-L56

You can check what version of Undici your build is using in process.versions.undici.

I'm using Node 20, which is latest LTE version and fetch there doesn't support H2.

Any chance you could send a repro? So we can reopen this issue and transfer it to the Undici repo.

aduh95 avatar Feb 13 '24 08:02 aduh95

I don't know much about HTTP/2, but here's an example that works in a browser (do it from the website itself to avoid CORS issue), while it fails in Node.js:

fetch('https://api.sandbox.push.apple.com').then(r => r.text()).then(console.log)

It's not easy to find a public URL that only works with HTTP/2.

targos avatar Feb 13 '24 09:02 targos

Created a minimal repro:

import { createServer, constants } from 'node:http2';
import { once } from 'node:events';

const {
  HTTP2_HEADER_STATUS,
  HTTP2_HEADER_CONTENT_TYPE,
} = constants;

const server = createServer();

server.on('stream', (stream, headers) => {
  stream.respond({
    [HTTP2_HEADER_STATUS]: 200,
    [HTTP2_HEADER_CONTENT_TYPE]: 'text/plain; charset=utf-8',
    'Access-Control-Allow-Origin': '*',
  });
  stream.write('hello world');
  stream.end();
});


server.listen(3000);
await once(server, 'listening');

const response = await fetch('http://localhost:3000/');

console.assert(response.ok, response.status);
console.log(await response.text());

server.close();

I can confirm the above code works with Deno and fails with the latest main.

aduh95 avatar Feb 13 '24 09:02 aduh95

HTTP/2 support in undici is experimental and not enabled by default. Note that we do not support H2, but only HTTP/2 (over TLS), due the necessary protocol selection.

To enable:

import { createServer, constants } from 'node:http2';
import { once } from 'node:events';
import { setGlobalDispatcher, Agent } from './index.js';

const {
  HTTP2_HEADER_STATUS,
  HTTP2_HEADER_CONTENT_TYPE,
} = constants;

setGlobalDispatcher(new Agent({
  allowH2: true
}));

fetch('https://api.sandbox.push.apple.com').then(r => r.text()).then(console.log)

I think we could enable it in the next major and see how it goes. Unfortunately it might be breaking in case of bugs.

mcollina avatar Feb 13 '24 09:02 mcollina

I believe it should be fine as soon as we keep it as experimental though enabled by default when chosen by the server.

metcoder95 avatar Feb 13 '24 14:02 metcoder95

I believe it should be fine as soon as we keep it as experimental though enabled by default when chosen by the server.

Good Idea! Repeating it for everybody:

We want to enable HTTP/2 if it is the only protocol that the server advertises in the TLS exchange, but prefer HTTP/1.1 for everything else. 100% agreed.PR?

mcollina avatar Feb 13 '24 15:02 mcollina

@mcolina Just to clraify: I'm using Http2SecureServer with allowHttp1: true to return an error on unexpectedProtocol. So sounds like my server would always return the error in this proposal.

Can this behavior be controlled by a flag?!

mctrafik avatar Feb 14 '24 01:02 mctrafik

@mctrafik you can already turn on http2 with:

import { setGlobalDispatcher, Agent } from 'undici';

setGlobalDispatcher(new Agent({
  allowH2: true
}));

mcollina avatar Feb 14 '24 06:02 mcollina

I believe it should be fine as soon as we keep it as experimental though enabled by default when chosen by the server.

Good Idea! Repeating it for everybody:

We want to enable HTTP/2 if it is the only protocol that the server advertises in the TLS exchange, but prefer HTTP/1.1 for everything else. 100% agreed.PR?

SGTM 👍

metcoder95 avatar Feb 14 '24 09:02 metcoder95

If anyone is looking for a way to enable H2 without requiring undici (which is currently not exposed as a node built-in), here's the trick:

global[Symbol.for("undici.globalDispatcher.1")] = new global[Symbol.for("undici.globalDispatcher.1")].constructor({
	allowH2: true,
});

Cheers

3x071c avatar Mar 16 '24 00:03 3x071c

Note that we do not support H2, but only HTTP/2

What does this mean?

From RFC 9113:

The string "h2" identifies the protocol where HTTP/2 uses Transport Layer Security (TLS)

I am trying to understand the nuance of not supporting H2, but only HTTP/2.

hjr3 avatar Apr 06 '24 21:04 hjr3

I am trying to understand the nuance of not supporting H2, but only HTTP/2.

I meant H2C, which is the non-tls variant.

mcollina avatar Apr 07 '24 08:04 mcollina