fastify / fastify-early-hints

Draft plugin of the HTTP 103 implementation
MIT License
21 stars 2 forks source link

http2 vs http1 support seems backwards #7

Open moonmeister opened 1 year ago

moonmeister commented 1 year ago

Prerequisites

Issue

So, in everything I've read, but most importantly, the Chrome docs, Early Hints are only supported on http2 and 3. But, this plugin throws an error if I try to set Fastify to use http2.

Is this not backwards? If correct as is, what's the reason we've disabled this on http/2? Thanks all!

climba03003 commented 1 year ago

Is this not backwards?

It is the choice from Chromium and it does not means Early Hints is not supported in HTTP/1.1.

What's the reason we've disabled this on http/2?

I believe is the complexity of directly write to socket for HTTP/2.

Uzlopak commented 1 year ago

Also we expect that fastify is behind a reverse proxy like Ingress/traefik which resolves a http2 request to http1.1 and vice versa.

Maybe we can transplant http2 from nodejs sourcecode to our plugin?

climba03003 commented 1 year ago

Maybe we can transplant http2 from nodejs sourcecode to our plugin?

Yes, we can use reply.raw.stream for the reply. Details on https://nodejs.org/api/http2.html#http2streamadditionalheadersheaders

mcollina commented 1 year ago

I think we should just use the native functionality: https://nodejs.org/api/http2.html#responsewriteearlyhintslinks.

climba03003 commented 1 year ago

I think we should just use the native functionality: https://nodejs.org/api/http2.html#responsewriteearlyhintslinks.

We need to provide a compatible layer for node@14 and node@16. Patching to use additionalHeaders and .socket are needed.

And, if the native one is available, for example node@18.11.0. We should absolutely go into that.

jsumners commented 1 year ago

Also we expect that fastify is behind a reverse proxy like Ingress/traefik which resolves a http2 request to http1.1 and vice versa.

That's not necessarily true. A reverse proxy can still use http/2 to communicate to the backend services.

Maybe we can transplant http2 from nodejs sourcecode to our plugin?

That would be a maintenance nightmare.

climba03003 commented 1 year ago

That would be a maintenance nightmare.

Not really re-invented the wheel, http2 already provide a method called additionalHeaders. We can directly pass the header with additional :status.

Uzlopak commented 1 year ago

My assessment:

I am wondering how we can test http2 because undici does not support http2

climba03003 commented 1 year ago

I am wondering how we can test http2 because undici does not support http2

Use built-in net or http2 package.

Uzlopak commented 1 year ago

created #8 for discussing http2 implementation

lilouartz commented 1 month ago

Looks like the PR was closed without resolution :-(

climba03003 commented 1 month ago

Looks like the PR was closed without resolution :-(

Since, Node.js already implemented the writeEarlyHints in both http and http2. You should use the native function instead of relying this project (unless you are in old Node.js version).

lilouartz commented 1 month ago

ah, would be useful to document this somewhere.

mcollina commented 1 month ago

https://github.com/fastify/fastify/issues/5479