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.

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 committed Oct 4, 2017
1 parent 8068577 commit 6e172be
Show file tree
Hide file tree
Showing 2 changed files with 50 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 @@ -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
});
}
};
}
Expand Down
45 changes: 43 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 @@ -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');
}

0 comments on commit 6e172be

Please sign in to comment.