Skip to content

Commit dd709fc

Browse files
joyeecheungtargos
authored andcommitted
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: #26635 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
1 parent 453510c commit dd709fc

File tree

12 files changed

+100
-28
lines changed

12 files changed

+100
-28
lines changed

doc/api/errors.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1148,6 +1148,12 @@ is set for the `Http2Stream`.
11481148
`http2.connect()` was passed a URL that uses any protocol other than `http:` or
11491149
`https:`.
11501150

1151+
<a id="ERR_INTERNAL_ASSERTION"></a>
1152+
### ERR_INTERNAL_ASSERTION
1153+
1154+
There was a bug in Node.js or incorrect usage of Node.js internals.
1155+
To fix the error, open an issue at https://github.com/nodejs/node/issues.
1156+
11511157
<a id="ERR_INCOMPATIBLE_OPTION_PAIR"></a>
11521158
### ERR_INCOMPATIBLE_OPTION_PAIR
11531159

lib/internal/assert.js

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,22 @@
11
'use strict';
22

3+
let error;
4+
function lazyError() {
5+
if (!error) {
6+
error = require('internal/errors').codes.ERR_INTERNAL_ASSERTION;
7+
}
8+
return error;
9+
}
310
function assert(value, message) {
411
if (!value) {
5-
require('assert')(value, message);
12+
const ERR_INTERNAL_ASSERTION = lazyError();
13+
throw new ERR_INTERNAL_ASSERTION(message);
614
}
715
}
816

917
function fail(message) {
10-
require('assert').fail(message);
18+
const ERR_INTERNAL_ASSERTION = lazyError();
19+
throw new ERR_INTERNAL_ASSERTION(message);
1120
}
1221

1322
assert.fail = fail;

