Discussion: Exit criteria for bringing http2 out of experimental

[jasnell]

It's time to start figuring out what are the final steps necessary to bring http2 out of experimental?

Let's build a list.

• Expectation + evidence of API stability (e.g. list of user-space modules/applications building on top of http2)
  • http2.live
  • fastify
  • grpc
  • ...?
• Modules in citgm that exercise the http2 API
  • fastify
• Reports of http2 deployed in production
  • http2.live
• Framework adoption
  • fastify
  • restify
• full (or at least fuller) test coverage

/cc @nodejs/http2 @nodejs/tsc

EDITED BY TROTT: added test coverage. I took the "let's build a list" as an invitation to edit. Feel free to undo my edit if I'm misunderstood.
EDITED BY OFROBOTS: edited 'citgm' bullet to complete as fastify has been added.

[ofrobots]

Suggestion:

• Expectation + evidence of API stability (e.g. list of use-space modules building on top of http2)

[jasnell]

Added... although I made it "modules/applications" because it's more likely to have more deployed applications building on the API than it is to have individual modules.

[mcollina]

• Having some modules in CITGM that exercise the HTTP2 API.
• someone reporting having deployed http2 in production.
• frameworks adopting it (we have already two, restify and fastify) - should we wait for Express?

[jasnell]

Added!

[wesleytodd]

Express is in an interesting position right now on this. This is the furthest along PR expressjs/express#3390.

I did some benchmarks last weekend to start proving out a best way forward for that PR, and was planning on spending some time over the next few weeks moving it forward. But if you wait for 5.x, I am worried at how long it will take.

TBQH, I really want to push 5.x out because it has been just dragging on for too long. So if this is the nudge we need to push it over the finish line that sounds great.

EDIT: I also recently meet with @DonutEspresso about collaboration between express and restify, and he mentioned that they had not looked at http2 support yet. Maybe I am wrong? Clearly I am wrong: restify/node-restify#1489

[devinivy]

hapi supports http/2 push under the underdog module. But my understanding is that framework-level http/2 support is in the med/long-term roadmap. Basically, http/2 works well with hapi today under the compatibility API.

[leodutra]

Would be possible to enable HTTP/2 without having to use a cert and key?
There's a lot of microservices running using HTTP without SSL/TLS and being shielded by Nginx / other proxies with a single point SSL/TLS config.

I think the answer will be dependant on something deeper, maybe Node core modules, but I really wanted to check and ask for it.

[styfle]

Would be possible to enable HTTP/2 without having to use a cert and key?

Yes, however it will not work if you attempt to connect to the server via web browser.

See http2.createServer vs http2.createSecureServer and note the comments in the code snippet 😉

[mudrz]

Adding hapi issue link for reference hapijs/hapi#3584

[paambaati]

Should the list of userspace modules also include client-side request modules? None of the popular ones (request, axios, got, urllib & needle to name a few) support HTTP/2 right now.

[devsnek]

node-fetch is waiting on http2 to have agents, but otherwise there's already a pr open to it with initial support

[mcollina]

node-fetch is waiting on http2 to have agents, but otherwise there's already a pr open to it with initial support

Considering that there is no PR in the work for that, or that I didn't know about this requirement, it might be a very long wait. It would be really good to get that implementation going. My idea for that is described in #17746.

[mcollina]

Should the list of userspace modules also include client-side request modules? None of the popular ones (request, axios, got, urllib & needle to name a few) support HTTP/2 right now.

There is no Compatibility layer for the client. We might want to add that, but it was never on-scope for the initial release, and I would be somewhat concerned about making that a requirement for the exit criteria.

I wrote https://github.com/mcollina/h2url for that.

[jasnell]

