From 4774af16fc879ac5a5d598a73227b9df09c75edf Mon Sep 17 00:00:00 2001
From: Teddy Katz <teddy.katz@gmail.com>
Date: Wed, 28 Mar 2018 01:16:20 -0400
Subject: [PATCH 1/2] test: ensure failed assertions cause build to fail

This updates the test in `test/parallel/test-assert-async.js` to add an
assertion that the Promises used in the test end up fulfilled.
Previously, if an assertion failure occurred, the Promises would have
rejected and a warning would have been logged, but the test would still
have exit code 0.
---
 test/parallel/test-assert-async.js | 96 ++++++++++++++----------------
 1 file changed, 45 insertions(+), 51 deletions(-)

diff --git a/test/parallel/test-assert-async.js b/test/parallel/test-assert-async.js
index 9fcde68bd5ae15..ab03a53cd302c7 100644
--- a/test/parallel/test-assert-async.js
+++ b/test/parallel/test-assert-async.js
@@ -9,58 +9,52 @@ const wait = promisify(setTimeout);
 // Test assert.rejects() and assert.doesNotReject() by checking their
 // expected output and by verifying that they do not work sync
 
-assert.rejects(
-  () => assert.fail(),
-  common.expectsError({
-    code: 'ERR_ASSERTION',
-    type: assert.AssertionError,
-    message: 'Failed'
-  })
-);
+common.crashOnUnhandledRejection();
 
-assert.doesNotReject(() => {});
+(async () => {
+  await assert.rejects(
+    () => assert.fail(),
+    common.expectsError({
+      code: 'ERR_ASSERTION',
+      type: assert.AssertionError,
+      message: 'Failed'
+    })
+  );
 
-{
-  const promise = assert.rejects(async () => {
-    await wait(1);
-    assert.fail();
-  }, common.expectsError({
-    code: 'ERR_ASSERTION',
-    type: assert.AssertionError,
-    message: 'Failed'
-  }));
-  assert.doesNotReject(() => promise);
-}
+  await assert.doesNotReject(() => {});
 
-{
-  const promise = assert.doesNotReject(async () => {
-    await wait(1);
-    throw new Error();
-  });
-  assert.rejects(() => promise,
-                 (err) => {
-                   assert(err instanceof assert.AssertionError,
-                          `${err.name} is not instance of AssertionError`);
-                   assert.strictEqual(err.code, 'ERR_ASSERTION');
-                   assert(/^Got unwanted rejection\.\n$/.test(err.message));
-                   assert.strictEqual(err.operator, 'doesNotReject');
-                   assert.ok(!err.stack.includes('at Function.doesNotReject'));
-                   return true;
-                 }
-  );
-}
+  {
+    const promise = assert.doesNotReject(async () => {
+      await wait(1);
+      throw new Error();
+    });
+    await assert.rejects(
+      () => promise,
+      (err) => {
+        assert(err instanceof assert.AssertionError,
+               `${err.name} is not instance of AssertionError`);
+        assert.strictEqual(err.code, 'ERR_ASSERTION');
+        assert(/^Got unwanted rejection\.\n$/.test(err.message));
+        assert.strictEqual(err.operator, 'doesNotReject');
+        assert.ok(!err.stack.includes('at Function.doesNotReject'));
+        return true;
+      }
+    );
+  }
 
-{
-  const promise = assert.rejects(() => {});
-  assert.rejects(() => promise,
-                 (err) => {
-                   assert(err instanceof assert.AssertionError,
-                          `${err.name} is not instance of AssertionError`);
-                   assert.strictEqual(err.code, 'ERR_ASSERTION');
-                   assert(/^Missing expected rejection\.$/.test(err.message));
-                   assert.strictEqual(err.operator, 'rejects');
-                   assert.ok(!err.stack.includes('at Function.rejects'));
-                   return true;
-                 }
-  );
-}
+  {
+    const promise = assert.rejects(() => {});
+    await assert.rejects(
+      () => promise,
+      (err) => {
+        assert(err instanceof assert.AssertionError,
+               `${err.name} is not instance of AssertionError`);
+        assert.strictEqual(err.code, 'ERR_ASSERTION');
+        assert(/^Missing expected rejection\.$/.test(err.message));
+        assert.strictEqual(err.operator, 'rejects');
+        assert.ok(!err.stack.includes('at Function.rejects'));
+        return true;
+      }
+    );
+  }
+})().then(common.mustCall());

From 75d2d1e289c49c82e3eafc9469a8396e853bd91c Mon Sep 17 00:00:00 2001
From: Teddy Katz <teddy.katz@gmail.com>
Date: Wed, 28 Mar 2018 01:21:31 -0400
Subject: [PATCH 2/2] assert: ensure .rejects() disallows sync throws

This updates `assert.rejects()` to disallow any errors that are thrown
synchronously from the given function. Previously, throwing an error
would cause the same behavior as returning a rejected Promise.

Fixes: https://github.com/nodejs/node/issues/19646
---
 lib/assert.js                      |  5 ++++-
 test/parallel/test-assert-async.js | 15 ++++++++++++++-
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/lib/assert.js b/lib/assert.js
index 2fc3cf33e80a18..9c900fcaf32055 100644
--- a/lib/assert.js
+++ b/lib/assert.js
@@ -451,8 +451,11 @@ async function waitForActual(block) {
   if (typeof block !== 'function') {
     throw new ERR_INVALID_ARG_TYPE('block', 'Function', block);
   }
+
+  // Return a rejected promise if `block` throws synchronously.
+  const resultPromise = block();
   try {
-    await block();
+    await resultPromise;
   } catch (e) {
     return e;
   }
diff --git a/test/parallel/test-assert-async.js b/test/parallel/test-assert-async.js
index ab03a53cd302c7..b6b744c9b1e3ae 100644
--- a/test/parallel/test-assert-async.js
+++ b/test/parallel/test-assert-async.js
@@ -13,7 +13,7 @@ common.crashOnUnhandledRejection();
 
 (async () => {
   await assert.rejects(
-    () => assert.fail(),
+    async () => assert.fail(),
     common.expectsError({
       code: 'ERR_ASSERTION',
       type: assert.AssertionError,
@@ -57,4 +57,17 @@ common.crashOnUnhandledRejection();
       }
     );
   }
+
+  {
+    const THROWN_ERROR = new Error();
+
+    await assert.rejects(() => {
+      throw THROWN_ERROR;
+    }).then(common.mustNotCall())
+      .catch(
+        common.mustCall((err) => {
+          assert.strictEqual(err, THROWN_ERROR);
+        })
+      );
+  }
 })().then(common.mustCall());