GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — net: add autoSelectFamily option by ShogunPanda · Pull Request #44731 · nodejs/node

Conversation

@ShogunPanda

@ShogunPanda ShogunPanda commented Sep 20, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

This PR loosely implements section 5 of RFC 8305 (Happy Eyeballs algorithm).

A new option autoSelectFamily is added to net.connect.
When set to a positive number (or true), the lookup phase will keep all records by setting all=true.
A connection attempt will be tried to all AAAA and A records (alternating families), in sequence, giving each connection autoSelectFamily milliseconds to be established.
Errors are raised only if no connection succeeded.

Fixes #41625.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

miyurus-xino, KhafraDev, xtx1130, oneitss, metcoder95, bricss, nephix, and devwatts reacted with thumbs up emoji
All reactions
  • 👍 8 reactions
@ShogunPanda
ShogunPanda requested review from addaleax, aduh95, mcollina and ronag September 20, 2022 12:31
@nodejs-github-bot

nodejs-github-bot commented Sep 20, 2022

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/net
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot nodejs-github-bot added errors Issues and PRs related to JavaScript errors originating in Node.js core. needs-ci PRs that need a full CI run. net Issues and PRs related to the net subsystem. labels Sep 20, 2022
@ShogunPanda
ShogunPanda force-pushed the happy-eyeballs branch from 5a35dae to 8e79310 Compare September 20, 2022 12:32
mcollina
mcollina reviewed Sep 20, 2022
View reviewed changes

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

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.

Can you please add a test for HTTPs too?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread doc/api/net.md Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread deps/llhttp/CMakeLists.txt Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread lib/net.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

@mcollina mcollina added notable-change PRs with changes that should be highlighted in changelogs. semver-minor PRs that contain new features and should be released in the next minor version. labels Sep 20, 2022
@ShogunPanda
ShogunPanda force-pushed the happy-eyeballs branch from 8e79310 to c301ed9 Compare September 20, 2022 12:37
@mcollina

mcollina commented Sep 20, 2022

Copy link
Copy Markdown
Member

@nodejs/tsc this is a notable change that we should land before v18 goes LTS. We have seen quite a few reports of bugs due to improper configuration of IPv6, making #41625 a necessity. For example, Docker on Mac only exposes ports on the IPv4 address, making all connections to localhost fail.

GeoffreyBooth, treysis, and styfle reacted with thumbs up emoji
All reactions
  • 👍 3 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

aduh95
aduh95 reviewed Sep 20, 2022
View reviewed changes
Comment thread lib/internal/errors.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread lib/net.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread lib/net.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread lib/net.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread lib/net.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread lib/net.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread lib/net.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread lib/net.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread lib/net.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

@Trott

Trott commented Sep 21, 2022

Copy link
Copy Markdown
Member

No comment (yet at least) one way or the other on the implementation, but +1000 to implementing happy eyeballs in Node.js.

RaisinTen and tniessen reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mcollina mcollina added the tsc-agenda Issues and PRs to discuss during Technical Steering Committee meetings. label Sep 21, 2022
@richardlau

richardlau commented Sep 21, 2022

Copy link
Copy Markdown
Member

The two new tests fail for me (Linux x64) and also in GitHub actions.

Details
=== release test-http-happy-eyeballs ===
Path: parallel/test-http-happy-eyeballs
Error: --- stderr ---
node:events:491
      throw er; // Unhandled 'error' event
      ^

Error: listen EADDRINUSE: address already in use :::33167
    at Server.setupListenHandle [as _listen2] (node:net:1649:16)
    at listenInCluster (node:net:1697:12)
    at doListen (node:net:1846:7)
    at process.processTicksAndRejections (node:internal/process/task_queues:83:21)
Emitted 'error' event on Server instance at:
    at emitErrorNT (node:net:1676:8)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21) {
  code: 'EADDRINUSE',
  errno: -98,
  syscall: 'listen',
  address: '::',
  port: 33167
}

Node.js v19.0.0-pre
Command: out/Release/node /home/runner/work/node/node/test/parallel/test-http-happy-eyeballs.js
=== release test-net-happy-eyeballs ===
Path: parallel/test-net-happy-eyeballs
Error: --- stderr ---
node:events:491
      throw er; // Unhandled 'error' event
      ^

Error: listen EADDRINUSE: address already in use :::37301
    at Server.setupListenHandle [as _listen2] (node:net:1649:16)
    at listenInCluster (node:net:1697:12)
    at doListen (node:net:1846:7)
    at process.processTicksAndRejections (node:internal/process/task_queues:83:21)
