v20 dns.reverse throws on ::1

[msimerson]

Version

20.0.0

Platform

Darwin (mac OS) & Ubuntu

Subsystem

dns

What steps will reproduce the bug?

> git:(master) node                                                                     
Welcome to Node.js v20.0.0.
Type ".help" for more information.
> dns.reverse('::1', (e,r) => { if (e) console.error(e); console.log(r); })
Uncaught Error: getHostByAddr EINVAL ::1
    at __node_internal_captureLargerStackTrace (node:internal/errors:490:5)
    at __node_internal_ (node:internal/errors:712:10)
    at ResolverBase.getHostByAddr (node:internal/dns/callback_resolver:74:20) {
  errno: -22,
  code: 'EINVAL',
  syscall: 'getHostByAddr',
  hostname: '::1'
}

How often does it reproduce? Is there a required condition?

Every time I've tried.

What is the expected behavior? Why is that the expected behavior?

I'm calling a function with a callback and a parameter. I'm expecting to receive a callback, perhaps with an error.

# node
Welcome to Node.js v16.16.0.
Type ".help" for more information.
> dns.reverse('::1', (e,r) => { if (e) console.error(e); console.log(r); })
QueryReqWrap {
  bindingName: 'getHostByAddr',
  callback: [Function (anonymous)],
  hostname: '::1',
  oncomplete: [Function: onresolve],
  ttl: false
}
> [ 'localhost.my.domain' ]

What do you see instead?

Uncaught Error: getHostByAddr EINVAL ::1

Additional information

The throwing behavior of dns.reverse is still wrong (since at least #3112) and should be fixed. The error should be returned in the callback instead of throwing. A test suite of mine is now failing on node.js v20 and now I have to go add wrapper code around a use of dns.resolve because the most used IPv6 address on the planet (::1) isn't being recognized as valid.

Also, throwing when an IP resolves to an empty list (#45575), which isn't even an error 🤯 , is at the very least a POLA violation. Doesn't returning an ENOTFOUND with an empty list seem a whole lot more sensible? If fixing the behavior isn't possible, is it not also a bug that throwing under ordinary and expected circumstances is not documented?

[bnoordhuis]

I can't reproduce and I have a hard time imagining how that particular error could happen. The only place where dns.reverse() synchronously throws that error is here:

node/src/cares_wrap.cc

Lines 1361 to 1369 in 76ae7be

	if (uv_inet_pton(AF_INET, name, &address_buffer) == 0) {
	length = sizeof(struct in_addr);
	family = AF_INET;
	} else if (uv_inet_pton(AF_INET6, name, &address_buffer) == 0) {
	length = sizeof(struct in6_addr);
	family = AF_INET6;
	} else {
	return UV_EINVAL; // So errnoException() reports a proper error.
	}

And I know for a fact that uv_inet_pton() parses ::1 just fine.

[msimerson]

Our CI tests for Haraka started failing on node 20 at least a couple weeks ago. Yesterday I dove in to figure out why. Here is a GitHub workflow action where the test passes on node 16 and 18 (and all previous versions) but fails on node 20.

I deinstalled node 18 on my development mac and installed v20 and was able to reproduce the error. I fixed it yesterday in this commit.

[bnoordhuis]

Long shot but can you try with v20.1.0?

[bnoordhuis]

No need to try - you should upgrade. :-)

v20.1.0 works, v20.0.0 doesn't - I just tested. Closing.