From d47b6786c91908e2601400f4e3e381143049beed Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Fri, 14 Jun 2019 16:14:50 +0200
Subject: [PATCH] assert: wrap validation function errors

This makes sure that validation function used by `assert.throws` and
`assert.rejects` always throw validatin errors instead of rethrowing
the received error.

That should improve the debugging experience for developers since
they have a better context where the error is coming from and they
also get to know what triggered it.

PR-URL: https://github.com/nodejs/node/pull/28263
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 doc/api/assert.md                  |  3 +++
 lib/assert.js                      | 16 +++++++++++++++-
 test/parallel/test-assert-async.js | 15 +++++++++++++++
 test/parallel/test-assert.js       | 17 +++++++++++++++++
 4 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/doc/api/assert.md b/doc/api/assert.md
index db222580c30235..06eb57bad48af0 100644
--- a/doc/api/assert.md
+++ b/doc/api/assert.md
@@ -1222,6 +1222,9 @@ assert.throws(
 
 Custom error validation:
 
+The function must return `true` to indicate all internal validations passed.
+It will otherwise fail with an AssertionError.
+
 ```js
 assert.throws(
   () => {
diff --git a/lib/assert.js b/lib/assert.js
index 1beb33265d6cd3..3d54dcd63fa040 100644
--- a/lib/assert.js
+++ b/lib/assert.js
@@ -653,7 +653,21 @@ function expectedException(actual, expected, message, fn) {
   // Check validation functions return value.
   const res = expected.call({}, actual);
   if (res !== true) {
-    throw actual;
+    if (!message) {
+      generatedMessage = true;
+      const name = expected.name ? `"${expected.name}" ` : '';
+      message = `The ${name}validation function is expected to return "true".` +
+        ` Received ${inspect(res)}`;
+    }
+    const err = new AssertionError({
+      actual,
+      expected,
+      message,
+      operator: fn.name,
+      stackStartFn: fn
+    });
+    err.generatedMessage = generatedMessage;
+    throw err;
   }
 }
 
diff --git a/test/parallel/test-assert-async.js b/test/parallel/test-assert-async.js
index 8aad1d865c097e..45447d456379e1 100644
--- a/test/parallel/test-assert-async.js
+++ b/test/parallel/test-assert-async.js
@@ -66,6 +66,21 @@ const invalidThenableFunc = () => {
       code: 'ERR_INVALID_RETURN_VALUE'
     })
   );
+
+  const err = new Error('foobar');
+  const validate = () => { return 'baz'; };
+  promises.push(assert.rejects(
+    () => assert.rejects(Promise.reject(err), validate),
+    {
+      message: 'The "validate" validation function is expected to ' +
+               "return \"true\". Received 'baz'",
+      code: 'ERR_ASSERTION',
+      actual: err,
+      expected: validate,
+      name: 'AssertionError',
+      operator: 'rejects',
+    }
+  ));
 }
 
 {
diff --git a/test/parallel/test-assert.js b/test/parallel/test-assert.js
index 13f9d6427b01e6..9c59227a78492e 100644
--- a/test/parallel/test-assert.js
+++ b/test/parallel/test-assert.js
@@ -1326,3 +1326,20 @@ assert.throws(
              'Received "[Array]"'
   }
 );
+
+{
+  const err = new TypeError('foo');
+  const validate = (() => () => ({ a: true, b: [ 1, 2, 3 ] }))();
+  assert.throws(
+    () => assert.throws(() => { throw err; }, validate),
+    {
+      message: 'The validation function is expected to ' +
+              `return "true". Received ${inspect(validate())}`,
+      code: 'ERR_ASSERTION',
+      actual: err,
+      expected: validate,
+      name: 'AssertionError',
+      operator: 'throws',
+    }
+  );
+}