Skip to content

Commit

Permalink
errors: make properties mutable
Browse files Browse the repository at this point in the history
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.

Backport-PR-URL: #16078
PR-URL: #15694
Fixes: #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>
  • Loading branch information
Trott authored and evanlucas committed Oct 23, 2017
1 parent 0b04869 commit 0bd2fd5
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 11 deletions.
16 changes: 7 additions & 9 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
});
}
};
}
Expand Down
46 changes: 44 additions & 2 deletions test/parallel/test-internal-errors.js
Original file line number Diff line number Diff line change
@@ -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}\\.$`);
Expand Down Expand Up @@ -235,3 +235,45 @@ assert.throws(
code: 'ERR_ASSERTION',
message: /^At least one arg needs to be specified$/
}));


// Test that `code` property is mutable and that changing it does not change the
// name.
{
const myError = new errors.Error('ERR_MISSING_ARGS', ['name']);
assert.strictEqual(myError.code, 'ERR_MISSING_ARGS');
const initialName = myError.name;
myError.code = 'FHQWHGADS';
assert.strictEqual(myError.code, 'FHQWHGADS');
assert.strictEqual(myError.name, initialName);
assert.ok(myError.name.includes('ERR_MISSING_ARGS'));
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_MISSING_ARGS', ['name']);
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');
}

0 comments on commit 0bd2fd5

Please sign in to comment.