From 6ff521b59b966adc202431c6a9a9091419bb5030 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Wed, 23 Aug 2017 22:16:10 -0700 Subject: [PATCH] errors: eliminate circular dependency on assert PR-URL: https://github.com/nodejs/node/pull/15002 Reviewed-By: Luigi Pinca Reviewed-By: Benjamin Gruenbaum Reviewed-By: Refael Ackermann Reviewed-By: Ruben Bridgewater --- lib/internal/errors.js | 31 ++++++++++++++-------- test/parallel/test-internal-errors.js | 37 ++++++++++++--------------- 2 files changed, 38 insertions(+), 30 deletions(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index dc744d61e7..6a8fe1fb64 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -12,7 +12,6 @@ const kCode = Symbol('code'); const messages = new Map(); // Lazily loaded -var assert = null; var util = null; function makeNodeError(Base) { @@ -55,11 +54,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; @@ -188,7 +198,7 @@ E('ERR_INDEX_OUT_OF_RANGE', 'Index out of range'); E('ERR_INVALID_ARG_TYPE', invalidArgType); E('ERR_INVALID_ARRAY_LENGTH', (name, len, actual) => { - assert.strictEqual(typeof actual, 'number'); + 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}`); @@ -280,7 +290,7 @@ E('ERR_VALID_PERFORMANCE_ENTRY_TYPE', E('ERR_VALUE_OUT_OF_RANGE', 'The value of "%s" must be %s. Received "%s"'); function invalidArgType(name, expected, actual) { - assert(name, 'name is required'); + internalAssert(name, 'name is required'); // determiner: 'must be' or 'must not be' let determiner; @@ -311,7 +321,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}"`); @@ -331,11 +341,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 ` + diff --git a/test/parallel/test-internal-errors.js b/test/parallel/test-internal-errors.js index 6269460cf3..1c16823e7a 100644 --- a/test/parallel/test-internal-errors.js +++ b/test/parallel/test-internal-errors.js @@ -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}`); @@ -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') }));