Close outgoing socket (fixes #559)

[glasser]

Note that this includes #560.

[jcrugzz]

We should also call onOutgoingError here since ending the proxySocket won't trigger its error event in anyway.

[jcrugzz]

@glasser besides my comment, LGTM. Good find!

[glasser]

Ok, in that case it probably shouldn't be called onOutgoingError. What should the event's args be? Right now the error event always gets the incoming socket as an arg even though the error is on the outgoing one. Should there be some way for the event to indicate which socket had the error?

[jcrugzz]

@glasser Well lets think of what we can do in these failure cases. Correct me if I'm wrong here, but in the case where the socket errors, we cannot re-proxy with that same socket. So it would make sense to have a different error function in that case that just returns the request if thats even useful? Cause it seems the connection is just FUBAR if we hit that case and we just need to kill the things and let someone know. So I'm thinking we may even want to destroy the socket when the socket errors as well if my assumption here is correct

thoughts @yawnt?

[glasser]

I don't think it is necessary to destroy a socket that has emitted error, from my limited understanding of how Socket works. Look in Node's net.js for instances of emitting error on a Socket. In v0.10 there are 3 (ignore the ones that are on Server instead). One is inside _destroy, one is immediately before a call to _destroy, and one is in a callback that is only placed on the Socket immediately before a call to destroySoon. So by the time you get this error it should already be on the path to destruction.

As to how to differentiate, seems like a design choice for the module maintainers... should there be lots of different error events with different names (which is what caronte seemed to originally have when it was using EventEmitter2) or a single error event with different parameters?

[jcrugzz]

@glasser I'm going to cherry pick part of this since the other PR was merged. And until I have a better idea of how the errors should be handled from messing around with the fail cases, I think we'll just emit the error event without a socket in the case that it errors since it will of course be dead in that case.

[jcrugzz]

Hmm actually I recant this, I don't believe we can really do anything if the client's socket errors so we just have to wait for them to reconnect. Emitting an error is actually not useful so your solution seems like the best course. The only reason I see for emitting some kind of event is for logging purposes. It should not be under error. Cherry picked your commit 4c3ba74