When identifying connection type, allow ALPN to negotiate 1.1

SimonWoolf commented 5 years ago

Currently, when testing to see whether an https endpoint supports http2, the library makes a tls connection with ALPNProtocols: ['h2'] only.

The problem is that some tls endpoints that don't support http2 really don't like that; they just abruptly terminate the connection.

This change results in it using ALPNProtocols: ['h2', 'http/1.1'] when testing for http2 support. AFAICT this doesn't break anything - it's not like the code is taking a successful connection as an indication of http2, it's correctly checking socket.alpnProtocol. If the server does support http2, the result of the negotiation will be h2.

To reproduce, do the following:

require('http2-client').get('https://echo.ably.io/', res => console.log("done", res.httpVersion))

Before this change, that results in:

events.js:174
      throw er; // Unhandled 'error' event
      ^

Error: Client network socket disconnected before secure TLS connection was established
    at TLSSocket.onConnectEnd (_tls_wrap.js:1088:19)
    at Object.onceWrapper (events.js:277:13)
    at TLSSocket.emit (events.js:194:15)
    at endReadableNT (_stream_readable.js:1125:12)
    at process._tickCallback (internal/process/next_tick.js:63:19)
Emitted 'error' event at:
    at emitErrorNT (internal/streams/destroy.js:82:8)
    at process._tickCallback (internal/process/next_tick.js:63:19)

After this change, it results in:

done 1.1

coveralls commented 5 years ago

Pull Request Test Coverage Report for Build 60

1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
2 unchanged lines in 1 file lost coverage.
Overall coverage decreased (-0.3%) to 77.986%

Files with Coverage Reduction	New Missed Lines	%
lib/request.js	2	81.41%
<!--	Total:	2		-->

Totals
Change from base Build 48:	-0.3%
Covered Lines:	324
Relevant Lines:	383

💛 - Coveralls

coveralls commented 5 years ago

Pull Request Test Coverage Report for Build 60

1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
2 unchanged lines in 1 file lost coverage.
Overall coverage decreased (-0.3%) to 77.986%

Files with Coverage Reduction	New Missed Lines	%
lib/request.js	2	81.41%
<!--	Total:	2		-->

Totals
Change from base Build 48:	-0.3%
Covered Lines:	324
Relevant Lines:	383

hisco / http2-client