connection-related errors could provide more information

[davepacheco]

For example, when we get ConnectionTimeoutErrors, all we get is:

ConnectTimeoutError: connect timeout after 4000ms

There are a couple of issues with this:

• people who see this don't know what we were connecting to (e.g., the IP or hostname and port)
• people who see this don't know what we were trying to do (e.g., what request this was)
• Based on rawRequest in lib/HttpClient.js, the problem isn't necessarily that we couldn't establish a connection, but rather that we couldn't obtain a socket within the timeout. The bulk of that time could have been spent doing a DNS lookup, queued behind other requests in the HTTP agent for the same host, or establishing the TCP connection.

What about something more like:

failed to complete HTTP request PUT /foo/bar to 10.1.2.3 port 1234: could not obtain connected socket within 4000ms

verror is intended to help build up the message in different layers of the stack.

This may sound excessively pedantic, but we run into the queueing and DNS issues kind of a lot and people burn a lot of time on the wrong path thinking that a connect timeout was a timeout attempting to establish a TCP connection.

[yunong]

@DonutEspresso mind taking a look?

[DonutEspresso]

@davepacheco That's a great idea.

It's been on my TODO list to remove restify-error's context object implementation now that verror supports the same via the info object.

In lieu of refactoring restify-errors right now, though, the quickest thing is probably to use verror directly within this module to capture lower level errors and their context, and then use those verrors as a cause for the top level http error. The top level http errors come from restify-errors, which currently inherit from WError.

So the two gotchas with this approach would be:

1. Being WError, the top level http4xx/5xx message won't have the concatenated messages of all the lower level errors
2. You'll need a serializer for the verror info object. I don't know if that's landed in bunyan core yet, but if not, restify-errors exports a serializer with support for verror info object, restify-error context object, and MultiError (https://github.com/restify/errors/blob/master/lib/serializer.js). It's what we've been using in most of our projects so far.

Let me know what you think.

[davepacheco]

Today, we seem to be getting the ConnectTimeoutError straight out of this repo:

clients/lib/HttpClient.js

Lines 94 to 102 in d415994

	function ConnectTimeoutError(ms) {
	if (Error.captureStackTrace) {
	Error.captureStackTrace(this, ConnectTimeoutError);
	}
	this.message = 'connect timeout after ' + ms + 'ms';
	this.name = 'ConnectTimeoutError';
	}
	util.inherits(ConnectTimeoutError, Error);

clients/lib/HttpClient.js

Line 142 in d415994

var err = new ConnectTimeoutError(opts.connectTimeout);

rather than from restify-errors.

[DonutEspresso]

Oops, apologies for the brain fart, guess I haven't been in the code very recently. That actually kind of works out, we should be able to just VError directly then.

Unfortunately I won't have time to tackle this until probably a week or two down the road - but if it's pressing feel free to file a PR!

[davepacheco]

Thanks. No problem! I likely wouldn't get to it before then, and I wasn't sure how best to get that information out without duplicating the logic in Node.

[DonutEspresso]

I believe this is all addressed by #109. Feel free to let us know if that's not the case. We'll be releasing 2.x shortly.