GitHub user jamesreggio opened a pull request:

    https://github.com/apache/thrift/pull/986

    THRIFT-3787 Fix Node.js Connection object error handling

    The `connected` property on `Connection` instances was not accurately 
maintained if reconnection retries are not enabled. Line 194 was moved to Line 
176 to fix this.
    
    Furthermore, reconnection retries are not possible with secure sockets, so 
this commit returns early in that case, preventing long delays waiting for the 
`secureConnect` event which will never fire again. It's not explicitly stated 
in the Node.js documentation, but calling `connect()` on an instance of a 
`TLSSocket` will only reconnect the underlying TCP connection; it is not 
possible to cause the socket to re-handshake at the TLS layer.
    
    (You can peruse the source for 
[`tls.connect`](https://github.com/nodejs/node/blob/a11d506decd2820221d6cd770990a585ca0261d5/lib/_tls_wrap.js#L964-1095)
 to see that it performs many more operations than just instantiating a 
`TLSSocket` instance and calling `connect()` on it. This stands in contrast to 
an ordinary TCP `Socket` instance from the `net` module, where reconnection is 
possible via these means.)
    
    Supporting reconnection for secure sockets would require a deep refactor to 
enable the Thrift `Connection` instance to replace its underlying socket, and I 
didn't feel up to the challenge today.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/jamesreggio/thrift THRIFT-3787

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/thrift/pull/986.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #986
    
----
commit 449b1d711f91a9252b64351a71e44945e4432911
Author: James Reggio <[email protected]>
Date:   2016-04-13T23:33:40Z

    THRIFT-3787 Fix Node.js Connection object error handling
    
    The `connected` property on a Connection instances was not accurately
    maintained if reconnection retries are not enabled.
    
    Furthermore, reconnection retries are not possible with secure sockets,
    so this commit returns early in that case, preventing long delays.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---

Reply via email to