lib: correct error.errno to always be numeric

Historically `error.errno` of system errors thrown by Node.js
can sometimes be the same as `err.code`, which are string
representations of the error numbers. This is useless and incorrect,
and results in an information loss for users since then they
will have to resort to something like
`process.binding('uv'[`UV_${errno}`])` to get to the numeric
error codes.

This patch corrects this behavior by always setting `error.errno`
to be negative numbers. For fabricated errors like `ENOTFOUND`,
`error.errno` is now undefined since there is no numeric equivalent
for them anyway. For c-ares errors, `error.errno` is now undefined
because the numeric representations (negated) can be in conflict
with libuv error codes - this is fine since numeric codes was
not available for c-ares errors anyway.

Users can use the public API `util.getSystemErrorName(errno)`
to retrieve string codes for these numbers.

PR-URL: https://github.com/nodejs/node/pull/28140
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

15 files changed, 73 insertions, 42 deletions

diff --git a/test/internet/test-dns.js b/test/internet/test-dns.js
index 3d331fd196..c74acc50dd 100644
--- a/test/internet/test-dns.js
+++ b/test/internet/test-dns.js
@@ -24,6 +24,7 @@
 const common = require('../common');
 const { addresses } = require('../common/internet');
 const { internalBinding } = require('internal/test/binding');
+const { getSystemErrorName } = require('util');
 const assert = require('assert');
 const dns = require('dns');
 const net = require('net');
