Skip to content

net: improve network family autoselection handle handling - #48464

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
ShogunPanda:network-selection-handle-improvements
Jun 27, 2023
Merged

net: improve network family autoselection handle handling#48464
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
ShogunPanda:network-selection-handle-improvements

Conversation

@ShogunPanda

Copy link
Copy Markdown
Contributor

This PR fixes network family autoselections in three area:

  1. Moves handle swapping before trying a new connection, as suggested in npm ERR! Exit handler never called! npm/cli#6409 (comment)
  2. Avoids spawning two different connection attempts, which could happen if a TLSSocket with a manually set backing Socket was used. See: PSA: If you're on node 20.0.0 and above, and want to use this library with client.configuration.connection.secure === true, use the argument --no-network-family-autoselection KararTY/dank-twitch-irc#13 (comment)
  3. Correctly handles AbortSignal when using network family auto selection.

Fixes: npm/cli#6409
Fixes: KararTY/dank-twitch-irc#13
Fixes: #47644

CC: @tniessen@silverwind

@ShogunPandaShogunPanda added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 15, 2023
@nodejs-github-botnodejs-github-bot added needs-ci PRs that need a full CI run. net Issues and PRs related to the net subsystem. labels Jun 15, 2023
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 15, 2023
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@mcollinamcollina 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.

lgtm

@ShogunPandaShogunPanda added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 22, 2023
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 22, 2023
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@ShogunPanda

Copy link
Copy Markdown
ContributorAuthor

I'll check the failing test locally this evening and then I'll update and finally merge this.

@ShogunPanda
ShogunPandaforce-pushed the network-selection-handle-improvements branch from 5388aa7 to 16ab933CompareJune 26, 2023 07:33
@ShogunPandaShogunPanda added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 26, 2023
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 26, 2023
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@ShogunPandaShogunPanda 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 Jun 27, 2023
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 27, 2023
@nodejs-github-bot
nodejs-github-bot merged commit fddd3ff into nodejs:mainJun 27, 2023
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in fddd3ff

@ShogunPanda
ShogunPanda deleted the network-selection-handle-improvements branch June 27, 2023 11:17
RafaelGSS pushed a commit that referenced this pull request Jul 3, 2023
PR-URL: #48464Fixes: npm/cli#6409Fixes: KararTY/dank-twitch-irc#13Fixes: #47644
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
@RafaelGSSRafaelGSS mentioned this pull request Jul 3, 2023
hail2u added a commit to hail2u/hail2u.net that referenced this pull request Jul 16, 2023
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
PR-URL: nodejs#48464Fixes: npm/cli#6409Fixes: KararTY/dank-twitch-irc#13Fixes: nodejs#47644
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
PR-URL: nodejs#48464Fixes: npm/cli#6409Fixes: KararTY/dank-twitch-irc#13Fixes: nodejs#47644
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
@ruyadorno

Copy link
Copy Markdown
Member

This commit does not land cleanly on v18.x-staging and will need manual backport in case we want it in v18.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

6 participants

@ShogunPanda@nodejs-github-bot@ruyadorno@mcollina@benjamingr@marco-ippolito