net.Socket.on('close', function(had_err) {}) doesn't set error in the case of TCP RST's

[mcavage]

Which is fairly unexpected. Code that explicitly disables this is here:

https://github.com/joyent/node/blob/f9fec3a2d65580b7e39edc9afd5904cd4775c87c/lib/net.js#L634-638

[nfitch]

Turns out I filed a duplicate here:
#4692

Closed out now. Pasting in the helpful bits from the other issue:

1. Node client opens connection to server, makes request.
2. Server starts sending data.
3. Client stops reading from connection.
4. Server times client out.
5. Client wakes back up, requests more data from server.
6. Server sends RST.
7. Node emits 'end' with no indication that server send RST.

Repro code can be found in this gist (thanks to issacs):
https://gist.github.com/4667685

Node should either emit an 'error', or set net.Socket.errorEmitted to true. And/or do what Mark suggests above.

[isaacs]

Yeah, this sucks. Of course, a reset is not properly an "error" that should throw, but just something that your app should handle, but it's impossible to handle it if you don't know that it was even reset.

[mcavage]

FWIW, I think a TCP reset actually does warrant an error event in the node sense - having the other end abruptly tear you down is generally different semantics than a graceful close. Most wrappers we use attempt to turn had_err into an Error and emit an error event for this reason.

[isaacs]

@bnoordhuis this change seems OK, but it breaks a few tests. Updated on https://github.com/isaacs/node/compare/econnreset, if you'd be so kind as to review.

I do wonder if it's appropriate to emit('error') on this case, since it was never a throwable offense in the past. I suspect we might have some grumpy users. It's not too uncommon to just destroy connections in HTTP. It might be worth suppressing this error in socketErrorListener, but in such a way that it can still be inspected.