Support http/2.

[sogaani]

This PR is rebased and added tests from https://github.com/expressjs/express/pull/3390 We need to disable some tests on http/2, as some node module have issue with http/2. PR pending on resolution of the following issues:

• [ ] // cookies with http2 has an issue https://github.com/pillarjs/cookies/pull/99
• [ ] // vhost with http2 has an issue https://github.com/expressjs/vhost/pull/29
• [x] ~~// HEAD with http2 does not support response body~~ response.sendFile has an issu. See also https://github.com/expressjs/express/pull/3730#issuecomment-419728593 fixed by commit https://github.com/expressjs/express/pull/3730/commits/2a78f4cb36ff63a18f1264ce1a0848c9bfedbf2b
• [x] supertest http2 support (supertest removed in https://github.com/visionmedia/superagent/commit/b08371c72fbdca40dda3ad39fe8bbbee4a2718d5) Fixed by https://github.com/visionmedia/superagent/pull/1414 Currently this PR does not depend on above issue, but this PR and the superagent PR have deplicate codes. We can delete those code after landing PRs.
• [x] http2ServerRequest.socket.destroy() has a bug (filed an issue ihttps://github.com/nodejs/node/issues/22855) Fixed in https://github.com/nodejs/node/pull/22896 Maybe land on node 10.12.0

[sogaani]

ping @dougwilson

[dougwilson]

I saw the PR come in, just haven't had time to look over it yet, I apologize.

[trivikr]

Update: Node.js v10.10.0 was released an hour ago, and http2 module is no longer experimental https://nodejs.org/en/blog/release/v10.10.0/

[dougwilson]

So I started taking a look into this, and it seems like this is good, but only a first pass. There are still a bunch of issues. Mainly, the test suite of Express doesn't extensively test all of it's dependencies, even just the high-level ones like express.static and express.json, etc. These seem to not work correctly under the HTTP/2 implementation in this PR. I think that definitely makes the PR a bit less than ideal here.

I'm going to write up a list of the issues I'm finding. Also, it seems like there is at least one more test disabled for HTTP/2 not mentioned in the OP. We may want to add a checkmark list (I did that already) and track what is the known-outstanding issues. One of the comments in the PR is HEAD with http2 does not support response body but I'm not sure what that means -- the test is not trying to send a response body to a HEAD, as HTTP/1 doesn't support a response body in a HEAD, either.

[sogaani]

One of the comments in the PR is HEAD with http2 does not support response body but I'm not sure what that means -- the test is not trying to send a response body to a HEAD, as HTTP/1 doesn't support a response body in a HEAD, either.

It's my misunderstanding. I look into it and find issue about sendFile with http2 head method. Failing sendFile with head method is caused by difference with http_outgoing_message and http2ServerResponse. http2ServerResponse.finished with head method is true from its made, but http_outgoing_message.finished with head method is false until calling http_outgoing_message.end().

res.sendFile with head method seems to depend on the order.

1. call onfile.
2. call onfinish.

However, http2ServerResponse.finished is always true so that onfinish is called first. I will fix it tomorrow.

[dougwilson]

@sogaani superagent just dropped the HTTP/2 support https://github.com/visionmedia/superagent/commit/b08371c72fbdca40dda3ad39fe8bbbee4a2718d5 maybe you can help them, or maybe we just need to use something else for our http/2 tests.

[sogaani]

@dougwilson Thank you telling me that. I made PR for superagent. https://github.com/visionmedia/superagent/pull/1414 I added same change here.

[dougwilson]

I think the tests are revealing some kind of Node.js bug:

     Uncaught TypeError: Cannot read property 'trailers' of undefined
      at Immediate.finishSendTrailers (internal/http2/core.js:1467:31)

[sogaani]

It is interesting. I will look into it.

[sogaani]

Regarding to TypeError, I found a nodejs bug and file the issue. https://github.com/nodejs/node/issues/22855

[sogaani]

@dougwilson Do you have a list of modules you will investigate to find issue? Maybe I can help you to find the issues by adding http/2 test to modules.

[p3x-robot]

hello guys, i know there was a PR that was merged into one of the branches that supports now http2. i am not seeing this merged PR in the latest releases. Do you guys it is now implemented (with http2 together with websockets)? If sow, which version?

Thanks!

[Abourass]

Since the person making this PR has also dropped off (judging by submission history, and the month or so since an update), what's the progress here? Is this the sort of thing we may see in a month? A year? I can't find any timeline for the next release of Express, so wondering whether this is coming soonish, or should I begin to look into other solution / switching frameworks?

[p3x-robot]

It looks like it only possible to work it with spdy right now.

[addaleax]

Is there anything I could do to help here?

[dougwilson]

It turns out that even though http2 in core has a compatibility API, it is not transparent and still causes a lot of breakages. There are the obvious issues of the colon-prefixed headers that show through in req.headers and so now our internals and existing middleware need to be reworked to properly handle these (think for example anything looking at req.headers.host), but also even the events that are emitted on the objects seem to be slightly different, which is having ramifications on our deep internals.

I've pretty much come to the conclusion and started working on what I think is the only real option: create an entirely new request and response object that completely wraps the node.js core ones and evens them out between http/1 and http/2 so no module ever have to understand the difference between them. Otherwise there is going to be a growing divide in the middlewares, and just using a middleware you'll not easily be able to tell if it works with http/1, http/2, or both.

I think that the common deployment behind lbs will be http/1 for the foreseeable future, but http/2 is growing and of course is needed to provide new features, so it does news to be supposed in express. But we cannot scrasifice the usability of middleware by forcing them to manually support 1&2 and hope that just works out.

This is a large effort to create this new abstraction layer, though I have recently started work on it. It would be awesome if the core http2 compatibility api was just this layer directly, but i think with it shipping as stable that ship has sailed and express needs to stop letting the node.js core object shine through at all to ensure a stable api.

[p3x-robot]

@dougwilson so it seems, it is not just straight put in some quick http2 code, but it needs more work, but it will be actually finally be working in the foreseeable future ?

[dougwilson]

Both are correct and on the latter point I am spending all my free time on this task currently.

[aapoalas]

Sorry to bug like this, but is there any status update on the HTTP/2 support? Any signs that it would make it to Express.js 5?

[p3x-robot]

i solved it using express http/v1.1 and i use a nginx https2 proxy, that is the best solution.

[alexander-akait]

Guys what is status? Maybe i can help?

[alexander-akait]

/cc @dougwilson friendly ping. Can we release this without major bump? How i can help? Maybe you can to list issues/PRs/TODOs?

[ghost]

Any updates? I can't wait for Lighthouse to stop screaming at me once this gets implemented.

[XOR-LIFE]

HTTP/2 is not gonna work in browsers unless you have TLS enabled (HTTPS), and you not gonna let Node/Express handle the SSL traffic because it's much of a burden on it, therefore, you would use Nginx as a reverse proxy in production for your node application. And Nginx will handle the SSL traffic and support HTTP/2.

So, for express or even node to support HTTP/2 is just a wild idea and not technically practical in real life (production environment).

See my tut: ( https://github.com/XOR-LIFE/node-production )

[m1o1]

HTTP/2 is also very valuable in service-to-service communication between microservices, not just from frontend browsers. gRPC uses HTTP/2 for example, though I guess that isn't totally relevant to express. But it would still be valuable for express to have HTTP/2 support for REST APIs

[asbjornu]

I make a case for why server frameworks (and not just (reverse) proxies) needs to support HTTP/2 in https://github.com/aspnet/AspNetCore/issues/4249#issuecomment-460288563. There are more use-cases, but I think the one explained there is a pretty strong one.

[alexander-akait]

/cc @dougwilson friendly ping again, what is blocker, where i can help?

[dougwilson]

https://github.com/expressjs/express/pull/3730#issuecomment-441338216

[alexander-akait]

@dougwilson thanks, any roadmap/ideas/spec how we can solve this and how i can help? It seems to me that now nobody works on this and it is very awful, a lot of software is used express and need this feature so I offer my help, but without feedback from core developers i can't help.

[dougwilson]

Currently we are trying to focus on the 5.0 release so it does not continue to be years without release. We will come back to focus on items like http/2 once the 5.0 is out and work to figure out the points you have raised around http/2