HTTP2 support

[gunta]

Hi!

Can you consider adding HTTP2 support to GOT?
Currently the most used client library is this: https://github.com/molnarg/node-http2

Thank you!

---

IssueHunt Summary

szmarczak has been rewarded.

Backers (Total: $200.00)

• issuehunt ($200.00)

Submitted pull Requests

• #1051 Rewrite Got

---

Tips

• Checkout the Issuehunt explorer to discover more funded issues.
• Need some help from other developers? Add your repositories on IssueHunt to raise funds.

[gunta]

Support for it can be detected with this: https://github.com/stefanjudis/is-http2

[julien-f]

As far as I know, the http2 module is not well maintained, you should use spdy instead.

And I don't believe it is needed to detect HTTP2, simply use spdy to make the request and the protocol will be properly negotiated.

@sindresorhus, @floatdrop you could make this optional by using a try {} catch (e) {} (like I did in this module) and documenting that spdy can be installed to enable HTTP2 support.

[gunta]

That's nice to know :)

[julien-f]

I think I've read somewhere that spdy is the HTTP2 implementation that will be merged into Node in the future.

[gunta]

It should, but it seems that is going to take time since there are too many arguing for it/against it.

The try/catch solution or perhaps adding spdy as an optional dependency should be a good solution meanwhile.

[sindresorhus]

Why not just use the agent option?

got('google.com', {agent: require('spdy').createAgent()});

[julien-f]

That's indeed the easiest approach, but it still requires to specify the agent everywhere while the inclusion in got make it transparent…

[SamVerschueren]

Create a spdy-got wrapper module that sets the agent for you? Little bit like gh-got.

[sindresorhus]

I guess we could try/catch it, but let's see what @floatdrop says.

[floatdrop]

@sindresorhus I don't have clear opinion on this right now – there are many things to consider:

• Is current interface of got suitable enough for HTTP2 features?
• What will happen to users, that bundle got to browser?
• Will spdy module increase performance or not?

But I like idea and have service on sight, that could be ported to HTTP2 to test this.

What I'm certain about - this should be major release 😄

I will take deep look into it on weekend.

[sindresorhus]

@floatdrop To be clear how it would work. Node.js module resolution reads dependencies recursively upwards in the dependency tree. That means we can try/catch trying to require spdy even though we don't directly depend on it, and we'll be able to require it if the user that also depends on got, depends on spdy. I think that's the best way to support it right now.

[sindresorhus]

Relevant: nodejs/node-eps#38

[pietermees]

Seems like this is going to merge soon: nodejs/node#14239

[pietermees]

I was wondering if there was any update on this now that it has made its way into Node 8?

[paambaati]

http/2 has now been in core for a while now. Even though the module's stability is marked as experimental, it would be worthwhile to start looking into this.

[sindresorhus]

I welcome a PR for this, but it's not something I can prioritize right now.

[sindresorhus]

Relevant request issue: request/request#2033

[szmarczak]

https://github.com/szmarczak/http2-wrapper

[sindresorhus]

Relevant Node.js issue about bringing HTTP2 support out of experimental: nodejs/node#19453

[hisco]

I've created an http2-client that completely mimcs the http / https native modules while supporting http2.

It was very easy to integrate it with this module (and a few others) :
http2-got - I literally created this integration module today.

However, it has very few lines of code, all it's behavior comes from : http2-client , got

Hope it somehow helps...

[hisco]

Forgot to say, it's obviously negotiates with the server the correct protocol (ALPN) to choose, https vs http2.

I would be happy to contribute in any way I can.

[szmarczak]

New version of http2-wrapper released: 0.4.0. Many bug fixes etc. Upgrade very recommended. It's much stable now.

Here's Got example for HTTP2 with ALPN negotiation (supports HTTP, HTTPS and HTTP2) 🎉

https://gist.github.com/szmarczak/59db2fa713aa7f52949d343d9cf93ff6

[pietermees]

@szmarczak awesome! I still have a test on my todo list, will let you know ;)

[paambaati]

@szmarczak Thanks for the release! Is there a way I can modify the initial connect request options? I tried setting it in beforeRequest but I get a Parse Error.

https://gist.github.com/paambaati/5e52b5bc42b2e6eebcfb55213d1853a1

[szmarczak]

@paambaati The problem is you are trying pass ClientRequest to options.request, while it should be a function. Just revert your changes and do this:

    const {body: h2body} = await got('https://nghttp2.org/httpbin/headers', {
        json: true,
        headerTableSize: 65536,
        maxConcurrentStreams: 1000,
        initialWindowSize: 6291456
    });

[szmarczak]

[email protected] has dedicated function for resolving ALPN using HTTP request options.
Now, it's much simpler to create Got instance with ALPN negotiation :D

const http2 = require('http2-wrapper');
const {extend: gotExtend} = require('got');

const got = gotExtend({
    hooks: {
        beforeRequest: [
            async options => {
                if (options.protocol === 'https:' && !options.request) {
                    const {alpnProtocol, socket} = await http2.auto.resolveALPN({
            			...options,
            			resolveSocket: !options.createConnection
            		});

            		if (socket) {
            			options.createConnection = () => socket;
            		}

                    if (alpnProtocol === 'h2') {
                        options.request = http2.request;
                    } else {
                        options.session = options.socketSession;
                    }
                }
            }
        ]
    }
});

