fix(socketDelay): support options.timeout #1848
Conversation
The current implementation only supports `req.setTimeout`, but Node.js allows for `options.timeout` to accomplish the same. https://nodejs.org/docs/latest-v12.x/api/http.html#http_http_request_url_options_callback
|
This looks good to me! I'll leave it open a couple more days in case anyone else wants to comment. |
| const onEnd = sinon.spy() | ||
|
|
||
| const req = http.request('http://example.test', { timeout: 5000 }, res => { | ||
| res.setEncoding('utf8') |
There was a problem hiding this comment.
Does this setEncoding have an affect on the test?
There was a problem hiding this comment.
I don't know to be fully honest. I copied the test case from above and made the minimal number of changes to make it work for options.timeout. I can take a look into it and adjust it for both tests cases if needed?
There was a problem hiding this comment.
@mastermatt any thoughts?
|
This is failing on Node 8 because of the lock file issues we're having. Once that's fixed up and master is merged in again, this should be good to go. |
|
🎉 This PR is included in version 11.8.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
The current implementation only supports
req.setTimeout, but Node.jsallows for
options.timeoutto accomplish the same.https://nodejs.org/docs/latest-v12.x/api/http.html#http_http_request_url_options_callback