net: add autoSelectFamily option #44731

ShogunPanda · 2022-09-20T12:31:46Z

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.

nodejs-github-bot · 2022-09-20T12:31:50Z

Review requested:

@nodejs/net

mcollina

Can you please add a test for HTTPs too?

doc/api/net.md

deps/llhttp/CMakeLists.txt

lib/net.js

mcollina · 2022-09-20T12:39:29Z

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

lib/internal/errors.js

lib/net.js

Trott · 2022-09-21T15:46:47Z

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

richardlau · 2022-09-21T16:19:04Z

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

=== 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
===

ShogunPanda · 2022-09-21T16:21:15Z

Acknowledged!
I'm taking care of them locally.

mhdawson · 2022-09-21T17:34:46Z

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.

aduh95

Can we add test for dnsPromises.Resolver as well?

lib/_tls_wrap.js

test/common/index.js

Trott · 2022-09-26T20:21:42Z

we should land before v18 goes LTS

@nodejs/tsc ~~Two comments.~~

~~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! 🎉 🚀
~~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.~~

Trott · 2022-09-26T20:38:02Z

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

mcollina · 2022-09-27T06:18:39Z

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.

Upgrade grunt-contrib-internal from 6 to 8, which updates CI from Node 10,12,14 to Node 14,16,18,20. Fix default connect() hostname to work on Node 18+, where otherwise CI fails as follows: > Running "nodeunit:tests" (nodeunit) task > Testing connect_test.jsFatal error: connect ECONNREFUSED ::1:8000 > Error: connect ECONNREFUSED ::1:8000 > at TCPConnectWrap.afterConnect [as oncomplete] (node:net:1494:16) Previously, grunt-contrib-connect was passing empty string or null down to `connect(,hostname)` to leave the default implicitly handled by `connect` and Node's built-in `http.Server` class. Then, later, it was doing a fallback to `0.0.0.0` but this only affected the CLI output text, not the value passed to `connect()`. Node 17+ changed per nodejs/node#40702 to prefer IPv6 loopback when available (`::`) instead of IPv4 loopback (`0.0.0.0`), unless `0.0.0.0` is explicitly passed. This was fixed in Node 20, with `http.get` underlying `net` TCP module implementing "Happy Eyeballs" (same as browsers and curl) which basically means trying both IPv6 and IPv4. nodejs/node#44731 The workaround is to call `dns.setDefaultResultOrder('ipv4first')` which ensures that the first try is over IPv4 instead of IPv6.

doc/api/net.md

@@ -1630,6 +1647,7 @@ net.isIPv6('fhqwhgads'); // returns false

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


doc/api/net.md

@@ -1630,6 +1647,7 @@ net.isIPv6('fhqwhgads'); // returns false

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


vidddd1

This commit upgrades Node.js version to v20.x in CI/CD environment. Previously used Node 18.x is moving towards end-of-life, with a planned date of 2025-04-30. In contrast, Node 20.x has been offering long-term support (LTS) since 2023-10-24. This makes Node 20.x a stable and recommended version for production environments. This commit also configures `actions/setup-node` with the `check-latest` flag to always use the latest Node 20.x version, keeping CI/CD setup up-to-date with minimal maintenance. Details: - actions/setup-node#165 - actions/setup-node#160 Using Node 20.x in CI/CD environments provides better compatibility with Electron v29.0 which moves to Node 20.x. Details: - electron/electron#40343 This upgrade improves network connection handling in CI/CD pipelines (where issues occur due to GitHub runners not supporting IPv6). Details: - actions/runner#3138 - actions/runner-images#668 - actions/runner#3213 - actions/runner-images#9540 Node 20.x adopts the Happy Eyeballs algorithm for improved IPv6 connectivity. - nodejs/node#40702 - nodejs/node#41625 - nodejs/node#44731 This mitigates issues like `UND_ERR_CONNECT_TIMEOUT` and localhost DNS resolution in CI/CD environments: Details: - nodejs/node#40537 - actions/runner#3213 - actions/runner-images#9540 Node 20 introduces `setDefaultAutoSelectFamily`, a global function from Node 19.4.0, enabling better IPv4 support, especially in environments with limited or problematic IPv6 support. Details: - nodejs/node#45777 Node 20.x defaults to the new `autoSelectFamily`, improving network connection reliability in GitHub runners lacking full IPv6 support. Details: - nodejs/node#46790

ShogunPanda requested review from mcollina, addaleax, ronag and aduh95 September 20, 2022 12:31

nodejs-github-bot added errors Issues and PRs related to JavaScript errors originated 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 force-pushed the happy-eyeballs branch from 5a35dae to 8e79310 Compare September 20, 2022 12:32

mcollina reviewed Sep 20, 2022

View reviewed changes

doc/api/net.md Outdated Show resolved Hide resolved

deps/llhttp/CMakeLists.txt Outdated Show resolved Hide resolved

lib/net.js Outdated Show resolved Hide resolved

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 force-pushed the happy-eyeballs branch from 8e79310 to c301ed9 Compare September 20, 2022 12:37

aduh95 reviewed Sep 20, 2022

View reviewed changes

mcollina added the tsc-agenda Issues and PRs to discuss during the meetings of the TSC. label Sep 21, 2022

ShogunPanda force-pushed the happy-eyeballs branch 2 times, most recently from 617860b to b1c7de5 Compare September 23, 2022 21:09

aduh95 reviewed Sep 24, 2022

View reviewed changes

lib/_tls_wrap.js Outdated Show resolved Hide resolved

test/common/index.js Outdated Show resolved Hide resolved

KhafraDev mentioned this pull request Sep 25, 2022

feat(rest): use Agent with higher connect timeout discordjs/discord.js#8679

Merged

mhdawson mentioned this pull request Sep 26, 2022

Node.js Technical Steering Committee (TSC) Meeting 2022-09-28 nodejs/TSC#1287

Closed

Trott force-pushed the happy-eyeballs branch from b1c7de5 to 25f2336 Compare September 26, 2022 20:38

Trott added the commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. label Sep 26, 2022

federicobadini mentioned this pull request Jul 3, 2023

Upgrade to node 18 iFixit/react-commerce#1779

Merged

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

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 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

vidddd1 reviewed Sep 3, 2023

View reviewed changes

This comment was marked as spam.

Sign in to view

acmacalister mentioned this pull request Sep 19, 2023

feat: add autoSelectFamily for happy eyeballs support dwmkerr/wait-port#99

Merged

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

scotttrinh mentioned this pull request Nov 8, 2023

Fix auth urls edgedb/edgedb-js#769

Merged

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

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

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 mentioned this pull request Oct 30, 2024

ext/node: implement autoSelectFamily option in net.connect denoland/deno#26641

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

net: add autoSelectFamily option #44731

net: add autoSelectFamily option #44731

ShogunPanda commented Sep 20, 2022 •

edited

Loading

nodejs-github-bot commented Sep 20, 2022

mcollina left a comment

mcollina commented Sep 20, 2022

Trott commented Sep 21, 2022

richardlau commented Sep 21, 2022

ShogunPanda commented Sep 21, 2022

mhdawson commented Sep 21, 2022 •

edited

Loading

aduh95 left a comment

Trott commented Sep 26, 2022 •

edited

Loading

Trott commented Sep 26, 2022

mcollina commented Sep 27, 2022

This comment was marked as spam.

This comment was marked as spam.

vidddd1 left a comment

This comment was marked as spam.

This comment was marked as spam.

net: add autoSelectFamily option #44731

net: add autoSelectFamily option #44731

Conversation

ShogunPanda commented Sep 20, 2022 • edited Loading

nodejs-github-bot commented Sep 20, 2022

mcollina left a comment

Choose a reason for hiding this comment

mcollina commented Sep 20, 2022

Trott commented Sep 21, 2022

richardlau commented Sep 21, 2022

ShogunPanda commented Sep 21, 2022

mhdawson commented Sep 21, 2022 • edited Loading

aduh95 left a comment

Choose a reason for hiding this comment

Trott commented Sep 26, 2022 • edited Loading

Trott commented Sep 26, 2022

mcollina commented Sep 27, 2022

This comment was marked as spam.

This comment was marked as spam.

vidddd1 left a comment

Choose a reason for hiding this comment

This comment was marked as spam.

This comment was marked as spam.

ShogunPanda commented Sep 20, 2022 •

edited

Loading

mhdawson commented Sep 21, 2022 •

edited

Loading

Trott commented Sep 26, 2022 •

edited

Loading