From b9f1e572017c146da66077331260bac0e60c0928 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Wed, 13 Mar 2019 22:43:00 +0800 Subject: [PATCH] lib: throw a special error in internal/assert Instead of using the public AssertionError, use a simplified error that describes potential causes of these assertions and suggests the user to open an issue. PR-URL: https://github.com/nodejs/node/pull/26635 Reviewed-By: Ruben Bridgewater --- doc/api/errors.md | 6 ++++++ lib/internal/assert.js | 13 +++++++++++-- lib/internal/errors.js | 7 +++++++ test/common/index.js | 14 ++++++++++++++ test/message/internal_assert.js | 7 +++++++ test/message/internal_assert.out | 15 +++++++++++++++ test/message/internal_assert_fail.js | 7 +++++++ test/message/internal_assert_fail.out | 16 ++++++++++++++++ test/parallel/test-internal-assert.js | 10 +++------- test/parallel/test-internal-errors.js | 8 +++----- test/parallel/test-tls-basic-validations.js | 7 ++----- test/sequential/test-fs-watch.js | 18 +++++++++--------- 12 files changed, 100 insertions(+), 28 deletions(-) create mode 100644 test/message/internal_assert.js create mode 100644 test/message/internal_assert.out create mode 100644 test/message/internal_assert_fail.js create mode 100644 test/message/internal_assert_fail.out diff --git a/doc/api/errors.md b/doc/api/errors.md index a21b4ac852d..79570ffb510 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -1148,6 +1148,12 @@ is set for the `Http2Stream`. `http2.connect()` was passed a URL that uses any protocol other than `http:` or `https:`. + +### ERR_INTERNAL_ASSERTION + +There was a bug in Node.js or incorrect usage of Node.js internals. +To fix the error, open an issue at https://github.com/nodejs/node/issues. + ### ERR_INCOMPATIBLE_OPTION_PAIR diff --git a/lib/internal/assert.js b/lib/internal/assert.js index e403fd4b60e..b62b2753c0b 100644 --- a/lib/internal/assert.js +++ b/lib/internal/assert.js @@ -1,13 +1,22 @@ 'use strict'; +let error; +function lazyError() { + if (!error) { + error = require('internal/errors').codes.ERR_INTERNAL_ASSERTION; + } + return error; +} function assert(value, message) { if (!value) { - require('assert')(value, message); + const ERR_INTERNAL_ASSERTION = lazyError(); + throw new ERR_INTERNAL_ASSERTION(message); } } function fail(message) { - require('assert').fail(message); + const ERR_INTERNAL_ASSERTION = lazyError(); + throw new ERR_INTERNAL_ASSERTION(message); } assert.fail = fail; diff --git a/lib/internal/errors.js b/lib/internal/errors.js index ef8ed8b9a16..2cdf40e2092 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -798,6 +798,13 @@ E('ERR_INSPECTOR_CLOSED', 'Session was closed', Error); E('ERR_INSPECTOR_COMMAND', 'Inspector error %d: %s', Error); E('ERR_INSPECTOR_NOT_AVAILABLE', 'Inspector is not available', Error); E('ERR_INSPECTOR_NOT_CONNECTED', 'Session is not connected', Error); +E('ERR_INTERNAL_ASSERTION', (message) => { + const suffix = 'This is caused by either a bug in Node.js ' + + 'or incorrect usage of Node.js internals.\n' + + 'Please open an issue with this stack trace at ' + + 'https://github.com/nodejs/node/issues\n'; + return message === undefined ? suffix : `${message}\n${suffix}`; +}, Error); E('ERR_INVALID_ADDRESS_FAMILY', function(addressType, host, port) { this.host = host; this.port = port; diff --git a/test/common/index.js b/test/common/index.js index 37327b916c5..02fe9039dd8 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -626,6 +626,19 @@ function expectsError(fn, settings, exact) { return mustCall(innerFn, exact); } +const suffix = 'This is caused by either a bug in Node.js ' + + 'or incorrect usage of Node.js internals.\n' + + 'Please open an issue with this stack trace at ' + + 'https://github.com/nodejs/node/issues\n'; + +function expectsInternalAssertion(fn, message) { + assert.throws(fn, { + message: `${message}\n${suffix}`, + name: 'Error', + code: 'ERR_INTERNAL_ASSERTION' + }); +} + function skipIfInspectorDisabled() { if (!process.features.inspector) { skip('V8 inspector is disabled'); @@ -729,6 +742,7 @@ module.exports = { enoughTestCpu, enoughTestMem, expectsError, + expectsInternalAssertion, expectWarning, getArrayBufferViews, getBufferSources, diff --git a/test/message/internal_assert.js b/test/message/internal_assert.js new file mode 100644 index 00000000000..fdb459b67ca --- /dev/null +++ b/test/message/internal_assert.js @@ -0,0 +1,7 @@ +'use strict'; + +// Flags: --expose-internals +require('../common'); + +const assert = require('internal/assert'); +assert(false); diff --git a/test/message/internal_assert.out b/test/message/internal_assert.out new file mode 100644 index 00000000000..ae8de3e1a0d --- /dev/null +++ b/test/message/internal_assert.out @@ -0,0 +1,15 @@ +internal/assert.js:* + throw new ERR_INTERNAL_ASSERTION(message); + ^ + +Error [ERR_INTERNAL_ASSERTION]: This is caused by either a bug in Node.js or incorrect usage of Node.js internals. +Please open an issue with this stack trace at https://github.com/nodejs/node/issues + + at assert (internal/assert.js:*:*) + at * (*test*message*internal_assert.js:7:1) + at * + at * + at * + at * + at * + at * diff --git a/test/message/internal_assert_fail.js b/test/message/internal_assert_fail.js new file mode 100644 index 00000000000..1b2cf135528 --- /dev/null +++ b/test/message/internal_assert_fail.js @@ -0,0 +1,7 @@ +'use strict'; + +// Flags: --expose-internals +require('../common'); + +const assert = require('internal/assert'); +assert.fail('Unreachable!'); diff --git a/test/message/internal_assert_fail.out b/test/message/internal_assert_fail.out new file mode 100644 index 00000000000..70f49ad33aa --- /dev/null +++ b/test/message/internal_assert_fail.out @@ -0,0 +1,16 @@ +internal/assert.js:* + throw new ERR_INTERNAL_ASSERTION(message); + ^ + +Error [ERR_INTERNAL_ASSERTION]: Unreachable! +This is caused by either a bug in Node.js or incorrect usage of Node.js internals. +Please open an issue with this stack trace at https://github.com/nodejs/node/issues + + at Function.fail (internal/assert.js:*:*) + at * (*test*message*internal_assert_fail.js:7:8) + at * + at * + at * + at * + at * + at * diff --git a/test/parallel/test-internal-assert.js b/test/parallel/test-internal-assert.js index 4fd443864bd..18528e9b277 100644 --- a/test/parallel/test-internal-assert.js +++ b/test/parallel/test-internal-assert.js @@ -1,17 +1,13 @@ // Flags: --expose-internals 'use strict'; +// This tests that the internal assert module works as expected. +// The failures are tested in test/message. + require('../common'); -const assert = require('assert'); const internalAssert = require('internal/assert'); // Should not throw. internalAssert(true); internalAssert(true, 'fhqwhgads'); - -assert.throws(() => { internalAssert(false); }, assert.AssertionError); -assert.throws(() => { internalAssert(false, 'fhqwhgads'); }, - { code: 'ERR_ASSERTION', message: 'fhqwhgads' }); -assert.throws(() => { internalAssert.fail('fhqwhgads'); }, - { code: 'ERR_ASSERTION', message: 'fhqwhgads' }); diff --git a/test/parallel/test-internal-errors.js b/test/parallel/test-internal-errors.js index 53fbf06c77e..10d79cb8faf 100644 --- a/test/parallel/test-internal-errors.js +++ b/test/parallel/test-internal-errors.js @@ -50,12 +50,10 @@ errors.E('TEST_ERROR_2', (a, b) => `${a} ${b}`, Error); } { - assert.throws( + common.expectsInternalAssertion( () => new errors.codes.TEST_ERROR_1(), - { - message: 'Code: TEST_ERROR_1; The provided arguments ' + - 'length (0) does not match the required ones (1).' - } + 'Code: TEST_ERROR_1; The provided arguments ' + + 'length (0) does not match the required ones (1).' ); } diff --git a/test/parallel/test-tls-basic-validations.js b/test/parallel/test-tls-basic-validations.js index 9bcc63c4545..925c6643a1a 100644 --- a/test/parallel/test-tls-basic-validations.js +++ b/test/parallel/test-tls-basic-validations.js @@ -78,12 +78,9 @@ common.expectsError( assert.throws(() => tls.createServer({ ticketKeys: Buffer.alloc(0) }), /TypeError: Ticket keys length must be 48 bytes/); -common.expectsError( +common.expectsInternalAssertion( () => tls.createSecurePair({}), - { - code: 'ERR_ASSERTION', - message: 'context.context must be a NativeSecureContext' - } + 'context.context must be a NativeSecureContext' ); { diff --git a/test/sequential/test-fs-watch.js b/test/sequential/test-fs-watch.js index a67de8c6d1c..fd0788dac9b 100644 --- a/test/sequential/test-fs-watch.js +++ b/test/sequential/test-fs-watch.js @@ -115,14 +115,14 @@ tmpdir.refresh(); // https://github.com/joyent/node/issues/6690 { let oldhandle; - assert.throws(() => { - const w = fs.watch(__filename, common.mustNotCall()); - oldhandle = w._handle; - w._handle = { close: w._handle.close }; - w.close(); - }, { - message: 'handle must be a FSEvent', - code: 'ERR_ASSERTION' - }); + common.expectsInternalAssertion( + () => { + const w = fs.watch(__filename, common.mustNotCall()); + oldhandle = w._handle; + w._handle = { close: w._handle.close }; + w.close(); + }, + 'handle must be a FSEvent' + ); oldhandle.close(); // clean up }