Document error handling in http2 client connect example

[jsha]

• Version: v8.7.0
• Platform: Linux membrane 4.4.0-97-generic Embeddable concept #120-Ubuntu SMP Tue Sep 19 17:28:18 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
• Subsystem: http2

Steps to reproduce:

Run the below program with node --expose_http2 sample.js

Expected result:

An error with ECONNREFUSED is thrown.

Actual result:

Program exits with no output.

var http2 = require('http2');
try {
  const client = http2.connect('https://localhost:12345');

  const req = client.request({ ':path': '/foo' });

  req.on('response', (headers) => {
    console.log(headers[':status']);
    console.log(headers['date']);
  });

  let data = '';
  req.setEncoding('utf8');
  req.on('data', (d) => data += d);
  req.on('end', () => client.destroy());
  req.on('error', (err) => console.error(err));
  req.end();
} catch(e) {
  console.error(e);
}

[jasnell]

that won't throw. Try adding a client.on('error', ...) handler

[jsha]

Isn't that a race, trying to add an error handler after calling connect?

[jasnell]

No. The underlying code is specifically designed to handle connection errors asynchronously. Attaching the handler after should work just fine

[jsha]

I added a client.on('error', ...), but it doesn't appear to fire:

var http2 = require('http2');
const client = http2.connect('https://localhost:12345');
client.on('error', (err) => {
  console.error(err);
});

Output:

$ node --expose_http2 sample.js 
(node:4905) ExperimentalWarning: The http2 module is an experimental API.

[apapirovski]

Socket errors are actually emitted on socketError. So you would need to do client.on('socketError', (err) => console.log(err)); — I just double checked and it does log an ECONNREFUSED error.

Here's the documentation for it: https://nodejs.org/dist/latest-v8.x/docs/api/http2.html#http2_event_socketerror

[jasnell]

Ha! Sigh that's right. Thank you for the correction @apapirovski!

[jsha]

Thanks, that worked! I updated the issue title to suggest updating the example code for client usage to reflect handling error and socketError.

As general feedback: I spent a while reading the http2 documentation, and to me it wasn't clear which methods apply to client usage vs server usage. The impression I'm starting to come to is that the http2 API is much more "symmetric" than the http API, in that lots of the events apply to both sides. Is that accurate? If so, the documentation might benefit from explaining this up front, and/or documenting for each event whether it can be thrown by clients or servers.

Thanks,
Jacob

[apapirovski]

I would say that's correct, yes — both the client and the server use the same basic building blocks in Http2Session & Http2Stream. The session is almost identical between client & server. There are a few more differences for the stream which you can see here:

https://nodejs.org/dist/latest-v8.x/docs/api/http2.html#http2_class_clienthttp2stream
https://nodejs.org/dist/latest-v8.x/docs/api/http2.html#http2_class_serverhttp2stream

In general, we are VERY happy to accept PRs that improve the docs. If you would like to add an example illustrating error handling or expand the existing example, that would be amazing!

[jsha]

Done: #16366.

Another bit of clarification: Is the compatibility API intended to include client-side code? I tried to port some existing code that used the https module, but it failed calling req.pipe(), because there was no pipe method available.

[apapirovski]

The compatibility layer is strictly server side at the moment. For the client-side, I don't know if there's any plan to make it fully h1 compatible but — as far as I understand — we at the very least want to add something similar to the h1 Agent model.

I'm surprised pipe didn't work though. @ronag did a lot of work with http2 when building node-http2-proxy and I'm reasonably certain that's using pipe a bunch. See the source here https://github.com/nxtedition/node-http2-proxy/blob/master/index.js