@nodejs/tsc @nodejs/http2 ... at this point, the HTTP/2 API has stablized. There are ecosystem users (most significantly Google's grpc now builds on top of http/2 in core). There have been some major bug fixes. I think we are reasonably close to having http/2 ready to come out of experimental. I'd like to target the end of August for the official switch over.

Any objections? Any remaining tasks?

[Trott]

Any remaining tasks?

If we don't have a few modules in CITGM that use http2, we should see about adding a few.

[apapirovski]

The current stream and session destroy flow needs to be significantly tightened. There are still a substantial number of bugs.

Ideally we would have a better story around the client side request api. Right now it's pretty hard to port over existing libraries and their tests to http2. That's the major stumbling block for express.

[jasnell]

@apapirovski ... Do you have a summary or checklist of the known bugs that are remaining to knock out?

[addaleax]

Any remaining tasks?

I think we want to get the contents down of https://github.com/nodejs/node/issues?utf8=%E2%9C%93&q=is%3Aopen+label%3Ahttp2+label%3Aconfirmed-bug to 0

[ChALkeR]

Also #21440.

It also means that those errors are not covered by any http2 tests, so perhaps http2 needs more test coverage.

Refs: https://coverage.nodejs.org/coverage-1bf42f4777bfc7ce/root/internal/http2/core.js.html

[mcollina]

@Trott

If we don't have a few modules in CITGM that use http2, we should see about adding a few.

I can add Fastify and h2url.

@apapirovski

Ideally we would have a better story around the client side request api. Right now it's pretty hard to port over existing libraries and their tests to http2. That's the major stumbling block for express.

What do you have in mind? A compatibility mode for the client side as well? Support for a pool of connections/agent-like?

[mcollina]

I've send the PR to add fastify to CITGM: nodejs/citgm#586.

[AyushG3112]

I'm not sure if we already have it for not, but is adding the GRPC sample to CITGM worth it?

[Trott]

Looking at coverage.nodejs.org, it seems like there are some code branches that should be covered by tests that aren't, particularly in core.js. Today's status on that file in particular: https://coverage.nodejs.org/coverage-e55bdde92513ecf4/root/internal/http2/core.js.html

[ofrobots]

Looking at the status since it was last asked:

• we have modules in CiTGM that exercise http2 (thanks @mcollina) as requested by @Trott. gRPC team is interested in getting their library added to CiTGM, but the repo currently uses submodules to do cross testing of HTTP2 based version with non-HTTP2 based version. CitGM doesn't support submodules. Fixing this will require some refactoring on the gRPC side. Either way, I think the original criteria has been met.
• http2 confirmed bugs list is down to zero as of this writing.
• @kjin is working on a back-port of http2 commits to 8.x.

Things that may still need addressing are:

• @apapirovski asked for a client side request story, but hasn't clarified when asked about specifics by @mcollina.
• Code coverage needs to be higher, as pointed out by @Trott. @kjin and I will work on this now.

Things are looking fairly solid to me at this point. I would like to propose that we remove the 'experimental' warning emitted on the console at this point. We still have some time before we mark it non-experimental in the docs.

[jasnell]

Code coverage is around 95% right now and is impacted by a number of defensive code paths that would be exceedingly difficult to construct even artificial tests for. The fact that some of those paths aren't ever take is actually a good thing test wise ;-) ... There are additional tests that could be added, but the coverage is actually really good in comparison to other modules and the current coverage numbers should not be a blocker to coming out of experimental.

I also don't think we should block on the current lack of a better client API.

[BridgeAR]

I agree that it's time to get http2 out of experimental. It is not a fast moving target anymore and any further issues could and should be fixed with the regular development and release cycle.

I personally would say increasing the coverage would still be good though.

We might also want to define some last goals if someone still has the feeling something has a rough edge or should be addressed before coming out of experimental.

I would like to get the opinion from @apapirovski on this as well.

[jasnell]

I will get a PR opened tonight :)

…

On Wed, Aug 22, 2018, 17:16 Ruben Bridgewater ***@***.***> wrote: I agree that it's time to get http2 out of experimental. It is not a fast moving target anymore and any further issues could and should be fixed with the regular development and release cycle. I personally would say increasing the coverage would still be good though. We might also want to define some last goals if someone still has the feeling something has a rough edge or should be addressed before coming out of experimental. I would like to get the opinion from @apapirovski <https://github.com/apapirovski> on this as well. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#19453 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAa2eRXrYlrgs_FCWqTGsuDVmgWQFyrYks5uTfRWgaJpZM4SwjUl> .

[szmarczak]

Please look at this #22268 IMO it's a bug.

[apapirovski]

I approved the PR for moving out of experimental so that should make opinion clear, but to sum it up: I think there was a good amount of work over the last 3-4 months in making http2 more stable and as such it should now be ready to graduate. At least we don't randomly segfault now ;)

What do you have in mind? A compatibility mode for the client side as well? Support for a pool of connections/agent-like?

Not sure, perhaps compatibility mode is the answer. Or perhaps this can be done in user land with what we already expose...

[jasnell]

It's out of experimental!