FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

net: fix family autoselection timeout handling. by ShogunPanda · Pull Request #47860 · nodejs/node · GitHub

/ node Public

net: fix family autoselection timeout handling. - #47860

Merged
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
ShogunPanda:autoselectfamily-timeout
May 11, 2023
ConversationCommits (2)ChecksFiles changed
Merged

net: fix family autoselection timeout handling.#47860
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
ShogunPanda:autoselectfamily-timeout

Conversation

ShogunPanda commented May 4, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

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.

nodejs-github-bot commented May 4, 2023

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/net

ShogunPanda added the request-ci Add this label to start a Jenkins CI on a PR. label May 4, 2023
nodejs-github-bot added needs-ci PRs that need a full CI run. net Issues and PRs related to the net subsystem. labels May 4, 2023
ShogunPanda force-pushed the autoselectfamily-timeout branch from 28763f1 to 28d9306 Compare May 4, 2023 12:50
ShogunPanda mentioned this pull request May 4, 2023
Merged
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 4, 2023

nodejs-github-bot commented May 4, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/51625/

nodejs-github-bot commented May 5, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/51650/

tniessen reviewed May 6, 2023
View reviewed changes

tniessen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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?

Comment thread test/parallel/test-net-autoselectfamily.js Outdated

ShogunPanda commented May 6, 2023

Copy link
Copy Markdown
Contributor Author

@tniessen I haven't checked specifically, but I think so.

net: update test typo
f8dda43 
Co-authored-by: Tobias Nießen <tniessen@tnie.de>
ShogunPanda mentioned this pull request May 9, 2023
Closed
ShogunPanda added the request-ci Add this label to start a Jenkins CI on a PR. label May 10, 2023
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 10, 2023

nodejs-github-bot commented May 10, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/51745/

mcollina approved these changes May 10, 2023
View reviewed changes

mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

lgtm

nodejs-github-bot commented May 10, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/51753/

github-actions Bot mentioned this pull request May 11, 2023
Open
24 tasks

nodejs-github-bot commented May 11, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/51758/

ShogunPanda added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels May 11, 2023
nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label May 11, 2023
nodejs-github-bot merged commit 2d24b29 into nodejs:main May 11, 2023

nodejs-github-bot commented May 11, 2023

Copy link
Copy Markdown
Collaborator

Landed in 2d24b29

ShogunPanda deleted the autoselectfamily-timeout branch May 11, 2023 22:47
github-actions Bot mentioned this pull request May 12, 2023
Open
23 tasks
targos pushed a commit that referenced this pull request May 12, 2023
net: fix family autoselection timeout handling
09a649d 
PR-URL: #47860
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
This was referenced May 14, 2023
Open
Open

targos commented May 15, 2023

Copy link
Copy Markdown
Member

This seems to introduce a crash in some cases: #48000

targos commented May 15, 2023

Copy link
Copy Markdown
Member

I'll exclude this change from v20.2.0 while it's being investigated.

ShogunPanda commented May 15, 2023

Copy link
Copy Markdown
Contributor Author

Thanks @targos. I'll take a look soon

This was referenced May 16, 2023
Open
Open
AnnaDamm added a commit to AnnaDamm/scribblehub-to-epub that referenced this pull request May 18, 2023
add engine restrictions, due to a bug in node 20, that is not fixed i…
4ae14d4 
…n a released version yet: nodejs/node#47860
ShogunPanda mentioned this pull request May 23, 2023
Open
silverwind mentioned this pull request May 25, 2023
Closed
dharesign mentioned this pull request May 26, 2023
Closed

dharesign commented May 26, 2023

Copy link
Copy Markdown
Contributor

See my update here. I think there might be additional fixes required (moving this block to here).

targos pushed a commit that referenced this pull request Jun 4, 2023
net: fix family autoselection timeout handling
750e53c 
PR-URL: #47860
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
targos mentioned this pull request Jun 4, 2023
Merged
juanarbol pushed a commit to juanarbol/node that referenced this pull request Jul 17, 2023
net: fix family autoselection timeout handling
6ef172f 
PR-URL: nodejs#47860
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
mhdawson pushed a commit to mhdawson/io.js that referenced this pull request Jul 26, 2023
net: fix family autoselection timeout handling
b041a48 
PR-URL: nodejs#47860
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
mhdawson pushed a commit to mhdawson/io.js that referenced this pull request Aug 4, 2023
net: fix family autoselection timeout handling
c96ef05 
PR-URL: nodejs#47860
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
ruyadorno pushed a commit that referenced this pull request Aug 14, 2023
net: fix family autoselection timeout handling
e8289a8 
PR-URL: #47860
Backport-PR-URL: #49016
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
ruyadorno mentioned this pull request Aug 17, 2023
Merged
mcollina mentioned this pull request Sep 11, 2023
Closed
vweevers mentioned this pull request Jun 1, 2024
Open
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

tniessen tniessen left review comments
mcollina mcollina approved these changes
jasnell jasnell approved these changes

Assignees

No one assigned

Labels

commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. needs-ci PRs that need a full CI run. net Issues and PRs related to the net subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

v20.0.0 ERR_SOCKET_CONNECTION_TIMEOUT when sending http request to some domains

9 participants

Back | FazBrowse Home | New Git URL