From 6e172beaf0390b037549c8484f4513bb79ccc84e Mon Sep 17 00:00:00 2001
From: Rich Trott <rtrott@gmail.com>
Date: Fri, 29 Sep 2017 17:06:28 -0700
Subject: [PATCH] errors: make properties mutable

Userland code can break if it depends on a mutable `code` property for
errors. Allow users to change the `code` property but do not propagate
changes to the error `name`.

Additionally, make `message` and `name` consistent with `Error` object
(non-enumerable). Test that `console.log()` and `.toString()` calls on
internal `Error` objects with mutated properties have analogous results
with the standard ECMAScript `Error` objects.

PR-URL: https://github.com/nodejs/node/pull/15694
Fixes: https://github.com/nodejs/node/issues/15658
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
---
 lib/internal/errors.js                | 16 +++++-----
 test/parallel/test-internal-errors.js | 45 +++++++++++++++++++++++++--
 2 files changed, 50 insertions(+), 11 deletions(-)

diff --git a/lib/internal/errors.js b/lib/internal/errors.js
index 48ef2dcc3dfaef..e3c18b48c76f54 100755
--- a/lib/internal/errors.js
+++ b/lib/internal/errors.js
@@ -22,15 +22,13 @@ function makeNodeError(Base) {
   return class NodeError extends Base {
     constructor(key, ...args) {
       super(message(key, args));
-      this[kCode] = key;
-    }
-
-    get name() {
-      return `${super.name} [${this[kCode]}]`;
-    }
-
-    get code() {
-      return this[kCode];
+      this[kCode] = this.code = key;
+      Object.defineProperty(this, 'name', {
+        configurable: true,
+        enumerable: false,
+        value: `${super.name} [${this[kCode]}]`,
+        writable: true
+      });
     }
   };
 }
diff --git a/test/parallel/test-internal-errors.js b/test/parallel/test-internal-errors.js
index 410c5aa9743184..c16036c21d84a6 100644
--- a/test/parallel/test-internal-errors.js
+++ b/test/parallel/test-internal-errors.js
@@ -1,9 +1,9 @@
 // Flags: --expose-internals
 'use strict';
-
 const common = require('../common');
-const errors = require('internal/errors');
+
 const assert = require('assert');
+const errors = require('internal/errors');
 
 function invalidKey(key) {
   return new RegExp(`^An invalid error message key was used: ${key}\\.$`);
@@ -301,3 +301,44 @@ assert.strictEqual(
     'The value "bar" is invalid for argument "foo"'
   );
 }
+
+// Test that `code` property is mutable and that changing it does not change the
+// name.
+{
+  const myError = new errors.Error('ERR_TLS_HANDSHAKE_TIMEOUT');
+  assert.strictEqual(myError.code, 'ERR_TLS_HANDSHAKE_TIMEOUT');
+  const initialName = myError.name;
+  myError.code = 'FHQWHGADS';
+  assert.strictEqual(myError.code, 'FHQWHGADS');
+  assert.strictEqual(myError.name, initialName);
+  assert.ok(myError.name.includes('ERR_TLS_HANDSHAKE_TIMEOUT'));
+  assert.ok(!myError.name.includes('FHQWHGADS'));
+}
+
+// Test that `name` and `message` are mutable and that changing them alters 
+// `toString()` but not `console.log()` results, which is the behavior of
+// `Error` objects in the browser.
+{
+  function test(prop) {
+    let initialConsoleLog = '';
+    common.hijackStdout((data) => { initialConsoleLog += data; });
+    const myError = new errors.Error('ERR_TLS_HANDSHAKE_TIMEOUT');
+    const initialToString = myError.toString();
+    console.log(myError);
+    assert.notStrictEqual(initialConsoleLog, '');
+
+    common.restoreStdout();
+
+    let subsequentConsoleLog = '';
+    common.hijackStdout((data) => { subsequentConsoleLog += data; });
+    myError[prop] = 'Fhqwhgads';
+    assert.notStrictEqual(myError.toString(), initialToString);
+    console.log(myError);
+    assert.strictEqual(subsequentConsoleLog, initialConsoleLog);
+
+    common.restoreStdout();
+  }
+
+  test('name');
+  test('message');
+}