lib/internal/errors.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -798,6 +798,13 @@ E('ERR_INSPECTOR_CLOSED', 'Session was closed', Error);
798798
E('ERR_INSPECTOR_COMMAND', 'Inspector error %d: %s', Error);
799799
E('ERR_INSPECTOR_NOT_AVAILABLE', 'Inspector is not available', Error);
800800
E('ERR_INSPECTOR_NOT_CONNECTED', 'Session is not connected', Error);
801+
E('ERR_INTERNAL_ASSERTION', (message) => {
802+
const suffix = 'This is caused by either a bug in Node.js ' +
803+
'or incorrect usage of Node.js internals.\n' +
804+
'Please open an issue with this stack trace at ' +
805+
'https://github.com/nodejs/node/issues\n';
806+
return message === undefined ? suffix : `${message}\n${suffix}`;
807+
}, Error);
801808
E('ERR_INVALID_ADDRESS_FAMILY', function(addressType, host, port) {
802809
this.host = host;
803810
this.port = port;

test/common/index.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -626,6 +626,19 @@ function expectsError(fn, settings, exact) {
626626
return mustCall(innerFn, exact);
627627
}
628628

629+
const suffix = 'This is caused by either a bug in Node.js ' +
630+
'or incorrect usage of Node.js internals.\n' +
631+
'Please open an issue with this stack trace at ' +
632+
'https://github.com/nodejs/node/issues\n';
633+
634+
function expectsInternalAssertion(fn, message) {
635+
assert.throws(fn, {
636+
message: `${message}\n${suffix}`,
637+
name: 'Error',
638+
code: 'ERR_INTERNAL_ASSERTION'
639+
});
640+
}
641+
629642
function skipIfInspectorDisabled() {
630643
if (!process.features.inspector) {
631644
skip('V8 inspector is disabled');
@@ -729,6 +742,7 @@ module.exports = {
729742
enoughTestCpu,
730743
enoughTestMem,
731744
expectsError,
745+
expectsInternalAssertion,
732746
expectWarning,
733747
getArrayBufferViews,
734748
getBufferSources,

test/message/internal_assert.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
'use strict';
2+
3+
// Flags: --expose-internals
4+
require('../common');
5+
6+
const assert = require('internal/assert');
7+
assert(false);

test/message/internal_assert.out

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
internal/assert.js:*
2+
throw new ERR_INTERNAL_ASSERTION(message);
3+
^
4+
5+
Error [ERR_INTERNAL_ASSERTION]: This is caused by either a bug in Node.js or incorrect usage of Node.js internals.
6+
Please open an issue with this stack trace at https://github.com/nodejs/node/issues
7+
8+
at assert (internal/assert.js:*:*)
9+
at * (*test*message*internal_assert.js:7:1)
10+
at *
11+
at *
12+
at *
13+
at *
14+
at *
15+
at *

test/message/internal_assert_fail.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
'use strict';
2+
3+
// Flags: --expose-internals
4+
require('../common');
5+
6+
const assert = require('internal/assert');
7+
assert.fail('Unreachable!');

test/message/internal_assert_fail.out

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
internal/assert.js:*
2+
throw new ERR_INTERNAL_ASSERTION(message);
3+
^
4+
5+
Error [ERR_INTERNAL_ASSERTION]: Unreachable!
6+
This is caused by either a bug in Node.js or incorrect usage of Node.js internals.
7+
Please open an issue with this stack trace at https://github.com/nodejs/node/issues
8+
9+
at Function.fail (internal/assert.js:*:*)
10+
at * (*test*message*internal_assert_fail.js:7:8)
11+
at *
12+
at *
13+
at *
14+
at *
15+
at *
16+
at *

test/parallel/test-internal-assert.js

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,13 @@
11
// Flags: --expose-internals
22
'use strict';
33

4+
// This tests that the internal assert module works as expected.
5+
// The failures are tested in test/message.
6+
47
require('../common');
58

6-
const assert = require('assert');
79
const internalAssert = require('internal/assert');
810

911
// Should not throw.
1012
internalAssert(true);
1113
internalAssert(true, 'fhqwhgads');
12-
13-
assert.throws(() => { internalAssert(false); }, assert.AssertionError);
14-
assert.throws(() => { internalAssert(false, 'fhqwhgads'); },
15-
{ code: 'ERR_ASSERTION', message: 'fhqwhgads' });
16-
assert.throws(() => { internalAssert.fail('fhqwhgads'); },
17-
{ code: 'ERR_ASSERTION', message: 'fhqwhgads' });

test/parallel/test-internal-errors.js

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,12 +50,10 @@ errors.E('TEST_ERROR_2', (a, b) => `${a} ${b}`, Error);
5050
}
5151

5252
{
53-
assert.throws(
53+
common.expectsInternalAssertion(
5454
() => new errors.codes.TEST_ERROR_1(),
55-
{
56-
message: 'Code: TEST_ERROR_1; The provided arguments ' +
57-
'length (0) does not match the required ones (1).'
58-
}
55+
'Code: TEST_ERROR_1; The provided arguments ' +
56+
'length (0) does not match the required ones (1).'
5957
);
6058
}
6159

test/parallel/test-tls-basic-validations.js

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,12 +78,9 @@ common.expectsError(
7878
assert.throws(() => tls.createServer({ ticketKeys: Buffer.alloc(0) }),
7979
/TypeError: Ticket keys length must be 48 bytes/);
8080

81-
common.expectsError(
81+
common.expectsInternalAssertion(
8282
() => tls.createSecurePair({}),
83-
{
84-
code: 'ERR_ASSERTION',
85-
message: 'context.context must be a NativeSecureContext'
86-
}
83+
'context.context must be a NativeSecureContext'
8784
);
8885

8986
{

test/sequential/test-fs-watch.js

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -115,14 +115,14 @@ tmpdir.refresh();
115115
// https://github.com/joyent/node/issues/6690
116116
{
117117
let oldhandle;
118-
assert.throws(() => {
119-
const w = fs.watch(__filename, common.mustNotCall());
120-
oldhandle = w._handle;
121-
w._handle = { close: w._handle.close };
122-
w.close();
123-
}, {
124-
message: 'handle must be a FSEvent',
125-
code: 'ERR_ASSERTION'
126-
});
118+
common.expectsInternalAssertion(
119+
() => {
120+
const w = fs.watch(__filename, common.mustNotCall());
121+
oldhandle = w._handle;
122+
w._handle = { close: w._handle.close };
123+
w.close();
124+
},
125+
'handle must be a FSEvent'
126+
);
127127
oldhandle.close(); // clean up
128128
}

0 commit comments

Comments
 (0)