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>
nodejs · Apr 27, 2019 · dd709fc · dd709fc
1 parent 453510c
commit dd709fc
Show file tree

Hide file tree

Showing 12 changed files with 100 additions and 28 deletions.
diff --git 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:`.
 
+<a id="ERR_INTERNAL_ASSERTION"></a>
+### 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.
+
 <a id="ERR_INCOMPATIBLE_OPTION_PAIR"></a>
 ### ERR_INCOMPATIBLE_OPTION_PAIR
 

diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
 }