net: give better error messages

[evanlucas]

Add address and/or port to errors where applicable for better reporting.

See nodejs/node-v0.x-archive#7005 and #16

[jonathanong]

uhhhhh

[evanlucas]

@jonathanong Good point. Removed.

[MauriceButler]

Thanks @evanlucas That 1 less thing I have to do this weekend. 😄

I had a brief look yesterday and there were other areas that used this same error logic, Socket being one of them.

Have you had a look through there as well?

[evanlucas]

I have not. I would be more than happy to though.

[rvagg]

It's unfortunate that we don't have a solid perf suite yet to assess these kinds of changes!

[evanlucas]

For sure. That was a concern when I had opened a PR on the node repo.

[indutny]

I think this whole chunk could be moved to a function.

[indutny]

@evanlucas looks really promising! Left some comments.

@rvagg I'm not sure how this even could be benchmarked, and what would these numbers mean. But technically it should be possible.

@evanlucas do you think you may create some benchmark that will simulate EPIPE or whatever in a large quantities?

[evanlucas]

Let me see what I can come up with on the benchmark

[bnoordhuis]

Using simple string concatenation here should be significantly faster than calling util.format(). I/O errors are the exceptional case, of course, but in a large scale applications, they probably happen frequently enough to merit optimizing for.

[evanlucas]

Ok, I broke it out into a function and am now setting the properties in code. The syscall is now only called in the event an error actually occurs and I went ahead and fixed up the tests as requested. As far as the benchmark goes, would that need to go in the benchmark directory? I have not been able to run the current net benchmark tests...they just hang, so wasn't sure exactly where you wanted me to put the new benchmark test. Thanks!

[xaka]

Did you mean localAddress and localPort because of bind(localAddress, localPort)? You're not passing address and port variables to that call.

[evanlucas]

Ok, went ahead and updated those tests. I also fixed the error message to include the local port and address on bind error as pointed out by @xaka. Thanks!

[bnoordhuis]

LGTM. Can you squash it into a single commit with a nice commit log? If you haven't seen it, CONTRIBUTING.md has an example of a good commit log.

Can I get one more LGTM from another committer? Thanks.

[evanlucas]

I squashed it into one commit with a better commit log. Thanks