From 5b0038accee6119ada172b58f6113ab67c923654 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Wed, 2 May 2018 16:13:16 +0200 Subject: [PATCH 1/3] doc: update assert documentation This adds concrete expected types to the assert documentation. It also fixes a `changes` entry and improves some minor comments. --- doc/api/assert.md | 50 +++++++++++++++++++++++------------------------ 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/doc/api/assert.md b/doc/api/assert.md index 43e5800ff031b8..30fd8d0d07048c 100644 --- a/doc/api/assert.md +++ b/doc/api/assert.md @@ -154,8 +154,8 @@ assert.deepEqual(/a/gi, new Date()); -* `value` {any} -* `message` {any} +* `value` {any} The input that is checked for being truthy. +* `message` {string|Error} An alias of [`assert.ok()`][]. @@ -181,7 +181,7 @@ changes: --> * `actual` {any} * `expected` {any} -* `message` {any} +* `message` {string|Error} **Strict mode** @@ -235,18 +235,18 @@ const obj3 = { const obj4 = Object.create(obj1); assert.deepEqual(obj1, obj1); -// OK, object is equal to itself +// OK +// Values of b are different: assert.deepEqual(obj1, obj2); // AssertionError: { a: { b: 1 } } deepEqual { a: { b: 2 } } -// values of b are different assert.deepEqual(obj1, obj3); -// OK, objects are equal +// OK +// Prototypes are ignored: assert.deepEqual(obj1, obj4); // AssertionError: { a: { b: 1 } } deepEqual {} -// Prototypes are ignored ``` If the values are not equal, an `AssertionError` is thrown with a `message` @@ -285,7 +285,7 @@ changes: --> * `actual` {any} * `expected` {any} -* `message` {any} +* `message` {string|Error} Tests for deep equality between the `actual` and `expected` parameters. "Deep" equality means that the enumerable "own" properties of child objects @@ -406,7 +406,7 @@ added: v10.0.0 --> * `block` {Function|Promise} * `error` {RegExp|Function} -* `message` {any} +* `message` {string|Error} Awaits the `block` promise or, if `block` is a function, immediately calls the function and awaits the returned promise to complete. It will then check that @@ -460,7 +460,7 @@ changes: --> * `block` {Function} * `error` {RegExp|Function} -* `message` {any} +* `message` {string|Error} Asserts that the function `block` does not throw an error. @@ -528,7 +528,7 @@ added: v0.1.21 --> * `actual` {any} * `expected` {any} -* `message` {any} +* `message` {string|Error} **Strict mode** @@ -565,7 +565,7 @@ parameter is an instance of an [`Error`][] then it will be thrown instead of the -* `message` {any} **Default:** `'Failed'` +* `message` {string|Error} **Default:** `'Failed'` Throws an `AssertionError` with the provided error message or a default error message. If the `message` parameter is an instance of an [`Error`][] then it @@ -598,7 +598,7 @@ changes: --> * `actual` {any} * `expected` {any} -* `message` {any} +* `message` {string|Error} * `operator` {string} **Default:** `'!='` * `stackStartFunction` {Function} **Default:** `assert.fail` @@ -659,8 +659,8 @@ changes: an `AssertionError` that contains the full stack trace. - version: v10.0.0 pr-url: https://github.com/nodejs/node/pull/18247 - description: Value may now only be `undefined` or `null`. Before any truthy - input was accepted. + description: Value may now only be `undefined` or `null`. Before all falsy + values were handled the same as `null` and did not throw. --> * `value` {any} @@ -717,7 +717,7 @@ changes: --> * `actual` {any} * `expected` {any} -* `message` {any} +* `message` {string|Error} **Strict mode** @@ -753,13 +753,13 @@ assert.notDeepEqual(obj1, obj1); // AssertionError: { a: { b: 1 } } notDeepEqual { a: { b: 1 } } assert.notDeepEqual(obj1, obj2); -// OK: obj1 and obj2 are not deeply equal +// OK assert.notDeepEqual(obj1, obj3); // AssertionError: { a: { b: 1 } } notDeepEqual { a: { b: 1 } } assert.notDeepEqual(obj1, obj4); -// OK: obj1 and obj4 are not deeply equal +// OK ``` If the values are deeply equal, an `AssertionError` is thrown with a `message` @@ -798,7 +798,7 @@ changes: --> * `actual` {any} * `expected` {any} -* `message` {any} +* `message` {string|Error} Tests for deep strict inequality. Opposite of [`assert.deepStrictEqual()`][]. @@ -821,7 +821,7 @@ added: v0.1.21 --> * `actual` {any} * `expected` {any} -* `message` {any} +* `message` {string|Error} **Strict mode** @@ -863,7 +863,7 @@ changes: --> * `actual` {any} * `expected` {any} -* `message` {any} +* `message` {string|Error} Tests strict inequality between the `actual` and `expected` parameters as determined by the [SameValue Comparison][]. @@ -897,7 +897,7 @@ changes: error message. --> * `value` {any} -* `message` {any} +* `message` {string|Error} Tests if `value` is truthy. It is equivalent to `assert.equal(!!value, true, message)`. @@ -960,7 +960,7 @@ added: v10.0.0 --> * `block` {Function|Promise} * `error` {RegExp|Function|Object|Error} -* `message` {any} +* `message` {string|Error} Awaits the `block` promise or, if `block` is a function, immediately calls the function and awaits the returned promise to complete. It will then check that @@ -1022,7 +1022,7 @@ changes: --> * `actual` {any} * `expected` {any} -* `message` {any} +* `message` {string|Error} Tests strict equality between the `actual` and `expected` parameters as determined by the [SameValue Comparison][]. @@ -1065,7 +1065,7 @@ changes: --> * `block` {Function} * `error` {RegExp|Function|Object|Error} -* `message` {any} +* `message` {string|Error} Expects the function `block` to throw an error. From 9fce09589c5ffceab078183a81e3943086350387 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Wed, 2 May 2018 16:21:40 +0200 Subject: [PATCH 2/3] assert: move AssertionError into own file This moves the `assert` parts from `internal/errors` into an own file. `internal/errors` got bigger and bigger and it was difficult to keep a good overview of what was going on. While doing so it also removes the `internalAssert` function and just lazy loads `assert`. --- lib/assert.js | 15 +- lib/internal/assert.js | 275 ++++++++++++++++++++++++++++++ lib/internal/errors.js | 314 ++--------------------------------- node.gyp | 1 + test/parallel/test-assert.js | 2 +- 5 files changed, 301 insertions(+), 306 deletions(-) create mode 100644 lib/internal/assert.js diff --git a/lib/assert.js b/lib/assert.js index 05b43b5b54d074..1c0fcee87e4131 100644 --- a/lib/assert.js +++ b/lib/assert.js @@ -25,15 +25,12 @@ const { isDeepEqual, isDeepStrictEqual } = require('internal/util/comparisons'); -const { - AssertionError, - errorCache, - codes: { - ERR_AMBIGUOUS_ARGUMENT, - ERR_INVALID_ARG_TYPE, - ERR_INVALID_RETURN_VALUE - } -} = require('internal/errors'); +const { codes: { + ERR_AMBIGUOUS_ARGUMENT, + ERR_INVALID_ARG_TYPE, + ERR_INVALID_RETURN_VALUE +} } = require('internal/errors'); +const { AssertionError, errorCache } = require('internal/assert'); const { openSync, closeSync, readSync } = require('fs'); const { inspect, types: { isPromise } } = require('util'); const { EOL } = require('os'); diff --git a/lib/internal/assert.js b/lib/internal/assert.js new file mode 100644 index 00000000000000..990065a9378f14 --- /dev/null +++ b/lib/internal/assert.js @@ -0,0 +1,275 @@ +'use strict'; + +const { inspect } = require('util'); +const { codes: { + ERR_INVALID_ARG_TYPE +} } = require('internal/errors'); + +let blue = ''; +let green = ''; +let red = ''; +let white = ''; + +const READABLE_OPERATOR = { + deepStrictEqual: 'Input A expected to strictly deep-equal input B', + notDeepStrictEqual: 'Input A expected to strictly not deep-equal input B', + strictEqual: 'Input A expected to strictly equal input B', + notStrictEqual: 'Input A expected to strictly not equal input B' +}; + +function copyError(source) { + const keys = Object.keys(source); + const target = Object.create(Object.getPrototypeOf(source)); + for (const key of keys) { + target[key] = source[key]; + } + Object.defineProperty(target, 'message', { value: source.message }); + return target; +} + +function inspectValue(val) { + // The util.inspect default values could be changed. This makes sure the + // error messages contain the necessary information nevertheless. + return inspect( + val, + { + compact: false, + customInspect: false, + depth: 1000, + maxArrayLength: Infinity, + // Assert compares only enumerable properties (with a few exceptions). + showHidden: false, + // Having a long line as error is better than wrapping the line for + // comparison. + breakLength: Infinity, + // Assert does not detect proxies currently. + showProxy: false + } + ).split('\n'); +} + +function createErrDiff(actual, expected, operator) { + var other = ''; + var res = ''; + var lastPos = 0; + var end = ''; + var skipped = false; + const actualLines = inspectValue(actual); + const expectedLines = inspectValue(expected); + const msg = READABLE_OPERATOR[operator] + + `:\n${green}+ expected${white} ${red}- actual${white}`; + const skippedMsg = ` ${blue}...${white} Lines skipped`; + + // Remove all ending lines that match (this optimizes the output for + // readability by reducing the number of total changed lines). + var a = actualLines[actualLines.length - 1]; + var b = expectedLines[expectedLines.length - 1]; + var i = 0; + while (a === b) { + if (i++ < 2) { + end = `\n ${a}${end}`; + } else { + other = a; + } + actualLines.pop(); + expectedLines.pop(); + if (actualLines.length === 0 || expectedLines.length === 0) + break; + a = actualLines[actualLines.length - 1]; + b = expectedLines[expectedLines.length - 1]; + } + if (i > 3) { + end = `\n${blue}...${white}${end}`; + skipped = true; + } + if (other !== '') { + end = `\n ${other}${end}`; + other = ''; + } + + const maxLines = Math.max(actualLines.length, expectedLines.length); + var printedLines = 0; + var identical = 0; + for (i = 0; i < maxLines; i++) { + // Only extra expected lines exist + const cur = i - lastPos; + if (actualLines.length < i + 1) { + if (cur > 1 && i > 2) { + if (cur > 4) { + res += `\n${blue}...${white}`; + skipped = true; + } else if (cur > 3) { + res += `\n ${expectedLines[i - 2]}`; + printedLines++; + } + res += `\n ${expectedLines[i - 1]}`; + printedLines++; + } + lastPos = i; + other += `\n${green}+${white} ${expectedLines[i]}`; + printedLines++; + // Only extra actual lines exist + } else if (expectedLines.length < i + 1) { + if (cur > 1 && i > 2) { + if (cur > 4) { + res += `\n${blue}...${white}`; + skipped = true; + } else if (cur > 3) { + res += `\n ${actualLines[i - 2]}`; + printedLines++; + } + res += `\n ${actualLines[i - 1]}`; + printedLines++; + } + lastPos = i; + res += `\n${red}-${white} ${actualLines[i]}`; + printedLines++; + // Lines diverge + } else if (actualLines[i] !== expectedLines[i]) { + if (cur > 1 && i > 2) { + if (cur > 4) { + res += `\n${blue}...${white}`; + skipped = true; + } else if (cur > 3) { + res += `\n ${actualLines[i - 2]}`; + printedLines++; + } + res += `\n ${actualLines[i - 1]}`; + printedLines++; + } + lastPos = i; + res += `\n${red}-${white} ${actualLines[i]}`; + other += `\n${green}+${white} ${expectedLines[i]}`; + printedLines += 2; + // Lines are identical + } else { + res += other; + other = ''; + if (cur === 1 || i === 0) { + res += `\n ${actualLines[i]}`; + printedLines++; + } + identical++; + } + // Inspected object to big (Show ~20 rows max) + if (printedLines > 20 && i < maxLines - 2) { + return `${msg}${skippedMsg}\n${res}\n${blue}...${white}${other}\n` + + `${blue}...${white}`; + } + } + + // Strict equal with identical objects that are not identical by reference. + if (identical === maxLines) { + // E.g., assert.deepStrictEqual(Symbol(), Symbol()) + const base = operator === 'strictEqual' ? + 'Input objects identical but not reference equal:' : + 'Input objects not identical:'; + + // We have to get the result again. The lines were all removed before. + const actualLines = inspectValue(actual); + + // Only remove lines in case it makes sense to collapse those. + // TODO: Accept env to always show the full error. + if (actualLines.length > 30) { + actualLines[26] = `${blue}...${white}`; + while (actualLines.length > 27) { + actualLines.pop(); + } + } + + return `${base}\n\n${actualLines.join('\n')}\n`; + } + return `${msg}${skipped ? skippedMsg : ''}\n${res}${other}${end}`; +} + +class AssertionError extends Error { + constructor(options) { + if (typeof options !== 'object' || options === null) { + throw new ERR_INVALID_ARG_TYPE('options', 'Object', options); + } + var { + actual, + expected, + message, + operator, + stackStartFn + } = options; + + if (message != null) { + super(message); + } else { + if (process.stdout.isTTY) { + // Reset on each call to make sure we handle dynamically set environment + // variables correct. + if (process.stdout.getColorDepth() !== 1) { + blue = '\u001b[34m'; + green = '\u001b[32m'; + white = '\u001b[39m'; + red = '\u001b[31m'; + } else { + blue = ''; + green = ''; + white = ''; + red = ''; + } + } + // Prevent the error stack from being visible by duplicating the error + // in a very close way to the original in case both sides are actually + // instances of Error. + if (typeof actual === 'object' && actual !== null && + typeof expected === 'object' && expected !== null && + 'stack' in actual && actual instanceof Error && + 'stack' in expected && expected instanceof Error) { + actual = copyError(actual); + expected = copyError(expected); + } + + if (operator === 'deepStrictEqual' || operator === 'strictEqual') { + super(createErrDiff(actual, expected, operator)); + } else if (operator === 'notDeepStrictEqual' || + operator === 'notStrictEqual') { + // In case the objects are equal but the operator requires unequal, show + // the first object and say A equals B + const res = inspectValue(actual); + const base = `Identical input passed to ${operator}:`; + + // Only remove lines in case it makes sense to collapse those. + // TODO: Accept env to always show the full error. + if (res.length > 30) { + res[26] = `${blue}...${white}`; + while (res.length > 27) { + res.pop(); + } + } + + // Only print a single input. + if (res.length === 1) { + super(`${base} ${res[0]}`); + } else { + super(`${base}\n\n${res.join('\n')}\n`); + } + } else { + let res = inspect(actual); + let other = inspect(expected); + if (res.length > 128) + res = `${res.slice(0, 125)}...`; + if (other.length > 128) + other = `${other.slice(0, 125)}...`; + super(`${res} ${operator} ${other}`); + } + } + + this.generatedMessage = !message; + this.name = 'AssertionError [ERR_ASSERTION]'; + this.code = 'ERR_ASSERTION'; + this.actual = actual; + this.expected = expected; + this.operator = operator; + Error.captureStackTrace(this, stackStartFn); + } +} + +module.exports = { + AssertionError, + errorCache: new Map() +}; diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 0228448dbf3f58..a0976eb4604485 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -15,18 +15,6 @@ const kInfo = Symbol('info'); const messages = new Map(); const codes = {}; -let blue = ''; -let green = ''; -let red = ''; -let white = ''; - -const READABLE_OPERATOR = { - deepStrictEqual: 'Input A expected to strictly deep-equal input B', - notDeepStrictEqual: 'Input A expected to strictly not deep-equal input B', - strictEqual: 'Input A expected to strictly equal input B', - notStrictEqual: 'Input A expected to strictly not equal input B' -}; - const { errmap, UV_EAI_NODATA, @@ -36,10 +24,10 @@ const { kMaxLength } = process.binding('buffer'); const { defineProperty } = Object; // Lazily loaded -var util; -var buffer; +let util; +let assert; -var internalUtil = null; +let internalUtil = null; function lazyInternalUtil() { if (!internalUtil) { internalUtil = require('internal/util'); @@ -47,35 +35,11 @@ function lazyInternalUtil() { return internalUtil; } -function copyError(source) { - const keys = Object.keys(source); - const target = Object.create(Object.getPrototypeOf(source)); - for (const key of keys) { - target[key] = source[key]; - } - Object.defineProperty(target, 'message', { value: source.message }); - return target; -} - -function inspectValue(val) { - // The util.inspect default values could be changed. This makes sure the - // error messages contain the necessary information nevertheless. - return util.inspect( - val, - { - compact: false, - customInspect: false, - depth: 1000, - maxArrayLength: Infinity, - // Assert compares only enumerable properties (with a few exceptions). - showHidden: false, - // Having a long line as error is better than wrapping the line for - // comparison. - breakLength: Infinity, - // Assert does not detect proxies currently. - showProxy: false - } - ).split('\n'); +let buffer; +function lazyBuffer() { + if (buffer === undefined) + buffer = require('buffer').Buffer; + return buffer; } // A specialized Error that includes an additional info property with @@ -240,254 +204,14 @@ function E(sym, val, def, ...otherClasses) { codes[sym] = def; } -function lazyBuffer() { - if (buffer === undefined) - buffer = require('buffer').Buffer; - return buffer; -} - -function createErrDiff(actual, expected, operator) { - var other = ''; - var res = ''; - var lastPos = 0; - var end = ''; - var skipped = false; - if (util === undefined) util = require('util'); - const actualLines = inspectValue(actual); - const expectedLines = inspectValue(expected); - const msg = READABLE_OPERATOR[operator] + - `:\n${green}+ expected${white} ${red}- actual${white}`; - const skippedMsg = ` ${blue}...${white} Lines skipped`; - - // Remove all ending lines that match (this optimizes the output for - // readability by reducing the number of total changed lines). - var a = actualLines[actualLines.length - 1]; - var b = expectedLines[expectedLines.length - 1]; - var i = 0; - while (a === b) { - if (i++ < 2) { - end = `\n ${a}${end}`; - } else { - other = a; - } - actualLines.pop(); - expectedLines.pop(); - if (actualLines.length === 0 || expectedLines.length === 0) - break; - a = actualLines[actualLines.length - 1]; - b = expectedLines[expectedLines.length - 1]; - } - if (i > 3) { - end = `\n${blue}...${white}${end}`; - skipped = true; - } - if (other !== '') { - end = `\n ${other}${end}`; - other = ''; - } - - const maxLines = Math.max(actualLines.length, expectedLines.length); - var printedLines = 0; - var identical = 0; - for (i = 0; i < maxLines; i++) { - // Only extra expected lines exist - const cur = i - lastPos; - if (actualLines.length < i + 1) { - if (cur > 1 && i > 2) { - if (cur > 4) { - res += `\n${blue}...${white}`; - skipped = true; - } else if (cur > 3) { - res += `\n ${expectedLines[i - 2]}`; - printedLines++; - } - res += `\n ${expectedLines[i - 1]}`; - printedLines++; - } - lastPos = i; - other += `\n${green}+${white} ${expectedLines[i]}`; - printedLines++; - // Only extra actual lines exist - } else if (expectedLines.length < i + 1) { - if (cur > 1 && i > 2) { - if (cur > 4) { - res += `\n${blue}...${white}`; - skipped = true; - } else if (cur > 3) { - res += `\n ${actualLines[i - 2]}`; - printedLines++; - } - res += `\n ${actualLines[i - 1]}`; - printedLines++; - } - lastPos = i; - res += `\n${red}-${white} ${actualLines[i]}`; - printedLines++; - // Lines diverge - } else if (actualLines[i] !== expectedLines[i]) { - if (cur > 1 && i > 2) { - if (cur > 4) { - res += `\n${blue}...${white}`; - skipped = true; - } else if (cur > 3) { - res += `\n ${actualLines[i - 2]}`; - printedLines++; - } - res += `\n ${actualLines[i - 1]}`; - printedLines++; - } - lastPos = i; - res += `\n${red}-${white} ${actualLines[i]}`; - other += `\n${green}+${white} ${expectedLines[i]}`; - printedLines += 2; - // Lines are identical - } else { - res += other; - other = ''; - if (cur === 1 || i === 0) { - res += `\n ${actualLines[i]}`; - printedLines++; - } - identical++; - } - // Inspected object to big (Show ~20 rows max) - if (printedLines > 20 && i < maxLines - 2) { - return `${msg}${skippedMsg}\n${res}\n${blue}...${white}${other}\n` + - `${blue}...${white}`; - } - } - - // Strict equal with identical objects that are not identical by reference. - if (identical === maxLines) { - // E.g., assert.deepStrictEqual(Symbol(), Symbol()) - const base = operator === 'strictEqual' ? - 'Input objects identical but not reference equal:' : - 'Input objects not identical:'; - - // We have to get the result again. The lines were all removed before. - const actualLines = inspectValue(actual); - - // Only remove lines in case it makes sense to collapse those. - // TODO: Accept env to always show the full error. - if (actualLines.length > 30) { - actualLines[26] = `${blue}...${white}`; - while (actualLines.length > 27) { - actualLines.pop(); - } - } - - return `${base}\n\n${actualLines.join('\n')}\n`; - } - return `${msg}${skipped ? skippedMsg : ''}\n${res}${other}${end}`; -} - -class AssertionError extends Error { - constructor(options) { - if (typeof options !== 'object' || options === null) { - throw new codes.ERR_INVALID_ARG_TYPE('options', 'Object', options); - } - var { - actual, - expected, - message, - operator, - stackStartFn - } = options; - - if (message != null) { - super(message); - } else { - if (process.stdout.isTTY) { - // Reset on each call to make sure we handle dynamically set environment - // variables correct. - if (process.stdout.getColorDepth() !== 1) { - blue = '\u001b[34m'; - green = '\u001b[32m'; - white = '\u001b[39m'; - red = '\u001b[31m'; - } else { - blue = ''; - green = ''; - white = ''; - red = ''; - } - } - if (util === undefined) util = require('util'); - // Prevent the error stack from being visible by duplicating the error - // in a very close way to the original in case both sides are actually - // instances of Error. - if (typeof actual === 'object' && actual !== null && - typeof expected === 'object' && expected !== null && - 'stack' in actual && actual instanceof Error && - 'stack' in expected && expected instanceof Error) { - actual = copyError(actual); - expected = copyError(expected); - } - - if (operator === 'deepStrictEqual' || operator === 'strictEqual') { - super(createErrDiff(actual, expected, operator)); - } else if (operator === 'notDeepStrictEqual' || - operator === 'notStrictEqual') { - // In case the objects are equal but the operator requires unequal, show - // the first object and say A equals B - const res = inspectValue(actual); - const base = `Identical input passed to ${operator}:`; - - // Only remove lines in case it makes sense to collapse those. - // TODO: Accept env to always show the full error. - if (res.length > 30) { - res[26] = `${blue}...${white}`; - while (res.length > 27) { - res.pop(); - } - } - - // Only print a single input. - if (res.length === 1) { - super(`${base} ${res[0]}`); - } else { - super(`${base}\n\n${res.join('\n')}\n`); - } - } else { - let res = util.inspect(actual); - let other = util.inspect(expected); - if (res.length > 128) - res = `${res.slice(0, 125)}...`; - if (other.length > 128) - other = `${other.slice(0, 125)}...`; - super(`${res} ${operator} ${other}`); - } - } - - this.generatedMessage = !message; - this.name = 'AssertionError [ERR_ASSERTION]'; - this.code = 'ERR_ASSERTION'; - this.actual = actual; - this.expected = expected; - this.operator = operator; - Error.captureStackTrace(this, stackStartFn); - } -} - -// 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 getMessage(key, args) { const msg = messages.get(key); + if (util === undefined) util = require('util'); + if (assert === undefined) assert = require('assert'); if (typeof msg === 'function') { - internalAssert( + assert( msg.length <= args.length, // Default options do not count. `Code: ${key}; The provided arguments length (${args.length}) does not ` + `match the required ones (${msg.length}).` @@ -496,7 +220,7 @@ function getMessage(key, args) { } const expectedLength = (msg.match(/%[dfijoOs]/g) || []).length; - internalAssert( + assert( expectedLength === args.length, `Code: ${key}; The provided arguments length (${args.length}) does not ` + `match the required ones (${expectedLength}).` @@ -690,11 +414,9 @@ module.exports = { uvException, isStackOverflowError, getMessage, - AssertionError, SystemError, codes, - E, // This is exported only to facilitate testing. - errorCache: new Map() // This is in here only to facilitate testing. + E // This is exported only to facilitate testing. }; // To declare an error message, use the E(sym, val, def) function above. The sym @@ -1042,7 +764,7 @@ E('ERR_VM_MODULE_STATUS', 'Module status %s', Error); E('ERR_ZLIB_INITIALIZATION_FAILED', 'Initialization failed', Error); function invalidArgType(name, expected, actual) { - internalAssert(typeof name === 'string', 'name must be a string'); + assert(typeof name === 'string', "'name' must be a string"); // determiner: 'must be' or 'must not be' let determiner; @@ -1068,7 +790,7 @@ function invalidArgType(name, expected, actual) { } function missingArgs(...args) { - internalAssert(args.length > 0, 'At least one arg needs to be specified'); + assert(args.length > 0, 'At least one arg needs to be specified'); let msg = 'The '; const len = args.length; args = args.map((a) => `"${a}"`); @@ -1088,11 +810,11 @@ function missingArgs(...args) { } function oneOf(expected, thing) { - internalAssert(typeof thing === 'string', '`thing` has to be of type string'); + assert(typeof thing === 'string', '`thing` has to be of type string'); if (Array.isArray(expected)) { const len = expected.length; - internalAssert(len > 0, - 'At least one expected value needs to be specified'); + assert(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/node.gyp b/node.gyp index 3e513a4a71ceb8..afd9eb5454c82d 100644 --- a/node.gyp +++ b/node.gyp @@ -80,6 +80,7 @@ 'lib/v8.js', 'lib/vm.js', 'lib/zlib.js', + 'lib/internal/assert.js', 'lib/internal/async_hooks.js', 'lib/internal/buffer.js', 'lib/internal/cli_table.js', diff --git a/test/parallel/test-assert.js b/test/parallel/test-assert.js index d892122ed62ff2..d5cc405c690728 100644 --- a/test/parallel/test-assert.js +++ b/test/parallel/test-assert.js @@ -29,7 +29,7 @@ const common = require('../common'); const assert = require('assert'); const { EOL } = require('os'); const EventEmitter = require('events'); -const { errorCache } = require('internal/errors'); +const { errorCache } = require('internal/assert'); const { writeFileSync, unlinkSync } = require('fs'); const { inspect } = require('util'); const a = assert; From 4c9c77d9954d74541f597fb39afc3776985a971c Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Wed, 2 May 2018 16:30:54 +0200 Subject: [PATCH 3/3] errors: move functions to error code This makes sure the functions are actually directly beneat the specification of an error code. That way it is not necessary to jump around when looking at the functionality. --- lib/internal/errors.js | 189 ++++++++++++++++++++--------------------- 1 file changed, 92 insertions(+), 97 deletions(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index a0976eb4604485..2c2d8c785c46fb 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -407,6 +407,26 @@ function isStackOverflowError(err) { err.message === maxStack_ErrorMessage; } +function oneOf(expected, thing) { + assert(typeof thing === 'string', '`thing` has to be of type string'); + if (Array.isArray(expected)) { + const len = expected.length; + assert(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 ` + + expected[len - 1]; + } else if (len === 2) { + return `one of ${thing} ${expected[0]} or ${expected[1]}`; + } else { + return `of ${thing} ${expected[0]}`; + } + } else { + return `of ${thing} ${String(expected)}`; + } +} + module.exports = { dnsException, errnoException, @@ -441,7 +461,15 @@ E('ERR_ARG_NOT_ITERABLE', '%s must be iterable', TypeError); E('ERR_ASSERTION', '%s', Error); E('ERR_ASYNC_CALLBACK', '%s must be a function', TypeError); E('ERR_ASYNC_TYPE', 'Invalid name for async "type": %s', TypeError); -E('ERR_BUFFER_OUT_OF_BOUNDS', bufferOutOfBounds, RangeError); +E('ERR_BUFFER_OUT_OF_BOUNDS', + // Using a default argument here is important so the argument is not counted + // towards `Function#length`. + (name = undefined) => { + if (name) { + return `"${name}" is outside of buffer bounds`; + } + return 'Attempt to write outside buffer bounds'; + }, RangeError); E('ERR_BUFFER_TOO_LARGE', `Cannot create a Buffer larger than 0x${kMaxLength.toString(16)} bytes`, RangeError); @@ -579,7 +607,32 @@ E('ERR_INSPECTOR_CLOSED', 'Session was closed', Error); E('ERR_INSPECTOR_NOT_AVAILABLE', 'Inspector is not available', Error); E('ERR_INSPECTOR_NOT_CONNECTED', 'Session is not connected', Error); E('ERR_INVALID_ADDRESS_FAMILY', 'Invalid address family: %s', RangeError); -E('ERR_INVALID_ARG_TYPE', invalidArgType, TypeError); +E('ERR_INVALID_ARG_TYPE', + (name, expected, actual) => { + assert(typeof name === 'string', "'name' must be a string"); + + // determiner: 'must be' or 'must not be' + let determiner; + if (typeof expected === 'string' && expected.startsWith('not ')) { + determiner = 'must not be'; + expected = expected.replace(/^not /, ''); + } else { + determiner = 'must be'; + } + + let msg; + if (name.endsWith(' argument')) { + // For cases like 'first argument' + msg = `The ${name} ${determiner} ${oneOf(expected, 'type')}`; + } else { + const type = name.includes('.') ? 'property' : 'argument'; + msg = `The "${name}" ${type} ${determiner} ${oneOf(expected, 'type')}`; + } + + // TODO(BridgeAR): Improve the output by showing `null` and similar. + msg += `. Received type ${typeof actual}`; + return msg; + }, TypeError); E('ERR_INVALID_ARG_VALUE', (name, value, reason = 'is invalid') => { let inspected = util.inspect(value); if (inspected.length > 128) { @@ -595,7 +648,16 @@ E('ERR_INVALID_ASYNC_ID', 'Invalid %s value: %s', RangeError); E('ERR_INVALID_BUFFER_SIZE', 'Buffer size must be a multiple of %s', RangeError); E('ERR_INVALID_CALLBACK', 'Callback must be a function', TypeError); -E('ERR_INVALID_CHAR', invalidChar, TypeError); +E('ERR_INVALID_CHAR', + // Using a default argument here is important so the argument is not counted + // towards `Function#length`. + (name, field = undefined) => { + let msg = `Invalid character in ${name}`; + if (field !== undefined) { + msg += ` ["${field}"]`; + } + return msg; + }, TypeError); E('ERR_INVALID_CURSOR_POS', 'Cannot set cursor row without setting its column', TypeError); E('ERR_INVALID_FD', @@ -644,7 +706,26 @@ E('ERR_IPC_DISCONNECTED', 'IPC channel is already disconnected', Error); E('ERR_IPC_ONE_PIPE', 'Child process can have only one IPC pipe', Error); E('ERR_IPC_SYNC_FORK', 'IPC cannot be used with synchronous forks', Error); E('ERR_METHOD_NOT_IMPLEMENTED', 'The %s method is not implemented', Error); -E('ERR_MISSING_ARGS', missingArgs, TypeError); +E('ERR_MISSING_ARGS', + (...args) => { + assert(args.length > 0, 'At least one arg needs to be specified'); + let msg = 'The '; + const len = args.length; + args = args.map((a) => `"${a}"`); + switch (len) { + case 1: + msg += `${args[0]} argument`; + break; + case 2: + msg += `${args[0]} and ${args[1]} arguments`; + break; + default: + msg += args.slice(0, len - 1).join(', '); + msg += `, and ${args[len - 1]} arguments`; + break; + } + return `${msg} must be specified`; + }, TypeError); E('ERR_MISSING_MODULE', 'Cannot find module %s', Error); E('ERR_MODULE_RESOLUTION_LEGACY', '%s not found by import in %s.' + @@ -665,7 +746,13 @@ E('ERR_NO_CRYPTO', E('ERR_NO_ICU', '%s is not supported on Node.js compiled without ICU', TypeError); E('ERR_NO_LONGER_SUPPORTED', '%s is no longer supported', Error); -E('ERR_OUT_OF_RANGE', outOfRange, RangeError); +E('ERR_OUT_OF_RANGE', + (name, range, value) => { + let msg = `The value of "${name}" is out of range.`; + if (range !== undefined) msg += ` It must be ${range}.`; + msg += ` Received ${value}`; + return msg; + }, RangeError); E('ERR_REQUIRE_ESM', 'Must use import to load ES Module: %s', Error); E('ERR_SCRIPT_EXECUTION_INTERRUPTED', 'Script execution was interrupted by `SIGINT`', Error); @@ -762,95 +849,3 @@ E('ERR_VM_MODULE_NOT_MODULE', 'Provided module is not an instance of Module', Error); E('ERR_VM_MODULE_STATUS', 'Module status %s', Error); E('ERR_ZLIB_INITIALIZATION_FAILED', 'Initialization failed', Error); - -function invalidArgType(name, expected, actual) { - assert(typeof name === 'string', "'name' must be a string"); - - // determiner: 'must be' or 'must not be' - let determiner; - if (typeof expected === 'string' && expected.startsWith('not ')) { - determiner = 'must not be'; - expected = expected.replace(/^not /, ''); - } else { - determiner = 'must be'; - } - - let msg; - if (name.endsWith(' argument')) { - // For cases like 'first argument' - msg = `The ${name} ${determiner} ${oneOf(expected, 'type')}`; - } else { - const type = name.includes('.') ? 'property' : 'argument'; - msg = `The "${name}" ${type} ${determiner} ${oneOf(expected, 'type')}`; - } - - // TODO(BridgeAR): Improve the output by showing `null` and similar. - msg += `. Received type ${typeof actual}`; - return msg; -} - -function missingArgs(...args) { - assert(args.length > 0, 'At least one arg needs to be specified'); - let msg = 'The '; - const len = args.length; - args = args.map((a) => `"${a}"`); - switch (len) { - case 1: - msg += `${args[0]} argument`; - break; - case 2: - msg += `${args[0]} and ${args[1]} arguments`; - break; - default: - msg += args.slice(0, len - 1).join(', '); - msg += `, and ${args[len - 1]} arguments`; - break; - } - return `${msg} must be specified`; -} - -function oneOf(expected, thing) { - assert(typeof thing === 'string', '`thing` has to be of type string'); - if (Array.isArray(expected)) { - const len = expected.length; - assert(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 ` + - expected[len - 1]; - } else if (len === 2) { - return `one of ${thing} ${expected[0]} or ${expected[1]}`; - } else { - return `of ${thing} ${expected[0]}`; - } - } else { - return `of ${thing} ${String(expected)}`; - } -} - -// Using a default argument here is important so the argument is not counted -// towards `Function#length`. -function bufferOutOfBounds(name = undefined) { - if (name) { - return `"${name}" is outside of buffer bounds`; - } - return 'Attempt to write outside buffer bounds'; -} - -// Using a default argument here is important so the argument is not counted -// towards `Function#length`. -function invalidChar(name, field = undefined) { - let msg = `Invalid character in ${name}`; - if (field !== undefined) { - msg += ` ["${field}"]`; - } - return msg; -} - -function outOfRange(name, range, value) { - let msg = `The value of "${name}" is out of range.`; - if (range !== undefined) msg += ` It must be ${range}.`; - msg += ` Received ${value}`; - return msg; -}