[pietermees]

@szmarczak I still feel like haven’t solved the connection caching and reuse question well. As far as I can tell we’ll still establish a new Socket for every request. I’ve tried to address this in alpn-agent and I’ll try to splice that into the example in the next couple of days.

[szmarczak]

@pietermees You're right. It is so, because to solve szmarczak/http2-wrapper#6 session[kState].pendingStreams (+some events) needs to be exposed. Let's raise a Node issue about that.

[pietermees]

Per the continued discussion on that thread, I think there's enough in place to provide a solution today, without changes to Node. But I'll try to prove that out with an example :)

[pietermees]

@szmarczak I've created a demo using got with @zentrick/alpn-agent and http2-wrapper here.

Seems to work great!

You can see debug output by running the script with DEBUG=alpn-agent* env var. You'll see how it caches TLS sessions, reuses connections, and deals transparently with h1 and h2.

[IssueHuntBot]

@IssueHunt has funded $200.00 to this issue.

---

• Submit pull request via IssueHunt to receive this reward.
• Want to contribute? Chip in to this issue via IssueHunt.
• Checkout the IssueHunt Issue Explorer to see more funded issues.
• Need help from developers? Add your repository on IssueHunt to raise funds.

[szmarczak]

@pietermees I just figured out that providing agent will cause sockets to be not reused. I don't think we can reuse sockets at all because people want the default Agent to manage the sockets.

Making the H1 Agent use our sockets is just too hard, IMO not worth it. I propose this:

1. Try to retrieve the ALPN protocol from our cache.
2. If the cache's empty, then open a new connection, retrieve the ALPN protocol and close the connection. If someone uses Agent the socket can't be reused either.

[pietermees]

@szmarczak could you explain why agent would prevent sockets from being reused? My understanding is that it's the opposite: agent with keepAlive: true is required to reuse sockets?

I'm concerned an ALPN cache might introduce some unexpected issues and behavior:

1. The same DNS name can point to many IPs, some of which can have one config, and some of which can have another.
2. Similarly, the same IP can change configuration over time, which matters for long running applications that are up for weeks or months.

[szmarczak]

could you explain why agent would prevent sockets from being reused? My understanding is that it's the opposite: agent with keepAlive: true is required to reuse sockets?

I mean it reuses sockets for HTTP2. But to reuse sockets for HTTP1 you need to provide another agent because the H2 agent is not compatible with H1 agent. So in total there will be two sockets.

Reusing H2 sockets for H1 is a bad idea because H2 sockets must not be malformed. There are different settings for H2 and H1...

Another example: imagine all seats for free sessions are taken and you've just finished your request. The session is free and it's being closed. If there's some pending H1 request it will throw an error.

The same DNS name can point to many IPs, some of which can have one config, and some of which can have another.

Good point.

Similarly, the same IP can change configuration over time, which matters for long running applications that are up for weeks or months.

I'll look into this.

[rainder]

@pietermees Thanks for the example. However, there seems to be a serious incompatibility with got implementation. Request just crashes the app on failure. Easy steps to reproduce:

• make runTest('http2-alpn', 'https://some-domain.com') to call every second so that you can make sure connection gets reused
• after few successful requests stop the server for some-domain.com
• get the following error which kills the client app

/Users/endriu/Developer/test/node_modules/@szmarczak/http-timer/source/index.js:58
                socket.once('lookup', lookupListener);
         ^
TypeError: Cannot read property 'once' of undefined
    at HTTP2ClientRequest.request.once.socket (/Users/endriu/Developer/test/node_modules/@szmarczak/http-timer/source/index.js:58:10)
    at Object.onceWrapper (events.js:273:13)
    at HTTP2ClientRequest.emit (events.js:187:15)
    at HTTP2ClientRequest.EventEmitter.emit (domain.js:442:20)
    at HTTP2ClientRequest.origin.emit.args [as emit] (/Users/endriu/Developer/test/node_modules/@szmarczak/http-timer/source/index.js:37:11)
    at HTTP2ClientRequest.process.nextTick (/Users/endriu/Developer/test/node_modules/http2-wrapper/source/client-request.js:112:31)
    at process._tickCallback (internal/process/next_tick.js:61:11)
[nodemon] app crashed - waiting for file changes before starting...

[leodutra]

How is the support for HTTP2?
Stable?

[szmarczak]

It's 99.99% stable, but the PR has not been merged yet due to a servername Node.js bug.
The workaround would be to create a Promise to check every 1ms if the servername is not false, but that doesn't look right.

[leodutra]

@szmarczak absolutely. It smells like no tests were done for that result.

[szmarczak]

@leodutra You're right. I've looked it Node tests right now and it seems they do check false servername, but only for the server-side.

[issuehunt-oss]

@sindresorhus has rewarded $180.00 to @szmarczak. See it on IssueHunt

• 💰 Total deposit: $200.00
• 🎉 Repository reward(0%): $0.00
• 🔧 Service fee(10%): $20.00