@@ -71,7 +72,10 @@ function checkWrap(req) {
 TEST(function test_reverse_bogus(done) {
   dnsPromises.reverse('bogus ip')
     .then(common.mustNotCall())
-    .catch(common.expectsError({ errno: 'EINVAL' }));
+    .catch(common.mustCall((err) => {
+      assert.strictEqual(err.code, 'EINVAL');
+      assert.strictEqual(getSystemErrorName(err.errno), 'EINVAL');
+    }));
 
   assert.throws(() => {
     dns.reverse('bogus ip', common.mustNotCall());
@@ -161,11 +165,13 @@ TEST(async function test_resolveMx(done) {
 TEST(function test_resolveMx_failure(done) {
   dnsPromises.resolveMx(addresses.INVALID_HOST)
     .then(common.mustNotCall())
-    .catch(common.expectsError({ errno: 'ENOTFOUND' }));
+    .catch(common.mustCall((err) => {
+      assert.strictEqual(err.code, 'ENOTFOUND');
+    }));
 
   const req = dns.resolveMx(addresses.INVALID_HOST, function(err, result) {
     assert.ok(err instanceof Error);
-    assert.strictEqual(err.errno, 'ENOTFOUND');
+    assert.strictEqual(err.code, 'ENOTFOUND');
 
     assert.strictEqual(result, undefined);
 
@@ -199,11 +205,13 @@ TEST(async function test_resolveNs(done) {
 TEST(function test_resolveNs_failure(done) {
   dnsPromises.resolveNs(addresses.INVALID_HOST)
     .then(common.mustNotCall())
-    .catch(common.expectsError({ errno: 'ENOTFOUND' }));
+    .catch(common.mustCall((err) => {
+      assert.strictEqual(err.code, 'ENOTFOUND');
+    }));
 
   const req = dns.resolveNs(addresses.INVALID_HOST, function(err, result) {
     assert.ok(err instanceof Error);
-    assert.strictEqual(err.errno, 'ENOTFOUND');
+    assert.strictEqual(err.code, 'ENOTFOUND');
 
     assert.strictEqual(result, undefined);
 
@@ -241,11 +249,13 @@ TEST(async function test_resolveSrv(done) {
 TEST(function test_resolveSrv_failure(done) {
   dnsPromises.resolveSrv(addresses.INVALID_HOST)
     .then(common.mustNotCall())
-    .catch(common.expectsError({ errno: 'ENOTFOUND' }));
+    .catch(common.mustCall((err) => {
+      assert.strictEqual(err.code, 'ENOTFOUND');
+    }));
 
   const req = dns.resolveSrv(addresses.INVALID_HOST, function(err, result) {
     assert.ok(err instanceof Error);
-    assert.strictEqual(err.errno, 'ENOTFOUND');
+    assert.strictEqual(err.code, 'ENOTFOUND');
 
     assert.strictEqual(result, undefined);
 
@@ -279,11 +289,13 @@ TEST(async function test_resolvePtr(done) {
 TEST(function test_resolvePtr_failure(done) {
   dnsPromises.resolvePtr(addresses.INVALID_HOST)
     .then(common.mustNotCall())
-    .catch(common.expectsError({ errno: 'ENOTFOUND' }));
+    .catch(common.mustCall((err) => {
+      assert.strictEqual(err.code, 'ENOTFOUND');
+    }));
 
   const req = dns.resolvePtr(addresses.INVALID_HOST, function(err, result) {
     assert.ok(err instanceof Error);
-    assert.strictEqual(err.errno, 'ENOTFOUND');
+    assert.strictEqual(err.code, 'ENOTFOUND');
 
     assert.strictEqual(result, undefined);
 
@@ -322,11 +334,13 @@ TEST(async function test_resolveNaptr(done) {
 TEST(function test_resolveNaptr_failure(done) {
   dnsPromises.resolveNaptr(addresses.INVALID_HOST)
     .then(common.mustNotCall())
-    .catch(common.expectsError({ errno: 'ENOTFOUND' }));
+    .catch(common.mustCall((err) => {
+      assert.strictEqual(err.code, 'ENOTFOUND');
+    }));
 
   const req = dns.resolveNaptr(addresses.INVALID_HOST, function(err, result) {
     assert.ok(err instanceof Error);
-    assert.strictEqual(err.errno, 'ENOTFOUND');
+    assert.strictEqual(err.code, 'ENOTFOUND');
 
     assert.strictEqual(result, undefined);
 
@@ -369,11 +383,13 @@ TEST(async function test_resolveSoa(done) {
 TEST(function test_resolveSoa_failure(done) {
   dnsPromises.resolveSoa(addresses.INVALID_HOST)
     .then(common.mustNotCall())
-    .catch(common.expectsError({ errno: 'ENOTFOUND' }));
+    .catch(common.mustCall((err) => {
+      assert.strictEqual(err.code, 'ENOTFOUND');
+    }));
 
   const req = dns.resolveSoa(addresses.INVALID_HOST, function(err, result) {
     assert.ok(err instanceof Error);
-    assert.strictEqual(err.errno, 'ENOTFOUND');
+    assert.strictEqual(err.code, 'ENOTFOUND');
 
     assert.strictEqual(result, undefined);
 
@@ -407,11 +423,13 @@ TEST(async function test_resolveCname(done) {
 TEST(function test_resolveCname_failure(done) {
   dnsPromises.resolveCname(addresses.INVALID_HOST)
     .then(common.mustNotCall())
-    .catch(common.expectsError({ errno: 'ENOTFOUND' }));
+    .catch(common.mustCall((err) => {
+      assert.strictEqual(err.code, 'ENOTFOUND');
+    }));
 
   const req = dns.resolveCname(addresses.INVALID_HOST, function(err, result) {
     assert.ok(err instanceof Error);
-    assert.strictEqual(err.errno, 'ENOTFOUND');
+    assert.strictEqual(err.code, 'ENOTFOUND');
 
     assert.strictEqual(result, undefined);
 
@@ -443,11 +461,13 @@ TEST(async function test_resolveTxt(done) {
 TEST(function test_resolveTxt_failure(done) {
   dnsPromises.resolveTxt(addresses.INVALID_HOST)
     .then(common.mustNotCall())
-    .catch(common.expectsError({ errno: 'ENOTFOUND' }));
+    .catch(common.mustCall((err) => {
+      assert.strictEqual(err.code, 'ENOTFOUND');
+    }));
 
   const req = dns.resolveTxt(addresses.INVALID_HOST, function(err, result) {
     assert.ok(err instanceof Error);
-    assert.strictEqual(err.errno, 'ENOTFOUND');
+    assert.strictEqual(err.code, 'ENOTFOUND');
 
     assert.strictEqual(result, undefined);
 
@@ -461,12 +481,12 @@ TEST(function test_resolveTxt_failure(done) {
 TEST(function test_lookup_failure(done) {
   dnsPromises.lookup(addresses.INVALID_HOST, 4)
     .then(common.mustNotCall())
-    .catch(common.expectsError({ errno: dns.NOTFOUND }));
+    .catch(common.expectsError({ code: dns.NOTFOUND }));
 
   const req = dns.lookup(addresses.INVALID_HOST, 4, (err) => {
     assert.ok(err instanceof Error);
-    assert.strictEqual(err.errno, dns.NOTFOUND);
-    assert.strictEqual(err.errno, 'ENOTFOUND');
+    assert.strictEqual(err.code, dns.NOTFOUND);
+    assert.strictEqual(err.code, 'ENOTFOUND');
     assert.ok(!/ENOENT/.test(err.message));
     assert.ok(err.message.includes(addresses.INVALID_HOST));
 
diff --git a/test/parallel/test-child-process-execfilesync-maxbuf.js b/test/parallel/test-child-process-execfilesync-maxbuf.js
index 6e89444f3d..9af7c0ed8d 100644
--- a/test/parallel/test-child-process-execfilesync-maxbuf.js
+++ b/test/parallel/test-child-process-execfilesync-maxbuf.js
@@ -5,6 +5,7 @@ require('../common');
 // works as expected.
 
 const assert = require('assert');
+const { getSystemErrorName } = require('util');
 const { execFileSync } = require('child_process');
 const msgOut = 'this is stdout';
 const msgOutBuf = Buffer.from(`${msgOut}\n`);
@@ -20,7 +21,8 @@ const args = [
     execFileSync(process.execPath, args, { maxBuffer: 1 });
   }, (e) => {
     assert.ok(e, 'maxBuffer should error');
-    assert.strictEqual(e.errno, 'ENOBUFS');
+    assert.strictEqual(e.code, 'ENOBUFS');
+    assert.strictEqual(getSystemErrorName(e.errno), 'ENOBUFS');
     // We can have buffers larger than maxBuffer because underneath we alloc 64k
     // that matches our read sizes.
     assert.deepStrictEqual(e.stdout, msgOutBuf);
@@ -44,7 +46,8 @@ const args = [
     );
   }, (e) => {
     assert.ok(e, 'maxBuffer should error');
-    assert.strictEqual(e.errno, 'ENOBUFS');
+    assert.strictEqual(e.code, 'ENOBUFS');
+    assert.strictEqual(getSystemErrorName(e.errno), 'ENOBUFS');
     return true;
   });
 }
diff --git a/test/parallel/test-child-process-execsync-maxbuf.js b/test/parallel/test-child-process-execsync-maxbuf.js
index 6400c49b02..0aafd1a74b 100644
--- a/test/parallel/test-child-process-execsync-maxbuf.js
+++ b/test/parallel/test-child-process-execsync-maxbuf.js
@@ -5,6 +5,7 @@ require('../common');
 // works as expected.
 
 const assert = require('assert');
+const { getSystemErrorName } = require('util');
 const { execSync } = require('child_process');
 const msgOut = 'this is stdout';
 const msgOutBuf = Buffer.from(`${msgOut}\n`);
@@ -20,7 +21,8 @@ const args = [
     execSync(`"${process.execPath}" ${args.join(' ')}`, { maxBuffer: 1 });
   }, (e) => {
     assert.ok(e, 'maxBuffer should error');
-    assert.strictEqual(e.errno, 'ENOBUFS');
+    assert.strictEqual(e.code, 'ENOBUFS');
+    assert.strictEqual(getSystemErrorName(e.errno), 'ENOBUFS');
     // We can have buffers larger than maxBuffer because underneath we alloc 64k
     // that matches our read sizes.
     assert.deepStrictEqual(e.stdout, msgOutBuf);
@@ -46,7 +48,8 @@ const args = [
     );
   }, (e) => {
     assert.ok(e, 'maxBuffer should error');
-    assert.strictEqual(e.errno, 'ENOBUFS');
+    assert.strictEqual(e.code, 'ENOBUFS');
+    assert.strictEqual(getSystemErrorName(e.errno), 'ENOBUFS');
     return true;
   });
 }
diff --git a/test/parallel/test-child-process-spawn-error.js b/test/parallel/test-child-process-spawn-error.js
index 33f66d0ba6..d6560ee9cc 100644
--- a/test/parallel/test-child-process-spawn-error.js
+++ b/test/parallel/test-child-process-spawn-error.js
@@ -21,6 +21,7 @@
 
 'use strict';
 const common = require('../common');
+const { getSystemErrorName } = require('util');
 const spawn = require('child_process').spawn;
 const assert = require('assert');
 const fs = require('fs');
@@ -42,7 +43,7 @@ assert.strictEqual(enoentChild.stdio[2], enoentChild.stderr);
 
 enoentChild.on('error', common.mustCall(function(err) {
   assert.strictEqual(err.code, 'ENOENT');
-  assert.strictEqual(err.errno, 'ENOENT');
+  assert.strictEqual(getSystemErrorName(err.errno), 'ENOENT');
   assert.strictEqual(err.syscall, `spawn ${enoentPath}`);
   assert.strictEqual(err.path, enoentPath);
   assert.deepStrictEqual(err.spawnargs, spawnargs);
diff --git a/test/parallel/test-child-process-spawnsync-maxbuf.js b/test/parallel/test-child-process-spawnsync-maxbuf.js
index 5f4e959b88..8d685350ad 100644
--- a/test/parallel/test-child-process-spawnsync-maxbuf.js
+++ b/test/parallel/test-child-process-spawnsync-maxbuf.js
@@ -6,6 +6,7 @@ require('../common');
 
 const assert = require('assert');
 const spawnSync = require('child_process').spawnSync;
+const { getSystemErrorName } = require('util');
 const msgOut = 'this is stdout';
 const msgOutBuf = Buffer.from(`${msgOut}\n`);
 
@@ -19,7 +20,8 @@ const args = [
   const ret = spawnSync(process.execPath, args, { maxBuffer: 1 });
 
   assert.ok(ret.error, 'maxBuffer should error');
-  assert.strictEqual(ret.error.errno, 'ENOBUFS');
+  assert.strictEqual(ret.error.code, 'ENOBUFS');
+  assert.strictEqual(getSystemErrorName(ret.error.errno), 'ENOBUFS');
   // We can have buffers larger than maxBuffer because underneath we alloc 64k
   // that matches our read sizes.
   assert.deepStrictEqual(ret.stdout, msgOutBuf);
@@ -39,7 +41,8 @@ const args = [
   const ret = spawnSync(process.execPath, args);
 
   assert.ok(ret.error, 'maxBuffer should error');
-  assert.strictEqual(ret.error.errno, 'ENOBUFS');
+  assert.strictEqual(ret.error.code, 'ENOBUFS');
+  assert.strictEqual(getSystemErrorName(ret.error.errno), 'ENOBUFS');
 }
 
 // Default maxBuffer size is 1024 * 1024.
diff --git a/test/parallel/test-child-process-spawnsync-timeout.js b/test/parallel/test-child-process-spawnsync-timeout.js
index 35df6cee22..155e757f5d 100644
--- a/test/parallel/test-child-process-spawnsync-timeout.js
+++ b/test/parallel/test-child-process-spawnsync-timeout.js
@@ -24,6 +24,7 @@ const common = require('../common');
 const assert = require('assert');
 
 const spawnSync = require('child_process').spawnSync;
+const { getSystemErrorName } = require('util');
 
 const TIMER = 200;
 const SLEEP = common.platformTimeout(5000);
@@ -39,7 +40,8 @@ switch (process.argv[2]) {
     const start = Date.now();
     const ret = spawnSync(process.execPath, [__filename, 'child'],
                           { timeout: TIMER });
-    assert.strictEqual(ret.error.errno, 'ETIMEDOUT');
+    assert.strictEqual(ret.error.code, 'ETIMEDOUT');
+    assert.strictEqual(getSystemErrorName(ret.error.errno), 'ETIMEDOUT');
     const end = Date.now() - start;
     assert(end < SLEEP);
     assert(ret.status > 128 || ret.signal);
diff --git a/test/parallel/test-child-process-spawnsync.js b/test/parallel/test-child-process-spawnsync.js
index 99ce75da12..ae87d5245c 100644
--- a/test/parallel/test-child-process-spawnsync.js
+++ b/test/parallel/test-child-process-spawnsync.js
@@ -23,6 +23,7 @@
 const common = require('../common');
 const assert = require('assert');
 const { spawnSync } = require('child_process');
+const { getSystemErrorName } = require('util');
 
 // `sleep` does different things on Windows and Unix, but in both cases, it does
 // more-or-less nothing if there are no parameters
@@ -33,7 +34,7 @@ assert.strictEqual(ret.status, 0);
 const ret_err = spawnSync('command_does_not_exist', ['bar']).error;
 
 assert.strictEqual(ret_err.code, 'ENOENT');
-assert.strictEqual(ret_err.errno, 'ENOENT');
+assert.strictEqual(getSystemErrorName(ret_err.errno), 'ENOENT');
 assert.strictEqual(ret_err.syscall, 'spawnSync command_does_not_exist');
 assert.strictEqual(ret_err.path, 'command_does_not_exist');
 assert.deepStrictEqual(ret_err.spawnargs, ['bar']);
diff --git a/test/parallel/test-dgram-send-error.js b/test/parallel/test-dgram-send-error.js
index f6d37f2c81..6aa326c8ec 100644
--- a/test/parallel/test-dgram-send-error.js
+++ b/test/parallel/test-dgram-send-error.js
@@ -5,6 +5,7 @@ const assert = require('assert');
 const dgram = require('dgram');
 const { internalBinding } = require('internal/test/binding');
 const { UV_UNKNOWN } = internalBinding('uv');
+const { getSystemErrorName } = require('util');
 const { kStateSymbol } = require('internal/dgram');
 const mockError = new Error('mock DNS error');
 
@@ -50,7 +51,7 @@ getSocket((socket) => {
     const callback = common.mustCall((err) => {
       socket.close();
       assert.strictEqual(err.code, 'UNKNOWN');
-      assert.strictEqual(err.errno, 'UNKNOWN');
+      assert.strictEqual(getSystemErrorName(err.errno), 'UNKNOWN');
       assert.strictEqual(err.syscall, 'send');
       assert.strictEqual(err.address, common.localhostIPv4);
       assert.strictEqual(err.port, port);
diff --git a/test/parallel/test-dns-cancel-reverse-lookup.js b/test/parallel/test-dns-cancel-reverse-lookup.js
index 0918178e12..e0cb4d1854 100644
--- a/test/parallel/test-dns-cancel-reverse-lookup.js
+++ b/test/parallel/test-dns-cancel-reverse-lookup.js
@@ -12,7 +12,6 @@ server.bind(0, common.mustCall(() => {
   resolver.setServers([`127.0.0.1:${server.address().port}`]);
   resolver.reverse('123.45.67.89', common.mustCall((err, res) => {
     assert.strictEqual(err.code, 'ECANCELLED');
-    assert.strictEqual(err.errno, 'ECANCELLED');
     assert.strictEqual(err.syscall, 'getHostByAddr');
     assert.strictEqual(err.hostname, '123.45.67.89');
     server.close();
diff --git a/test/parallel/test-dns-channel-cancel.js b/test/parallel/test-dns-channel-cancel.js
index 797d61efb4..0c59ee5316 100644
--- a/test/parallel/test-dns-channel-cancel.js
+++ b/test/parallel/test-dns-channel-cancel.js
@@ -12,7 +12,6 @@ server.bind(0, common.mustCall(() => {
   resolver.setServers([`127.0.0.1:${server.address().port}`]);
   resolver.resolve4('example.org', common.mustCall((err, res) => {
     assert.strictEqual(err.code, 'ECANCELLED');
-    assert.strictEqual(err.errno, 'ECANCELLED');
     assert.strictEqual(err.syscall, 'queryA');
     assert.strictEqual(err.hostname, 'example.org');
     server.close();
diff --git a/test/parallel/test-dns.js b/test/parallel/test-dns.js
index 40e0e605cd..ad62cefc12 100644
--- a/test/parallel/test-dns.js
+++ b/test/parallel/test-dns.js
@@ -319,7 +319,6 @@ common.expectsError(() => {
 
 {
   dns.resolveMx('foo.onion', function(err) {
-    assert.deepStrictEqual(err.errno, 'ENOTFOUND');
     assert.deepStrictEqual(err.code, 'ENOTFOUND');
     assert.deepStrictEqual(err.syscall, 'queryMx');
     assert.deepStrictEqual(err.hostname, 'foo.onion');
diff --git a/test/parallel/test-net-normalize-args.js b/test/parallel/test-net-normalize-args.js
index 4677b396e2..347c18d638 100644
--- a/test/parallel/test-net-normalize-args.js
+++ b/test/parallel/test-net-normalize-args.js
@@ -4,6 +4,7 @@ const common = require('../common');
 const assert = require('assert');
 const net = require('net');
 const { normalizedArgsSymbol } = require('internal/net');
+const { getSystemErrorName } = require('util');
 
 function validateNormalizedArgs(input, output) {
   const args = net._normalizeArgs(input);
@@ -32,7 +33,7 @@ validateNormalizedArgs([{ port: 1234 }, assert.fail], res);
 
     socket.on('error', common.mustCall((err) => {
       assert(possibleErrors.includes(err.code));
-      assert(possibleErrors.includes(err.errno));
+      assert(possibleErrors.includes(getSystemErrorName(err.errno)));
       assert.strictEqual(err.syscall, 'connect');
       server.close();
     }));
diff --git a/test/parallel/test-stdout-close-catch.js b/test/parallel/test-stdout-close-catch.js
index 6c0db3ea90..924b52715a 100644
--- a/test/parallel/test-stdout-close-catch.js
+++ b/test/parallel/test-stdout-close-catch.js
@@ -3,6 +3,7 @@ const common = require('../common');
 const assert = require('assert');
 const child_process = require('child_process');
 const fixtures = require('../common/fixtures');
+const { getSystemErrorName } = require('util');
 
 const testScript = fixtures.path('catch-stdout-error.js');
 
@@ -13,11 +14,6 @@ const cmd = `${JSON.stringify(process.execPath)} ` +
 
 const child = child_process.exec(cmd);
 let output = '';
-const outputExpect = {
-  code: 'EPIPE',
-  errno: 'EPIPE',
-  syscall: 'write'
-};
 
 child.stderr.on('data', function(c) {
   output += c;
@@ -32,6 +28,8 @@ child.on('close', common.mustCall(function(code) {
     process.exit(1);
   }
 
-  assert.deepStrictEqual(output, outputExpect);
+  assert.strictEqual(output.code, 'EPIPE');
+  assert.strictEqual(getSystemErrorName(output.errno), 'EPIPE');
+  assert.strictEqual(output.syscall, 'write');
   console.log('ok');
 }));
diff --git a/test/parallel/test-uv-errno.js b/test/parallel/test-uv-errno.js
index 078db29adc..768e2f27dd 100644
--- a/test/parallel/test-uv-errno.js
+++ b/test/parallel/test-uv-errno.js
@@ -20,7 +20,7 @@ keys.forEach((key) => {
   const name = uv.errname(uv[key]);
   assert.strictEqual(getSystemErrorName(uv[key]), name);
   assert.strictEqual(err.code, name);
-  assert.strictEqual(err.code, err.errno);
+  assert.strictEqual(err.code, getSystemErrorName(err.errno));
   assert.strictEqual(err.message, `test ${name}`);
 });
 
diff --git a/test/sequential/test-child-process-execsync.js b/test/sequential/test-child-process-execsync.js
index d6b6fff968..171ccacab6 100644
--- a/test/sequential/test-child-process-execsync.js
+++ b/test/sequential/test-child-process-execsync.js
@@ -24,6 +24,7 @@ const common = require('../common');
 const assert = require('assert');
 
 const { execFileSync, execSync, spawnSync } = require('child_process');
+const { getSystemErrorName } = require('util');
 
 const TIMER = 200;
 const SLEEP = 2000;
@@ -48,7 +49,7 @@ try {
   ret = execSync(cmd, { timeout: TIMER });
 } catch (e) {
   caught = true;
-  assert.strictEqual(e.errno, 'ETIMEDOUT');
+  assert.strictEqual(getSystemErrorName(e.errno), 'ETIMEDOUT');
   err = e;
 } finally {
   assert.strictEqual(ret, undefined,