From 7436ac757c67d502651349ba34986c53930a1cf7 Mon Sep 17 00:00:00 2001 From: Rich Trott 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. Fixes: https://github.com/nodejs/node/issues/15658 --- lib/internal/errors.js | 16 ++++------ test/parallel/test-internal-errors.js | 46 +++++++++++++++++++++++++-- 2 files changed, 51 insertions(+), 11 deletions(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 175144078a1a34..71af1c85d7e444 100755 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -18,15 +18,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 1c16823e7a1389..56c34ae23c6538 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}\\.$`); @@ -284,3 +284,45 @@ assert.strictEqual( assert.strictEqual( errors.message('ERR_INVALID_ASYNC_ID', ['asyncId', undefined]), 'Invalid asyncId value: undefined'); + + +// 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'); +}