tls: make tls.connect() accept a timeout option#25517
Closed
Uh oh!
There was an error while loading. Please reload this page.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Collaborator
nodejs-github-bot commented Jan 15, 2019
@lpinca sadly an error occured when I tried to trigger a build :( |
cjihrig reviewed Jan 15, 2019
doc/api/tls.md Outdated
Contributor
There was a problem hiding this comment.
https://github.com/nodejs/node/pull/25517. Just pointing it out because I know it's easy to forget.
MemberAuthor
There was a problem hiding this comment.
Yes will do just wasn't sure what id to use before submitting.
sam-github approved these changes Jan 15, 2019
doc/api/tls.md Outdated
Contributor
There was a problem hiding this comment.
maybe hyperlink to the setTimeout docs?
lpincaforce-pushed the add/timeout-option branch from ef94920 to cad7708CompareIf specified, and only when a socket is created internally, the option will make `socket.setTimeout()` to be called on the created socket with the given timeout. This is consistent with the `timeout` option of `net.connect()` and prevents the `timeout` option of the `https.Agent` from being ignored when a socket is created.
lpincaforce-pushed the add/timeout-option branch from cad7708 to 7c158bfCompare
silverwind approved these changes Jan 15, 2019
| lookup: options.lookup | ||
| }; | ||
| if(options.timeout){ |
Contributor
There was a problem hiding this comment.
typeof options.timeout === 'number' to allow setting a timeout of 0.
MemberAuthor
There was a problem hiding this comment.
0 == disabled idle timeout so I'm not sure if it's very useful. I mean it's like not setting it at all.
Contributor
There was a problem hiding this comment.
This code for tls.connect() is identical to that in net.connect(),
Lines 158 to 160 in 2c7f4f4
MemberAuthor
lpinca commented Jan 16, 2019
BridgeAR approved these changes Jan 16, 2019
MemberAuthor
lpinca commented Jan 20, 2019
MemberAuthor
lpinca commented Jan 20, 2019
Landed in aaa7547. |
lpinca added a commit that referenced this pull request Jan 20, 2019
If specified, and only when a socket is created internally, the option will make `socket.setTimeout()` to be called on the created socket with the given timeout. This is consistent with the `timeout` option of `net.connect()` and prevents the `timeout` option of the `https.Agent` from being ignored when a socket is created. PR-URL: #25517 Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Roman Reiss <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
addaleax pushed a commit that referenced this pull request Jan 23, 2019
If specified, and only when a socket is created internally, the option will make `socket.setTimeout()` to be called on the created socket with the given timeout. This is consistent with the `timeout` option of `net.connect()` and prevents the `timeout` option of the `https.Agent` from being ignored when a socket is created. PR-URL: #25517 Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Roman Reiss <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
addaleax added the semver-minor 
Merged
MylesBorins added a commit that referenced this pull request Jan 24, 2019
Notable Changes: * events: * For unhandled `error` events with an argument that is not an `Error` object, the resulting exeption will have more information about the argument. #25621 * child_process: * When the `maxBuffer` option is passed, `stdout` and `stderr` will be truncated rather than unavailable in case of an error. #24951 * policy: * Experimental support for module integrity checks through a manifest file is implemented now. #23834 * n-api: * The `napi_threadsafe_function` feature is now stable. #25556 * report: * An experimental diagnostic API for capturing process state is available as `process.report` and through command line flags. #22712 * tls: * `tls.connect()` takes a `timeout` option analogous to the `net.connect()` one. #25517 * worker: * `process.umask()` is available as a read-only function inside Worker threads now. #25526 * An `execArgv` option that supports a subset of Node.js command line options is supported now. #25467 PR-URL: #25687
MylesBorins added a commit that referenced this pull request Jan 25, 2019
Notable Changes: * events: * For unhandled `error` events with an argument that is not an `Error` object, the resulting exeption will have more information about the argument. #25621 * child_process: * When the `maxBuffer` option is passed, `stdout` and `stderr` will be truncated rather than unavailable in case of an error. #24951 * policy: * Experimental support for module integrity checks through a manifest file is implemented now. #23834 * n-api: * The `napi_threadsafe_function` feature is now stable. #25556 * report: * An experimental diagnostic API for capturing process state is available as `process.report` and through command line flags. #22712 * tls: * `tls.connect()` takes a `timeout` option analogous to the `net.connect()` one. #25517 * worker: * `process.umask()` is available as a read-only function inside Worker threads now. #25526 * An `execArgv` option that supports a subset of Node.js command line options is supported now. #25467 PR-URL: #25687
1 task
BethGriggs pushed a commit that referenced this pull request Apr 29, 2019
If specified, and only when a socket is created internally, the option will make `socket.setTimeout()` to be called on the created socket with the given timeout. This is consistent with the `timeout` option of `net.connect()` and prevents the `timeout` option of the `https.Agent` from being ignored when a socket is created. PR-URL: #25517 Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Roman Reiss <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
Merged
BethGriggs pushed a commit that referenced this pull request May 10, 2019
If specified, and only when a socket is created internally, the option will make `socket.setTimeout()` to be called on the created socket with the given timeout. This is consistent with the `timeout` option of `net.connect()` and prevents the `timeout` option of the `https.Agent` from being ignored when a socket is created. PR-URL: #25517 Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Roman Reiss <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
MylesBorins pushed a commit that referenced this pull request May 16, 2019
If specified, and only when a socket is created internally, the option will make `socket.setTimeout()` to be called on the created socket with the given timeout. This is consistent with the `timeout` option of `net.connect()` and prevents the `timeout` option of the `https.Agent` from being ignored when a socket is created. PR-URL: #25517 Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Roman Reiss <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
This was referenced May 29, 2019
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
If specified, and only when a socket is created internally, the option
will make
socket.setTimeout()to be called on the created socket withthe given timeout.
This is consistent with the
timeoutoption ofnet.connect()andprevents the
timeoutoption of thehttps.Agentfrom being ignoredwhen a socket is created.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes