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

Support HTTP/2 #342

Open
guybedford opened this issue Sep 9, 2017 · 43 comments
Open

Support HTTP/2 #342

guybedford opened this issue Sep 9, 2017 · 43 comments

Comments

@guybedford
Copy link

I just wanted to try and start a discussion here to see if this is something suitable for this project, or if it is something more suited to another one?

In the browser HTTP/2 gets used opaquely - would be really nice to get the same experience in NodeJS to provide a unified experience, perhaps with just an opt-in flag for using the Node core support here.

I wonder what the technical hurdles would be faced with the API? I guess HTTP/2 doesn't have an "agent" interface the same way, but perhaps a userland agent wrapper could be created for HTTP/2, would that make sense? Apart from that what other API's may provide issues?

@TimothyGu
Copy link
Collaborator

I would agree with the general sentiment expressed in the OP: that HTTP/2 should be provided transparently, like it is in browsers.

The biggest non-technical challenge right now, is that I haven't kept myself real up-to-date about the development of the http2 module in Node.js ;)

Joking aside, I see the following issues. They may or may not be faced by other workalike modules.

  1. The Fetch Standard isn't really defined in terms of HTTP/2. Yes, there is verbiage about it in specific situations, but it'll be hard to correspond one-to-one. Features like HTTP/2 Push are not really exposed through the browser API (see HTTP/2 server push support whatwg/fetch#51), and ultimately we are reluctant to expose too many Node.js-specific features.

  2. Node.js' HTTP/2 requires opening a Http2Session, from which requests are to be done. We can certainly create an Http2Session per request, but that could bring a performance hit. Ideally a module, either in core or otherwise, could be used to handle Http2Session cache for us (cache it by URL origin or target server, automatically invalidate the cache when session shuts down, etc.). Though if this cache should be provided by the user or be a global one will require some investigation into what browsers do.

  3. I'm not familiar with how protocol negotiation works with HTTP/2, and if it is supported by Node.js' HTTP/2 client (I know ALPN is supported by the server).

I would of course appreciate any insights or literature about these topics, and if anyone volunteers to take a stab at this or any of the above issues turn out to be non-concerns – even better :)

@bitinn
Copy link
Collaborator

bitinn commented Sep 9, 2017

I am not sure HTTP/2 is actually enforced in Fetch, see their headers requirement: #260, that's explicitly not HTTP/2 AFAIK.

(EDIT: also interesting to know what sort of use-cases people can think of, make Fetch HTTP/2-aware is unlikely going to give much performance or usability improvement over a simple keepalive on Nodejs?)

@TimothyGu
Copy link
Collaborator

@bitinn Compressed headers are still nice though.

@bitinn
Copy link
Collaborator

bitinn commented Sep 10, 2017

@TimothyGu a few things:

  • So I didn't know NodeJS come out with a native HTTP/2 support recently, that at least make this feasible without much userland code. https://nodejs.org/api/http2.html#http2_compatibility_api

  • Should we assume it will be eventually as simple a matter as setting a flag and changing to a require("http2") line? IF so, sure, we will look into that. (EDIT: This is the part I think a separate module might help, a compatible API for http and http2.)

  • But I am not certain about the performance gain, HTTP/2 seem to offer more benefit to web browser than API or other use cases. See their argument: 1, 2.

@TimothyGu
Copy link
Collaborator

Should we assume it will be eventually as simple a matter as setting a flag and changing to a require("http2") line?

