errors: make message non-enumerable

A error message should always be non-enumerable. This makes sure that is true for dns errors as well. It also adds another check in `common.expectsError` to make sure no other regressions are introduced going forward. Fixes nodejs#19716 PR-URL: nodejs#19719 Fixes: nodejs#19716 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: James M Snell <[email protected]>
joyeecheung · May 2, 2018 · 74e3b14 · 74e3b14
1 parent d8b637f
commit 74e3b14
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 18 deletions.
diff --git a/lib/internal/errors.js b/lib/internal/errors.js
@@ -204,33 +204,33 @@ function exceptionWithHostPort(err, syscall, address, port, additional) {
 }
 
 /**
- * @param {number|string} err - A libuv error number or a c-ares error code
+ * @param {number|string} code - A libuv error number or a c-ares error code
  * @param {string} syscall
  * @param {string} [hostname]
  * @returns {Error}
  */
-function dnsException(err, syscall, hostname) {
-  const ex = new Error();
+function dnsException(code, syscall, hostname) {
+  let message;
   // FIXME(bnoordhuis) Remove this backwards compatibility nonsense and pass
   // the true error to the user. ENOTFOUND is not even a proper POSIX error!
-  if (err === UV_EAI_MEMORY ||
-      err === UV_EAI_NODATA ||
-      err === UV_EAI_NONAME) {
-    err = 'ENOTFOUND';  // Fabricated error name.
+  if (code === UV_EAI_MEMORY ||
+      code === UV_EAI_NODATA ||
+      code === UV_EAI_NONAME) {
+    code = 'ENOTFOUND'; // Fabricated error name.
   }
-  if (typeof err === 'string') {  // c-ares error code.
-    const errHost = hostname ? ` ${hostname}` : '';
-    ex.message = `${syscall} ${err}${errHost}`;
-    // TODO(joyeecheung): errno is supposed to be a number, like in uvException
-    ex.code = ex.errno = err;
-    ex.syscall = syscall;
+  if (typeof code === 'string') { // c-ares error code.
+    message = `${syscall} ${code}${hostname ? ` ${hostname}` : ''}`;
   } else {  // libuv error number
-    const code = lazyInternalUtil().getSystemErrorName(err);
-    ex.message = `${syscall} ${code}`;
-    // TODO(joyeecheung): errno is supposed to be err, like in uvException
-    ex.code = ex.errno = code;
-    ex.syscall = syscall;
+    code = lazyInternalUtil().getSystemErrorName(code);
+    message = `${syscall} ${code}`;
   }
+  // eslint-disable-next-line no-restricted-syntax
+  const ex = new Error(message);
+  // TODO(joyeecheung): errno is supposed to be a number / err, like in
+  // uvException.
+  ex.errno = code;
+  ex.code = code;
+  ex.syscall = syscall;
   if (hostname) {
     ex.hostname = hostname;
   }

diff --git a/test/common/index.js b/test/common/index.js
@@ -692,6 +692,9 @@ exports.expectsError = function expectsError(fn, settings, exact) {
   }
   function innerFn(error) {
     assert.strictEqual(error.code, settings.code);
+    const descriptor = Object.getOwnPropertyDescriptor(error, 'message');
+    assert.strictEqual(descriptor.enumerable,
+                       false, 'The error message should be non-enumerable');
     if ('type' in settings) {
       const type = settings.type;
       if (type !== Error && !Error.isPrototypeOf(type)) {

diff --git a/test/parallel/test-dns-lookup.js b/test/parallel/test-dns-lookup.js
@@ -81,6 +81,9 @@ assert.doesNotThrow(() => {
     assert(error);
     assert.strictEqual(tickValue, 1);
     assert.strictEqual(error.code, 'ENOENT');
+    const descriptor = Object.getOwnPropertyDescriptor(error, 'message');
+    assert.strictEqual(descriptor.enumerable,
+                       false, 'The error message should be non-enumerable');
   }));
 
   // Make sure that the error callback is called

diff --git a/test/parallel/test-dns-resolveany-bad-ancount.js b/test/parallel/test-dns-resolveany-bad-ancount.js
@@ -30,6 +30,9 @@ server.bind(0, common.mustCall(() => {
     assert.strictEqual(err.code, 'EBADRESP');
     assert.strictEqual(err.syscall, 'queryAny');
     assert.strictEqual(err.hostname, 'example.org');
+    const descriptor = Object.getOwnPropertyDescriptor(err, 'message');
+    assert.strictEqual(descriptor.enumerable,
+                       false, 'The error message should be non-enumerable');
     server.close();
   }));
 }));