Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

errors: make code property mutable #15694

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 26 additions & 4 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,8 @@
const kCode = Symbol('code');
const messages = new Map();

const {
kMaxLength
} = process.binding('buffer');
const { kMaxLength } = process.binding('buffer');
const { defineProperty } = Object;

// Lazily loaded
var util = null;
Expand All @@ -22,16 +21,39 @@ function makeNodeError(Base) {
return class NodeError extends Base {
constructor(key, ...args) {
super(message(key, args));
this[kCode] = key;
defineProperty(this, kCode, {
configurable: true,
enumerable: false,
value: key,
writable: true
});
}

get name() {
return `${super.name} [${this[kCode]}]`;
}

set name(value) {
defineProperty(this, 'name', {
configurable: true,
enumerable: true,
value,
writable: true
});
}

get code() {
return this[kCode];
}

set code(value) {
defineProperty(this, 'code', {
configurable: true,
enumerable: true,
value,
writable: true
});
}
};
}

Expand Down
59 changes: 57 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,58 @@ 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');
assert.strictEqual(myError.hasOwnProperty('code'), false);
assert.strictEqual(myError.hasOwnProperty('name'), false);
assert.deepStrictEqual(Object.keys(myError), []);
const initialName = myError.name;
myError.code = 'FHQWHGADS';
assert.strictEqual(myError.code, 'FHQWHGADS');
assert.strictEqual(myError.name, initialName);
assert.deepStrictEqual(Object.keys(myError), ['code']);
assert.ok(myError.name.includes('ERR_TLS_HANDSHAKE_TIMEOUT'));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO these two are redundant since L300 assert.strictEqual(myError.name, initialName); covers that.
I'd add:

  myError.name = 'hgnsdhjsd';
  assert.strictEqual(myError.name , 'hgnsdhjsd');
  assert.deepStrictEqual(Object.keys(myError), ['code', 'name']);
  assert.deepStrictEqual(myError.toString(), 'hgnsdhjsd: FHQWHGADS');
  const eProto = Object.getPrototypeOf(myError);
  const getCode = Object.getOwnPropertyDescriptor(eProto, 'code').get;
  assert.deepStrictEqual(getCode.call(myError), 'ERR_TLS_HANDSHAKE_TIMEOUT');
  delete myError.code;
  delete myError.name;
  assert.deepStrictEqual(myError.toString().includes('Error:'), true);
  assert.deepStrictEqual(myError.toString().includes('ERR_TLS_HANDSHAKE_TIMEOUT'), true);

assert.ok(!myError.name.includes('FHQWHGADS'));
}

// Test that `name` is mutable and that changing it alters `toString()` but not
// `console.log()` results, which is the behavior of `Error` objects in the
// browser. Note that `name` becomes enumerable after being assigned.
{
const myError = new errors.Error('ERR_TLS_HANDSHAKE_TIMEOUT');
assert.deepStrictEqual(Object.keys(myError), []);
const initialToString = myError.toString();

myError.name = 'Fhqwhgads';
assert.deepStrictEqual(Object.keys(myError), ['name']);
assert.notStrictEqual(myError.toString(), initialToString);
}

// Test that `message` is mutable and that changing it alters `toString()` but
// not `console.log()` results, which is the behavior of `Error` objects in the
// browser. Note that `message` remains non-enumerable after being assigned.
{
let initialConsoleLog = '';
common.hijackStdout((data) => { initialConsoleLog += data; });
const myError = new errors.Error('ERR_TLS_HANDSHAKE_TIMEOUT');
assert.deepStrictEqual(Object.keys(myError), []);
const initialToString = myError.toString();
console.log(myError);
assert.notStrictEqual(initialConsoleLog, '');

common.restoreStdout();

let subsequentConsoleLog = '';
common.hijackStdout((data) => { subsequentConsoleLog += data; });
myError.message = 'Fhqwhgads';
assert.deepStrictEqual(Object.keys(myError), []);
assert.notStrictEqual(myError.toString(), initialToString);
console.log(myError);
assert.strictEqual(subsequentConsoleLog, initialConsoleLog);

common.restoreStdout();
}