Emitted 'error' event on Server instance at:
    at emitErrorNT (node:net:1676:8)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21) {
  code: 'EADDRINUSE',
  errno: -98,
  syscall: 'listen',
  address: '::',
  port: 37301
}

Node.js v19.0.0-pre
Command: out/Release/node /home/runner/work/node/node/test/parallel/test-net-happy-eyeballs.js

===
=== 2 tests failed
===
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ShogunPanda

ShogunPanda commented Sep 21, 2022

Copy link
Copy Markdown
Contributor Author

Acknowledged!
I'm taking care of them locally.

richardlau reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mhdawson

mhdawson commented Sep 21, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

No comment (yet at least) one way or the other on the implementation, but +1000 to implementing happy eyeballs in Node.js.

Same goes for me as well.

GeoffreyBooth and tniessen reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ShogunPanda
ShogunPanda force-pushed the happy-eyeballs branch 2 times, most recently from 617860b to b1c7de5 Compare September 23, 2022 21:09
aduh95
aduh95 reviewed Sep 24, 2022
View reviewed changes

@aduh95 aduh95 left a comment

Copy link
Copy Markdown
Contributor

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.

Can we add test for dnsPromises.Resolver as well?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread lib/_tls_wrap.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread test/common/index.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

@KhafraDev KhafraDev mentioned this pull request Sep 25, 2022
feat(rest): use Agent with higher connect timeout discordjs/discord.js#8679
Merged
@mhdawson mhdawson mentioned this pull request Sep 26, 2022
Node.js Technical Steering Committee (TSC) Meeting 2022-09-28 nodejs/TSC#1287
Closed
@Trott

Trott commented Sep 26, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

we should land before v18 goes LTS

@nodejs/tsc Two comments.

  1. If I understand correctly, this is a breaking change. It should be semver major. EDIT: Oh, wait, this is an option that is off by default? So people would need to opt in to get the fix? Oh, then I can't imagine this is controversial at all. Yes, let's get this working and landed! Woo hoo! 🎉 🚀
  2. However...I am in favor of landing this in the 18.x line as a special case of an important bug fix. Specifically:

Docker on Mac only exposes ports on the IPv4 address, making all connections to localhost fail.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Trott

Trott commented Sep 26, 2022

Copy link
Copy Markdown
Member

I'm going to rebase and force push to at least get the tarball job working.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Trott
Trott force-pushed the happy-eyeballs branch from b1c7de5 to 25f2336 Compare September 26, 2022 20:38
@Trott Trott added the commit-queue-squash PRs the Commit Queue should land as one squashed commit. label Sep 26, 2022
@mcollina

mcollina commented Sep 27, 2022

Copy link
Copy Markdown
Member

Oh, wait, this is an option that is off by default? So people would need to opt in to get the fix?

My case is that we should make it enabled by default. This causes so many headaches to newbie developers that know little of IPv4 vs IPv6 and why localhost is two IPs.

We should make this change before v18 goes LTS.

treysis, styfle, and fhanggi reacted with thumbs up emoji
All reactions
  • 👍 3 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mzz2017 mzz2017 mentioned this pull request Jun 19, 2023
Happyeyeballs is not fully supported by some softwares daeuniverse/dae#152
Closed
@federicobadini federicobadini mentioned this pull request Jul 3, 2023
Upgrade to node 18 iFixit/react-commerce#1779
Merged
@chinthakagodawita chinthakagodawita mentioned this pull request Jul 11, 2023
[@types/node] update 'net.LookupFunction' to reflect that it looks up all addresses instead of one DefinitelyTyped/DefinitelyTyped#66031
Merged
7 of 8 tasks
This was referenced Jul 11, 2023
Infrastructure: Update localhost references for regression tests w3c/aria-practices#2749
Merged
Infrastructure: Remove temp pinning to Node 16 to work around tests failing with Node 18 in CI w3c/aria-practices#2636
Closed
@oscrx oscrx mentioned this pull request Aug 4, 2023
[BUG] npm install started to hang on "still idealTree buildDeps" for 60 seconds if there are dependencies pointing to specific gitlab repository url (a regression compared to npm v6) npm/cli#3359
Open
1 task done
vidddd1
vidddd1 reviewed Sep 3, 2023
View reviewed changes
Comment thread doc/api/net.md

[IPC]: #ipc-support
[Identifying paths for IPC connections]: #identifying-paths-for-ipc-connections
[RFC 8305]: https://www.rfc-editor.org/rfc/rfc8305.txt

This comment was marked as spam.

Sign in to view

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Comment thread doc/api/net.md

[IPC]: #ipc-support
[Identifying paths for IPC connections]: #identifying-paths-for-ipc-connections
[RFC 8305]: https://www.rfc-editor.org/rfc/rfc8305.txt

