Skip to content

Commit

Permalink
errors: eliminate circular dependency on assert
Browse files Browse the repository at this point in the history
PR-URL: nodejs#15002
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
  • Loading branch information
jasnell committed Sep 25, 2017
1 parent 2e03138 commit 4df100f
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 29 deletions.
34 changes: 25 additions & 9 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ const kCode = Symbol('code');
const messages = new Map();

// Lazily loaded
var assert = null;
var util = null;

function makeNodeError(Base) {
Expand Down Expand Up @@ -60,11 +59,22 @@ class AssertionError extends Error {
}
}

// This is defined here instead of using the assert module to avoid a
// circular dependency. The effect is largely the same.
function internalAssert(condition, message) {
if (!condition) {
throw new AssertionError({
message,
actual: false,
expected: true,
operator: '=='
});
}
}

function message(key, args) {
if (assert === null) assert = require('assert');
assert.strictEqual(typeof key, 'string');
const msg = messages.get(key);
assert(msg, `An invalid error message key was used: ${key}.`);
internalAssert(msg, `An invalid error message key was used: ${key}.`);
let fmt;
if (typeof msg === 'function') {
fmt = msg;
Expand Down Expand Up @@ -184,6 +194,11 @@ E('ERR_HTTP2_UNSUPPORTED_PROTOCOL',
(protocol) => `protocol "${protocol}" is unsupported.`);
E('ERR_INDEX_OUT_OF_RANGE', 'Index out of range');
E('ERR_INVALID_ARG_TYPE', invalidArgType);
E('ERR_INVALID_ARRAY_LENGTH',
(name, len, actual) => {
internalAssert(typeof actual === 'number', 'actual must be a number');
return `The array "${name}" (length ${actual}) must be of length ${len}.`;
});
E('ERR_INVALID_ASYNC_ID', (type, id) => `Invalid ${type} value: ${id}`);
E('ERR_INVALID_CALLBACK', 'callback must be a function');
E('ERR_INVALID_FD', (fd) => `"fd" must be a positive integer: ${fd}`);
Expand Down Expand Up @@ -239,7 +254,7 @@ E('ERR_VALID_PERFORMANCE_ENTRY_TYPE',
// Add new errors from here...

function invalidArgType(name, expected, actual) {
assert(name, 'name is required');
internalAssert(name, 'name is required');
var msg = `The "${name}" argument must be ${oneOf(expected, 'type')}`;
if (arguments.length >= 3) {
msg += `. Received type ${actual !== null ? typeof actual : 'null'}`;
Expand All @@ -248,7 +263,7 @@ function invalidArgType(name, expected, actual) {
}

function missingArgs(...args) {
assert(args.length > 0, 'At least one arg needs to be specified');
internalAssert(args.length > 0, 'At least one arg needs to be specified');
let msg = 'The ';
const len = args.length;
args = args.map((a) => `"${a}"`);
Expand All @@ -268,11 +283,12 @@ function missingArgs(...args) {
}

function oneOf(expected, thing) {
assert(expected, 'expected is required');
assert(typeof thing === 'string', 'thing is required');
internalAssert(expected, 'expected is required');
internalAssert(typeof thing === 'string', 'thing is required');
if (Array.isArray(expected)) {
const len = expected.length;
assert(len > 0, 'At least one expected value needs to be specified');
internalAssert(len > 0,
'At least one expected value needs to be specified');
expected = expected.map((i) => String(i));
if (len > 2) {
return `one of ${thing} ${expected.slice(0, len - 1).join(', ')}, or ` +
Expand Down
37 changes: 17 additions & 20 deletions test/parallel/test-internal-errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,9 @@ const common = require('../common');
const errors = require('internal/errors');
const assert = require('assert');

const errMessages = {
objectString: /^'object' === 'string'$/,
booleanString: /^'boolean' === 'string'$/,
numberString: /^'number' === 'string'$/,
invalidKey: /^An invalid error message key was used: TEST_FOO_KEY\.$/,
};
function invalidKey(key) {
return new RegExp(`^An invalid error message key was used: ${key}\\.$`);
}

errors.E('TEST_ERROR_1', 'Error for testing purposes: %s');
errors.E('TEST_ERROR_2', (a, b) => `${a} ${b}`);
Expand Down Expand Up @@ -50,86 +47,86 @@ assert.throws(
() => new errors.Error('TEST_FOO_KEY'),
common.expectsError({
code: 'ERR_ASSERTION',
message: errMessages.invalidKey
message: invalidKey('TEST_FOO_KEY')
}));
// Calling it twice yields same result (using the key does not create it)
assert.throws(
() => new errors.Error('TEST_FOO_KEY'),
common.expectsError({
code: 'ERR_ASSERTION',
message: errMessages.invalidKey
message: invalidKey('TEST_FOO_KEY')
}));
assert.throws(
() => new errors.Error(1),
common.expectsError({
code: 'ERR_ASSERTION',
message: errMessages.numberString
message: invalidKey(1)
}));
assert.throws(
() => new errors.Error({}),
common.expectsError({
code: 'ERR_ASSERTION',
message: errMessages.objectString
message: invalidKey('\\[object Object\\]')
}));
assert.throws(
() => new errors.Error([]),
common.expectsError({
code: 'ERR_ASSERTION',
message: errMessages.objectString
message: invalidKey('')
}));
assert.throws(
() => new errors.Error(true),
common.expectsError({
code: 'ERR_ASSERTION',
message: errMessages.booleanString
message: invalidKey('true')
}));
assert.throws(
() => new errors.TypeError(1),
common.expectsError({
code: 'ERR_ASSERTION',
message: errMessages.numberString
message: invalidKey(1)
}));
assert.throws(
() => new errors.TypeError({}),
common.expectsError({
code: 'ERR_ASSERTION',
message: errMessages.objectString
message: invalidKey('\\[object Object\\]')
}));
assert.throws(
() => new errors.TypeError([]),
common.expectsError({
code: 'ERR_ASSERTION',
message: errMessages.objectString
message: invalidKey('')
}));
assert.throws(
() => new errors.TypeError(true),
common.expectsError({
code: 'ERR_ASSERTION',
message: errMessages.booleanString
message: invalidKey('true')
}));
assert.throws(
() => new errors.RangeError(1),
common.expectsError({
code: 'ERR_ASSERTION',
message: errMessages.numberString
message: invalidKey(1)
}));
assert.throws(
() => new errors.RangeError({}),
common.expectsError({
code: 'ERR_ASSERTION',
message: errMessages.objectString
message: invalidKey('\\[object Object\\]')
}));
assert.throws(
() => new errors.RangeError([]),
common.expectsError({
code: 'ERR_ASSERTION',
message: errMessages.objectString
message: invalidKey('')
}));
assert.throws(
() => new errors.RangeError(true),
common.expectsError({
code: 'ERR_ASSERTION',
message: errMessages.booleanString
message: invalidKey('true')
}));


Expand Down

0 comments on commit 4df100f

Please sign in to comment.