lib: correct error.errno to always be numeric

doc/api/errors.md

@@ -419,7 +419,7 @@ attempts to read a file that does not exist.
 * `code` {string} The string error code
 * `dest` {string} If present, the file path destination when reporting a file
   system error
-* `errno` {number|string} The system-provided error number
+* `errno` {number} The system-provided error number
 * `info` {Object} If present, extra details about the error condition
 * `message` {string} A system-provided human-readable description of the error
 * `path` {string} If present, the file path when reporting a file system error
@@ -448,13 +448,15 @@ system error.
 
 ### error.errno
 
-* {string|number}
+* {number}
+
+The `error.errno` property is a negative number which corresponds
+to the error code defined in [`libuv Error handling`].
+
+On Windows the error number provided by the system will be normalized by libuv.
 
-The `error.errno` property is a number or a string. If it is a number, it is a
-negative value which corresponds to the error code defined in
-[`libuv Error handling`]. See the libuv `errno.h` header file
-(`deps/uv/include/uv/errno.h` in the Node.js source tree) for details. In case
-of a string, it is the same as `error.code`.
+To get the string representation of the error code, use
+[`util.getSystemErrorName(error.errno)`].
 
 ### error.info
 
@@ -2357,6 +2359,7 @@ such as `process.stdout.on('data')`.
 [`stream.write()`]: stream.html#stream_writable_write_chunk_encoding_callback
 [`subprocess.kill()`]: child_process.html#child_process_subprocess_kill_signal
 [`subprocess.send()`]: child_process.html#child_process_subprocess_send_message_sendhandle_options_callback
+[`util.getSystemErrorName(error.errno)`]: util.html#util_util_getsystemerrorname_err
 [`zlib`]: zlib.html
 [ES Module]: esm.html
 [ICU]: intl.html#intl_internationalization_support

lib/internal/cluster/round_robin_handle.js

@@ -2,7 +2,6 @@
 const assert = require('internal/assert');
 const net = require('net');
 const { sendHelper } = require('internal/cluster/utils');
-const uv = internalBinding('uv');
 const { constants } = internalBinding('tcp_wrap');
 
 module.exports = RoundRobinHandle;
@@ -58,10 +57,7 @@ RoundRobinHandle.prototype.add = function(worker, send) {
   // Still busy binding.
   this.server.once('listening', done);
   this.server.once('error', (err) => {
-    // Hack: translate 'EADDRINUSE' error string back to numeric error code.
-    // It works but ideally we'd have some backchannel between the net and
-    // cluster modules for stuff like this.
-    send(uv[`UV_${err.errno}`], null);
+    send(err.errno, null);
   });
 };
 

lib/internal/errors.js

@@ -423,7 +423,7 @@ function uvExceptionWithHostPort(err, syscall, address, port) {
   const ex = new Error(`${message}${details}`);
   Error.stackTraceLimit = tmpLimit;
   ex.code = code;
-  ex.errno = code;
+  ex.errno = err;
   ex.syscall = syscall;
   ex.address = address;
   if (port) {
@@ -455,8 +455,8 @@ function errnoException(err, syscall, original) {
 
   // eslint-disable-next-line no-restricted-syntax
   const ex = new Error(message);
-  // TODO(joyeecheung): errno is supposed to err, like in uvException
-  ex.code = ex.errno = code;
+  ex.errno = err;
+  ex.code = code;
   ex.syscall = syscall;
 
   // eslint-disable-next-line no-restricted-syntax
@@ -499,9 +499,9 @@ function exceptionWithHostPort(err, syscall, address, port, additional) {
   Error.stackTraceLimit = 0;
   // eslint-disable-next-line no-restricted-syntax
   const ex = new Error(`${syscall} ${code}${details}`);
-  // TODO(joyeecheung): errno is supposed to err, like in uvException
   Error.stackTraceLimit = tmpLimit;
-  ex.code = ex.errno = code;
+  ex.errno = err;
+  ex.code = code;
   ex.syscall = syscall;
   ex.address = address;
   if (port) {
@@ -520,9 +520,16 @@ function exceptionWithHostPort(err, syscall, address, port, additional) {
  * @returns {Error}
  */
 function dnsException(code, syscall, hostname) {
+  let errno;
   // If `code` is of type number, it is a libuv error number, else it is a
   // c-ares error code.
+  // TODO(joyeecheung): translate c-ares error codes into numeric ones and
+  // make them available in a property that's not error.errno (since they
+  // can be in conflict with libuv error codes). Also make sure
+  // util.getSystemErrorName() can understand them when an being informed that
+  // the number is a c-ares error code.
   if (typeof code === 'number') {
+    errno = code;
     // ENOTFOUND is not a proper POSIX error, but this error has been in place
     // long enough that it's not practical to remove it.
     if (code === lazyUv().UV_EAI_NODATA || code === lazyUv().UV_EAI_NONAME) {
@@ -539,10 +546,8 @@ function dnsException(code, syscall, hostname) {
   Error.stackTraceLimit = 0;
   // eslint-disable-next-line no-restricted-syntax
   const ex = new Error(message);
-  // TODO(joyeecheung): errno is supposed to be a number / err, like in
   Error.stackTraceLimit = tmpLimit;
-  // uvException.
-  ex.errno = code;
+  ex.errno = errno;
   ex.code = code;
   ex.syscall = syscall;
   if (hostname) {

lib/internal/net.js

@@ -56,9 +56,7 @@ function makeSyncWrite(fd) {
     writeBuffer(fd, chunk, 0, chunk.length, null, undefined, ctx);
     if (ctx.errno !== undefined) {
       const ex = errors.uvException(ctx);
-      // Legacy: net writes have .code === .errno, whereas writeBuffer gives the
-      // raw errno number in .errno.
-      ex.errno = ex.code;
+      ex.errno = ctx.errno;
       return cb(ex);
     }
     cb();

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

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;
   });
 }

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;
   });
 }

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

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.