This comment was marked as spam.

Sign in to view

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vidddd1
vidddd1 reviewed Sep 3, 2023
View reviewed changes

@vidddd1 vidddd1 left a comment

Copy link
Copy Markdown

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@vidddd1

This comment was marked as spam.

Sign in to view
@vidddd1

This comment was marked as spam.

Sign in to view
@acmacalister acmacalister mentioned this pull request Sep 19, 2023
feat: add autoSelectFamily for happy eyeballs support dwmkerr/wait-port#99
Merged
@saiarcot895 saiarcot895 mentioned this pull request Oct 19, 2023
[Bug] On a dual-stack network, ping for a single-stack (IPv4 or IPv6 only) service may fail gethomepage/homepage#2211
Closed
3 tasks done
@scotttrinh scotttrinh mentioned this pull request Nov 8, 2023
Fix auth urls geldata/gel-js#769
Merged
@skyd0me skyd0me mentioned this pull request Nov 14, 2023
Localhost usebruno/bruno#124
Closed
This was referenced Nov 21, 2023
ERR_INTERNAL_ASSERTION #50754
Closed
Node js internal assertion error in socket #50841
Closed
This was referenced Nov 24, 2023
An unknown error [ERR_INTERNAL_ASSERTION]: This is caused by either a bug in Node.js or incorrect usage of Node.js internals. #50893
Closed
ERR_INTERNAL_ASSERTION #50895
Closed
This was referenced Dec 2, 2023
ERR_INTERNAL_ASSERTION #51015
Closed
Nodejs 20.9 Internal Assertion Error #50765
Closed
Bug: Node.js ERR_INTERNAL_ASSERTION error when updating and saving content with Strapi [v4.15.0] #50436
Closed
jest test runner throws node:internal/assert:14 throw new ERR_INTERNAL_ASSERTION(message); #50383
Closed
@chinthakagodawita chinthakagodawita mentioned this pull request Jan 18, 2024
net: Support lookup of one address in net.LookupFunction (autoSelectFamily: false) DefinitelyTyped/DefinitelyTyped#68213
Merged
7 tasks done
@hainenber hainenber mentioned this pull request May 14, 2024
chore(build): uplift webpack-related packages to v5, 2nd attempt apache/superset#28440
Closed
9 tasks
@kt3k kt3k mentioned this pull request Oct 30, 2024
ext/node: implement autoSelectFamily option in net.connect denoland/deno#26641
Closed
@gauthier-th gauthier-th mentioned this pull request Jan 16, 2025
feat(settings): add settings for custom DNS servers and IPv4 resolution first seerr-team/seerr#1266
Merged
2 of 3 tasks
This was referenced Aug 7, 2026
Módulo de monitorías: plan de trabajo, evidencias y certificado de horas jorgejc/nodosys#2
Merged
Reduce platform and data usage waste monicaestimon-ctrl/louiemae2#76
Merged
@coderabbitai coderabbitai Bot mentioned this pull request Aug 18, 2026
fix(api): retry mailserver connections after DNS changes openagentemail/openagentemail#49
Merged
@coderabbitai coderabbitai Bot mentioned this pull request Sep 23, 2026
Feature/app completion IEEE-CU/SB-APP#15
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tniessen tniessen tniessen left review comments
@aduh95 aduh95 aduh95 approved these changes
@mcollina mcollina mcollina approved these changes
@jasnell jasnell jasnell approved these changes
@anonrig anonrig anonrig approved these changes
@ronag ronag ronag approved these changes
@RafaelGSS RafaelGSS RafaelGSS approved these changes
@addaleax addaleax Awaiting requested review from addaleax
@RaisinTen RaisinTen Awaiting requested review from RaisinTen
@Trott Trott Awaiting requested review from Trott
+3 more reviewers
@apapirovski apapirovski apapirovski left review comments
@targos targos targos left review comments
@vidddd1 vidddd1 vidddd1 left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

commit-queue-squash PRs the Commit Queue should land as one squashed commit. errors Issues and PRs related to JavaScript errors originating in Node.js core. needs-ci PRs that need a full CI run. net Issues and PRs related to the net subsystem. notable-change PRs with changes that should be highlighted in changelogs. semver-minor PRs that contain new features and should be released in the next minor version.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Happy Eyeballs support (address IPv6 issues in Node 17)

Uh oh!

There was an error while loading. Please reload this page.

19 participants

@ShogunPanda @nodejs-github-bot @mcollina @Trott @richardlau @mhdawson @targos @GeoffreyBooth @tniessen @LunNova @aduh95 @RaisinTen @Moumouls @vidddd1 @apapirovski @jasnell @anonrig @ronag @RafaelGSS
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.