No :( I don't think so, at least.

We would want to make the HTTP/1 and HTTP/2 distinction fully transparent, and doing so requires either ALPN (for HTTPS) or Upgrade header or Alt-Svc (for unencrypted HTTP). I think browsers usually only allow encrypted HTTP/2 though.

This is the part I think a separate module might help, a compatible API for http and http2.

👍 I'll try to raise this to Node.js for some help on making such a module, as I'm not entirely sure how (or if it would be possible) to create a client HTTP/2 session from an existing socket in Node.js yet.

@guybedford
Copy link
Author

Really glad to hear there might be interest. Yeah James seems to have dropped quite the bomb with this and it seems really impressive work that will take some time to integrate and upgrade ourselves for :P

It seems like one of the differences handling HTTP/2 is due to the fact that Fetch in browsers has a cache layer which can deal with push requests. A separate "on('push')" or similar may be needed to have a special Node handler for this case, without assuming default cache population, which I think would be a fine solution.

The main advantage to focus on though is getting the high throughput multiplexing of HTTP/2 when using fetch for multiple URLs all to the same remote. Ideally pooling should be handled internally, just like agents do for HTTP/1 currently (partially why I suggested a pseudo agent implementation above, although may be off track here on my lack of internals knowledge). Just like HTTP/1 as well, one can assume that the connection should be closed after a timeout when the pool is empty for a given IP and port socket connection (or origin and port, but I think IP-based sockets for cross-credentialed-concerns are supported with HTTP/2 - there was a whatwg spec discussion on this recently I believe). The ALPN negotiation seems fairly well documented for the Node HTTP/2 implementation as well.

I'm sure the HTTP/2 implementation is in need of feedback as well, so if something like reusing a previously created socket is a concern, these are likely useful discussions to have and be seeking assistance on.

This side of things is not really my area, but I would definitely do as much as I can to help here as the pay off from a performance perspective would be amazing to see.

@TimothyGu
Copy link
Collaborator

Yeah, I agree with the Agent thing. Connection pooling is substantially more well-defined for HTTP/2.

But at the same time, an HTTP/2 connection is not stateless like an HTTP/1.1 connection is: there are SETTINGS controlling behaviors in an HTTP/2 connection. For this reason, while Node.js' HTTP/1.1 client has global agents, a similar concept for HTTP/2 would be difficult to reason about, either node-fetch-global or Node.js-global.

Also, as long as we don't provide caching and Fetch doesn't provide server push APIs, I don't really want to try to support server push

@TimothyGu
Copy link
Collaborator

The ALPN negotiation seems fairly well documented for the Node HTTP/2 implementation as well.

Only for the server side, unfortunately.

@guybedford
Copy link
Author

guybedford commented Sep 12, 2017

For this reason, while Node.js' HTTP/1.1 client has global agents, a similar concept for HTTP/2 would be difficult to reason about, either node-fetch-global or Node.js-global.

Aren't the settings limited to connection-level settings though? Window size etc mostly for managing the correct bandwidth settings? Or are there other more stateful things to do with the connection? I would have said based on this that the same Agent model of default global pooling would apply, but otherwise http2 could be enabled say through a pool option or similar perhaps fetch(url, { http2: { pool: new Http2Pool(opts) } }.

Also, as long as we don't provide caching and Fetch doesn't provide server push APIs, I don't really want to try to support server push

If there's a way to disable PUSH for HTTP/2 servers then great, but as far as I'm aware the CANCEL response for PUSH only happens after data is pushed, such that responding to the event may well still be beneficial.

@TimothyGu
Copy link
Collaborator

Aren't the settings limited to connection-level settings though? Window size etc mostly for managing the correct bandwidth settings? Or are there other more stateful things to do with the connection?

Hmm I think you are right. There is SETTINGS_ENABLE_PUSH, but as long as we have a module-global cache that have push always disabled that should be fine.

@guybedford
Copy link
Author

I wasn't aware it was possible to negotiate an opt-out of push through settings. That sounds like a sensible default then.

@grantila
Copy link

Given this module is for Node, and often it will be used between services in a backend (where encryption isn't always important), user will more or less have to know (and choose) whether to connect to an HTTP/1 or HTTP/2 server. The session/socket/agent handling is radically different between the two, and services may almost depend on H2 in terms of multiple long-polling streams etc.

The choice of the web browsers to only speak H2 over TLS has a subtle benefit - they don't need to know in beforehand if the server speaks HTTP/1 or 2. For clients speaking unencrypted H2, it'll get tricker, so a fetch() "transparent" to this would be quite messy.

Because of the above, (and also because I quickly needed a nice http2 module frontend), I wrote fetch-h2 which is practically this module but dedicated to the http2 module. It's written from scratch (in TypeScript) so I'm not sure if we could benefit from much code sharing, but I would be happy to break out parts of it (like Headers, Request, Response, etc) if there will be serious work for HTTP/2 in this module, and we'd all benefit from reusing some of this logic.

I published it yesterday, so it's very early, but the http2 module is in very early stage too, e.g. it can't connect to an encrypted H2 server (i.e. HTTPS/2), only unencrypted..

@guybedford
Copy link
Author

@grantila great work, I haven't had a chance to play around with it yet but reading through the readme it looks very promising! And really appreciated for joining the discussion here for possible collaboration. How closely does it follow the same sort of API here down to the request and stream details?

Looking through the API compatibility it seems like they are very similar, in providing timeout and redirect. I wasn't sure what the exact type of body is but would be easy to confirm it works roughly the same. compress and handling credentials seems the main features missing then as far as I can see?

For credentials, it could be nice to possible mimic the agent interface here that has all the set credentials options on it already. This could perhaps also even double as the context option for pooling just like an agent. Just a suggestion, would be interested to hear your thoughts. Also would it not be possible to provide support for credentials, so long as they were provided with each request regardless of cookie management? I understand there may be some stateful considerations here with servers that may not allow multiple authentications on the same connection, but the context feature seems like something that could possibly provide the necessary isolation here if necessary?

In terms of how to select between HTTP/1 and HTTP/2, do you think a boolean option provide enough information here - http2: true/false?

If that would be adequate, there would be no reason not to keep these two as separate projects, but then have fetch-h2 as a dependency of this one that is at some level delegated to for that option. At least that seems like it would get us where we want to be pretty fast!

Of course this is all up to @TimothyGu here, so will wait for him to respond further.

@TimothyGu
Copy link
Collaborator

I would prefer it all to be transparent, i.e. not add another dependency that does the same thing we can do in just a few lines. In nodejs/node#16256 Node.js seems to be making good progress towards transparent HTTP 1.1 vs. 2 for TLS/ALPN at least, and if they do end up adding agent support we should be good to go too.

@hthetiot
Copy link

For alternative http2 pure JS implementation I recommend checking http2.js see details here:

@TimothyGu
Copy link
Collaborator

@hthetiot We will not be using any HTTP/2 implementation other than the one in Node.js, when the Node.js used has HTTP/2 support.

@shellscape
Copy link

What's the status on this? I'm presently writing tests leveraging node-fetch, for a new module which is capable of using native HTTP/2 when run via Node v9+.

@devsnek
Copy link

devsnek commented Feb 22, 2018

i'm going to take a look at this now, i have a working alpn negotiation script which connects to sites (rn hardcoded as google 😛) either with h2 or http/1.1

@shellscape
Copy link

@devsnek do you mean to say that you're going to take a look at updating this module? looking to clarify before I get too excited 🙃

/cc @TimothyGu @bitinn

@devsnek
Copy link

devsnek commented Feb 22, 2018

@shellscape yes thats what i'm looking into

@TimothyGu i would call Http2ClientSession a valid "Agent", i'll look into abstracting the interface

@devsnek
Copy link

devsnek commented Feb 22, 2018

> void require('.')('https://nghttp2.org/httpbin/get').then(r => r.json()).then(console.log)
undefined
> (node:71326) ExperimentalWarning: The http2 module is an experimental API.
{ args: {},
  headers: { Host: 'nghttp2.org:443', Via: '2 nghttpx' },
  origin: '7x.7x.20x.7x',
  url: 'https://nghttp2.org:443/httpbin/get' }

right now the only problem is a new Http2ClientSession for every request (back to the agent stuff)

@guybedford
Copy link
Author

I just noticed this project - https://github.com/hisco/http2-client.

Since it mimics the http / https APIs that might provide a drop-in path to supporting transparent HTTP/2 in this project?

I'm sure there will be issues along the way, but it could be worth trying if anyone is interested in working on this!

@shellscape
Copy link

@guybedford thanks for sharing that. definitely going to have a look at that this weekend.

@sebdeckers
Copy link

@guybedford That's a neat compatibility layer.

I generally like the Fetch API as a common ground between Node.js and browserland. One thing I miss are server push streams/events. Anyone know if there is progress on this in the standards effort? Perhaps a general mechanism that could be extended to other HTTP/2 frames like ping, altsvc, etc.

@NotMoni
Copy link
Member

NotMoni commented May 6, 2020

Agree with @bitinn I’ll get started on a PR

@xxczaki
Copy link
Member

xxczaki commented May 9, 2020

@NotMoni While I created a PR for that, feel free to submit another one or review mine ;)

@NotMoni
Copy link
Member

NotMoni commented May 10, 2020

Sure

@xxczaki xxczaki changed the title Discussion: http/2 support Support HTTP/2 May 26, 2020
@thernstig
Copy link

@bitinn Did you work on this? :)

