net: fix family autoselection timeout handling.#47860
Conversation
|
Review requested:
|
28763f1 to
28d9306
Compare
tniessen
left a comment
There was a problem hiding this comment.
As far as I can tell from https://github.com/nodejs/reliability, tests such as test-x509-escaping started being flaky with ERR_SOCKET_CONNECTION_TIMEOUT from within internalConnectMultiple in the first half of April, shortly after #46790 was merged. Is this PR a fix for those flaky tests, too?
|
@tniessen I haven't checked specifically, but I think so. |
Co-authored-by: Tobias NieΓen <tniessen@tnie.de>
|
Landed in 2d24b29 |
PR-URL: #47860 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
|
This seems to introduce a crash in some cases: #48000 |
|
I'll exclude this change from v20.2.0 while it's being investigated. |
|
Thanks @targos. I'll take a look soon |
β¦n a released version yet: nodejs/node#47860
|
See my update here. I think there might be additional fixes required (moving this block to here). |
PR-URL: #47860 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
PR-URL: nodejs#47860 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
PR-URL: nodejs#47860 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
PR-URL: nodejs#47860 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
PR-URL: nodejs#47860 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
PR-URL: nodejs#47860 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
PR-URL: nodejs#47860 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
PR-URL: nodejs#47860 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
This PR fixes timeout handling of the family autoselection.
@nodejs/net @nodejs/tsc To ensure better user experience, I disabled the attempt timeout when connecting to the last IP address available, otherwise each connection attempt would be bound to the same (pretty low) timeout.
Do you think this is a reasonable approach? Also, is this considered a semver-major change? I think people are affected by this so we might want to include in 20.x anyway.
Fixes #47822.