From 147aeedc8d6d58313778d3bb030aad7858b84d48 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Sat, 9 Dec 2017 21:47:49 -0200
Subject: assert: improve assert.throws

Throw a TypeError in case a error message is provided in the second
argument and a third argument is present as well.
This is clearly a mistake and should not be done.

Backport-PR-URL: https://github.com/nodejs/node/pull/23223
PR-URL: https://github.com/nodejs/node/pull/17585
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
---
 doc/api/assert.md | 37 +++++++++++++++++++++++++++++++------
 1 file changed, 31 insertions(+), 6 deletions(-)

(limited to 'doc')

diff --git a/doc/api/assert.md b/doc/api/assert.md
index d337a09b328..b1c7168e94c 100644
--- a/doc/api/assert.md
+++ b/doc/api/assert.md
@@ -691,17 +691,42 @@ assert.throws(
 
 Note that `error` can not be a string. If a string is provided as the second
 argument, then `error` is assumed to be omitted and the string will be used for
-`message` instead. This can lead to easy-to-miss mistakes:
+`message` instead. This can lead to easy-to-miss mistakes. Please read the
+example below carefully if using a string as the second argument gets
+considered:
 
 <!-- eslint-disable no-restricted-syntax -->
 ```js
-// THIS IS A MISTAKE! DO NOT DO THIS!
-assert.throws(myFunction, 'missing foo', 'did not throw with expected message');
-
-// Do this instead.
-assert.throws(myFunction, /missing foo/, 'did not throw with expected message');
+function throwingFirst() {
+  throw new Error('First');
+}
+function throwingSecond() {
+  throw new Error('Second');
+}
+function notThrowing() {}
+
+// The second argument is a string and the input function threw an Error.
+// In that case both cases do not throw as neither is going to try to
+// match for the error message thrown by the input function!
+assert.throws(throwingFirst, 'Second');
+assert.throws(throwingSecond, 'Second');
+
+// The string is only used (as message) in case the function does not throw:
+assert.throws(notThrowing, 'Second');
+// AssertionError [ERR_ASSERTION]: Missing expected exception: Second
+
+// If it was intended to match for the error message do this instead:
+assert.throws(throwingSecond, /Second$/);
+// Does not throw because the error messages match.
+assert.throws(throwingFirst, /Second$/);
+// Throws a error:
+// Error: First
+//     at throwingFirst (repl:2:9)
 ```
 
+Due to the confusing notation, it is recommended not to use a string as the
+second argument. This might lead to difficult-to-spot errors.
+
 ## Caveats
 
 For the following cases, consider using ES2015 [`Object.is()`][],
-- 
cgit v1.2.3