@bitinn
Copy link
Collaborator

bitinn commented Feb 25, 2021

@thernstig nope, however we will take a HTTP 1+2 wrapper PR if it's done properly, the main thing is we want http.Agent to still be usable (so users don't have to remember 2 sets of options). So far it has proven to be a great obstacle.

@willynlabs
Copy link

+1 on this! I tried to move to cross-fetch across my server and client code, which uses node-fetch underneath. It failed when parsing the response from the Apple apns server, which requires HTTP2:
https://developer.apple.com/documentation/usernotifications/setting_up_a_remote_notification_server/sending_notification_requests_to_apns/

@JudahGabriel
Copy link

+1, this is a need for our project at PWABuilder.

We use node-fetch in our backend to package PWAs for different app stores.

What we are seeing is many PWAs are now using HTTP/2. Our software fails on these sites because node-fetch doesn't work with HTTP/2.

@xxczaki
Copy link
Member

xxczaki commented May 3, 2021

Once we transition to ESM (#1141) I will be able to look into this once again.

andreban added a commit to andreban/llama-pack that referenced this issue May 7, 2021
- `node-fetch` doesn't support HTTP/2 and has had an issue for
  support open since 2017: node-fetch/node-fetch#342
- `fetch-h2` supports HTTP1.1 and HTTP/2 with upgrade.
andreban added a commit to andreban/llama-pack that referenced this issue May 25, 2021
- `node-fetch` doesn't support HTTP/2 and has had an issue for
  support open since 2017: node-fetch/node-fetch#342
- `fetch-h2` supports HTTP1.1 and HTTP/2 with upgrade.
andreban added a commit to GoogleChromeLabs/bubblewrap that referenced this issue May 26, 2021
* Replaces `node-fetch` with `fetch-h2` on `core`

- `node-fetch` doesn't support HTTP/2 and has had an issue for
  support open since 2017: node-fetch/node-fetch#342
- `fetch-h2` supports HTTP1.1 and HTTP/2 with upgrade.
- Defaults to using `fetch-h2`.
- Allows optionally using `node-fetch`.
- Centralises calls to `fetch` in `FetchUtils`.
@o-alexandrov

This comment was marked as spam.

@petarzarkov

This comment was marked as spam.

@lisonge

This comment was marked as spam.

@itaitai

This comment was marked as spam.

@MartynFitzgerald

This comment was marked as spam.

@cdrn

This comment was marked as spam.

@jimmywarting
Copy link
Collaborator

I'm afraid there won't be any real effort into trying to accomplice this complicated task of trying to upgrade connections to HTTP/2 by conditionally check if the server supports it or not. node-fetch is kind of being replaced by built in fetch provided by NodeJS (undici) themself

I think it will be best to follow: nodejs/undici#399

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

Successfully merging a pull request may close this issue.