-
Notifications
You must be signed in to change notification settings - Fork 29.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: handle errors correctly in test-gc-http-client-timeout.js #22185
Conversation
@@ -39,7 +39,9 @@ function getall() { | |||
pathname: '/', | |||
port: server.address().port | |||
}, cb); | |||
req.on('error', cb); | |||
req.on('error', (err) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not having this listener would result in the same outcome, I believe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed.
Run again on linuxone: https://ci.nodejs.org/job/node-test-commit-linuxone/3775/ |
In test-gc-http-client-timeout.js, res.resume is not a function if error occurs. Remove the error handler. PR-URL: nodejs#22185 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Rich Trott <[email protected]>
Landed in 2e37d0f |
In test-gc-http-client-timeout.js, res.resume is not a function if error occurs. Remove the error handler. PR-URL: #22185 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Rich Trott <[email protected]>
Before this,
test-gc-http-client-timeout.js
logsres.resume is not a function
when errors are emitted.This test sometimes fails on my OSX because
getaddrinfo
may returnENOTFOUND
. (The actual uv error code isUV_EAI_NONAME
.)Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes