Revert the change of network interfaces family from string to integer

[aduh95]

This reverts commits 68fb0bf, 4ad342a, and 3a26db9.

Fixes: #43014

[nodejs-github-bot]

Review requested:

• @nodejs/net

[targos]

IMO the revert doesn't make sense if it is semver-major, because everyone will have to adapt to the change for v18 (which is going to be LTS).

[mcollina]

lgtm

[mcollina]

lgtm

[mcollina]

+1 to land it on Node v18.

[mcollina]

@nodejs/tsc at Today meeting we did not have quorum to LGTM as a non-semver-major change. Does anybody from the TSC objects to this?

We actually had quorum at the end of the meeting, we approve landing this as semver-minor.

[RaisinTen]

Is it okay if we remove this from the tsc-agenda now, since everyone in the meeting was +1 with reverting?

[Trott]

Rubber stamp

[richardlau]

Can anyone can help me understand where the CI failures come from? It's clearly related to the changes on this PR because it's consistently failing on SmartOS and AIX, but I have no clue why. @nodejs/dns maybe?

No idea what the "fix" is, but taking test/parallel/test-net-socket-connect on AIX these are the differences in behabiour between Node.js 17.9.0 and this PR:

With this PR:

iojs@nodejs03:[/home/iojs/build/workspace/node-test-commit-aix/nodes/aix72-ppc64]NODE_DEBUG=net ./out/Release/node test/parallel/test-net-socket-connect->
NET 20251068: setupListenHandle null 0 4 0 undefined
NET 20251068: setupListenHandle: create a handle
NET 20251068: bind to ::
NET 20251068: pipe false undefined
NET 20251068: connect: find host localhost
NET 20251068: connect: dns options { family: 'IPv6', hints: 8 }
NET 20251068: destroy
NET 20251068: close
NET 20251068: close handle
node:events:505
      throw er; // Unhandled 'error' event
      ^

Error: getaddrinfo ENOTFOUND localhost
    at GetAddrInfoReqWrap.onlookup [as oncomplete] (node:dns:83:26)
Emitted 'error' event on Socket instance at:
    at emitErrorNT (node:internal/streams/destroy:151:8)
    at emitErrorCloseNT (node:internal/streams/destroy:116:3)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21) {
  errno: -3008,
  code: 'ENOTFOUND',
  syscall: 'getaddrinfo',
  hostname: 'localhost'
}

Node.js v19.0.0-pre

Compared to Node.js 17.9.0:

iojs@nodejs03:[/home/iojs/build/workspace/node-test-commit-aix/nodes/aix72-ppc64]NODE_DEBUG=net /tmp/node-v17.9.0-aix-ppc64/bin/node  test/parallel/test->
NET 20251072: setupListenHandle null 0 4 0 undefined
NET 20251072: setupListenHandle: create a handle
NET 20251072: bind to ::
NET 20251072: pipe false undefined
NET 20251072: connect: find host localhost
NET 20251072: connect: dns options { family: 'IPv6', hints: 8 }
(node:20251072) [DEP0153] DeprecationWarning: Type coercion of dns.lookup options is deprecated
(Use `node --trace-deprecation ...` to show where the warning was created)
NET 20251072: onconnection
NET 20251072: _read
NET 20251072: Socket._handle.readStart
NET 20251072: _final: not ended, call shutdown()
NET 20251072: SERVER _emitCloseIfDrained
NET 20251072: SERVER handle? false   connections? 1
NET 20251072: afterConnect
NET 20251072: _final: not ended, call shutdown()
NET 20251072: _read
NET 20251072: Socket._handle.readStart
NET 20251072: afterShutdown destroyed=false ReadableState {
  objectMode: false,
  highWaterMark: 16384,
  buffer: BufferList { head: null, tail: null, length: 0 },
  length: 0,
  pipes: [],
  flowing: null,
  ended: false,
  endEmitted: false,
  reading: true,
  constructed: true,
  sync: false,
  needReadable: true,
  emittedReadable: false,
  readableListening: false,
  resumeScheduled: false,
  errorEmitted: false,
  emitClose: false,
  autoDestroy: true,
  destroyed: false,
  errored: null,
  closed: false,
  closeEmitted: false,
  defaultEncoding: 'utf8',
  awaitDrainWriters: null,
  multiAwaitDrain: false,
  readingMore: false,
  dataEmitted: false,
  decoder: null,
  encoding: null,
  [Symbol(kPaused)]: null
}
NET 20251072: afterShutdown destroyed=false ReadableState {
  objectMode: false,
  highWaterMark: 16384,
  buffer: BufferList { head: null, tail: null, length: 0 },
  length: 0,
  pipes: [],
  flowing: null,
  ended: false,
  endEmitted: false,
  reading: true,
  constructed: true,
  sync: false,
  needReadable: true,
  emittedReadable: false,
  readableListening: false,
  resumeScheduled: false,
  errorEmitted: false,
  emitClose: false,
  autoDestroy: true,
  destroyed: false,
  errored: null,
  closed: false,
  closeEmitted: false,
  defaultEncoding: 'utf8',
  awaitDrainWriters: null,
  multiAwaitDrain: false,
  readingMore: false,
  dataEmitted: false,
  decoder: null,
  encoding: null,
  [Symbol(kPaused)]: null
}
NET 20251072: destroy
NET 20251072: close
NET 20251072: close handle
NET 20251072: has server
NET 20251072: SERVER _emitCloseIfDrained
NET 20251072: SERVER: emit close
NET 20251072: destroy
NET 20251072: close
NET 20251072: close handle
NET 20251072: emit close
NET 20251072: emit close
iojs@nodejs03:[/home/iojs/build/workspace/node-test-commit-aix/nodes/aix72-ppc64]

We can see that

NET 20251072: connect: find host localhost
NET 20251072: connect: dns options { family: 'IPv6', hints: 8 }

With this PR:

iojs@nodejs03:[/home/iojs/build/workspace/node-test-commit-aix/nodes/aix72-ppc64]./out/Release/node
Welcome to Node.js v19.0.0-pre.
Type ".help" for more information.
> dns.lookup("localhost",  { family: 'IPv6', hints: 8 }, console.log)
GetAddrInfoReqWrap {
  callback: [Function: log],
  family: 6,
  hostname: 'localhost',
  oncomplete: [Function: onlookup]
}
> Error: getaddrinfo ENOTFOUND localhost
    at GetAddrInfoReqWrap.onlookup [as oncomplete] (node:dns:83:26)
    at GetAddrInfoReqWrap.callbackTrampoline (node:internal/async_hooks:130:17) {
  errno: -3008,
  code: 'ENOTFOUND',
  syscall: 'getaddrinfo',
  hostname: 'localhost'
}

iojs@nodejs03:[/home/iojs/build/workspace/node-test-commit-aix/nodes/aix72-ppc64]

With 17.9.0:

iojs@nodejs03:[/home/iojs/build/workspace/node-test-commit-aix/nodes/aix72-ppc64]/tmp/node-v17.9.0-aix-ppc64/bin/node
Welcome to Node.js v17.9.0.
Type ".help" for more information.
> dns.lookup("localhost",  { family: 'IPv6', hints: 8 }, console.log)
GetAddrInfoReqWrap {
  callback: [Function: log],
  family: 0,
  hostname: 'localhost',
  oncomplete: [Function: onlookup]
}
> (node:19399164) [DEP0153] DeprecationWarning: Type coercion of dns.lookup options is deprecated
(Use `node --trace-deprecation ...` to show where the warning was created)
null 127.0.0.1 4

iojs@nodejs03:[/home/iojs/build/workspace/node-test-commit-aix/nodes/aix72-ppc64]

Also setting/not setting hints affects the lookup (this PR):

iojs@nodejs03:[/home/iojs/build/workspace/node-test-commit-aix/nodes/aix72-ppc64]./out/Release/node
Welcome to Node.js v19.0.0-pre.
Type ".help" for more information.
> dns.lookup("localhost",  { family: 'IPv6', hints: 8 }, console.log)
GetAddrInfoReqWrap {
  callback: [Function: log],
  family: 6,
  hostname: 'localhost',
  oncomplete: [Function: onlookup]
}
> Error: getaddrinfo ENOTFOUND localhost
    at GetAddrInfoReqWrap.onlookup [as oncomplete] (node:dns:83:26)
    at GetAddrInfoReqWrap.callbackTrampoline (node:internal/async_hooks:130:17) {
  errno: -3008,
  code: 'ENOTFOUND',
  syscall: 'getaddrinfo',
  hostname: 'localhost'
}
> dns.lookup("localhost",  { family: 'IPv6' }, console.log)
GetAddrInfoReqWrap {
  callback: [Function: log],
  family: 6,
  hostname: 'localhost',
  oncomplete: [Function: onlookup]
}
> null ::1 6

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[aduh95]

For some reason, interpreting 'IPv4' as 4 and 'IPv6' as 6 was not working on some hosts, so now it is interpreted as 0, as it was the case in Node.js v17.x and below. Obviously that's far from ideal, but that seems to be the only way I could find to revert the change on node:net and have all the tests pass.

[nodejs-github-bot]

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