Connection.js: socket needs to be set up in the case of tls#472
Connection.js: socket needs to be set up in the case of tls#472jhansen-tt wants to merge 1 commit intomscdex:masterfrom
Conversation
…ls for all of the event handlers to work properly.
|
Hmmm.... This might explain some weirdness I've been seeing since moving to node 0.12 against gmail? |
|
It explains a lot of weirdness :) |
|
We should probably make sure it still works for node v0.10 though and I remember some time back (not sure if it was 0.8 or 0.10) there were some events that were not propogated to the cleartext stream object and were only visible on the original socket. That is why it's currently coded the way it is. If this change works (including error handling) on the latest node v0.10 release, then I have no problem merging it. |
|
I've only tested this on 0.12, but it works great there. |
|
This fixed my issue not being able to catch errors on TLS connect (like connection refused or host not found on an itinerant connection). I originally hacked it locally by adding socket.on('error', _onError) but this is much cleaner and fixed other problems. I can't find the docs in node's TLS module that say you're allowed to supply your own socket to connect at all. I'm using 0.12. |
|
@evarsanyi It's the |
|
Thanks, and thanks for the fix -- no more server crashes on transient DNS errors. |
|
This going to get merged? |
Otherwise the event handlers are set up on the wrong socket object.