[
https://issues.apache.org/jira/browse/THRIFT-3787?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15240260#comment-15240260
]
ASF GitHub Bot commented on THRIFT-3787:
----------------------------------------
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.
----
> Node.js Connection object doesn't handle errors correctly
> ---------------------------------------------------------
>
> Key: THRIFT-3787
> URL: https://issues.apache.org/jira/browse/THRIFT-3787
> Project: Thrift
> Issue Type: Bug
> Components: Node.js - Library
> Reporter: James Reggio
> Priority: Minor
>
> There are a handful of operation-ordering problems in the
> Connection.prototype.connection_gone() method and its friends.
> See the pull request for more details.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
