Permalink
Browse files

assert: validate the block return type

This makes sure the returned value when calling `block` is actually
of type promise in case `assert.rejects` or `assert.doesNotReject`
is called.

PR-URL: #19886
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
  • Loading branch information...
BridgeAR committed Apr 8, 2018
1 parent ad1d105 commit 7007eee6a272398054c65f2f32bdf3467a29ee9f
Showing with 56 additions and 18 deletions.
  1. +9 −4 doc/api/assert.md
  2. +6 −0 doc/api/errors.md
  3. +7 −1 lib/assert.js
  4. +10 −0 lib/internal/errors.js
  5. +24 −13 test/parallel/test-assert-async.js
@@ -387,8 +387,10 @@ function and awaits the returned promise to complete. It will then check that
the promise is not rejected.
If `block` is a function and it throws an error synchronously,
`assert.doesNotReject()` will return a rejected Promise with that error without
checking the error handler.
`assert.doesNotReject()` will return a rejected Promise with that error. If the
function does not return a promise, `assert.doesNotReject()` will return a
rejected Promise with an [`ERR_INVALID_RETURN_VALUE`][] error. In both cases the
error handler is skipped.
Please note: Using `assert.doesNotReject()` is actually not useful because there
is little benefit by catching a rejection and then rejecting it again. Instead,
@@ -929,8 +931,10 @@ function and awaits the returned promise to complete. It will then check that
the promise is rejected.
If `block` is a function and it throws an error synchronously,
`assert.rejects()` will return a rejected Promise with that error without
checking the error handler.
`assert.rejects()` will return a rejected Promise with that error. If the
function does not return a promise, `assert.rejects()` will return a rejected
Promise with an [`ERR_INVALID_RETURN_VALUE`][] error. In both cases the error
handler is skipped.
Besides the async nature to await the completion behaves identically to
[`assert.throws()`][].
@@ -1138,6 +1142,7 @@ assert.throws(throwingFirst, /Second$/);
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.
[`ERR_INVALID_RETURN_VALUE`]: errors.html#errors_err_invalid_return_value
[`Class`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Classes
[`Error.captureStackTrace`]: errors.html#errors_error_capturestacktrace_targetobject_constructoropt
[`Error`]: errors.html#errors_class_error
@@ -1186,6 +1186,12 @@ An invalid `options.protocol` was passed.
Both `breakEvalOnSigint` and `eval` options were set in the REPL config, which
is not supported.
<a id="ERR_INVALID_RETURN_VALUE"></a>
### ERR_INVALID_RETURN_VALUE
Thrown in case a function option does not return an expected value on execution.
For example when a function is expected to return a promise.
<a id="ERR_INVALID_SYNC_FORK_INPUT"></a>
### ERR_INVALID_SYNC_FORK_INPUT
@@ -30,7 +30,8 @@ const {
errorCache,
codes: {
ERR_AMBIGUOUS_ARGUMENT,
ERR_INVALID_ARG_TYPE
ERR_INVALID_ARG_TYPE,
ERR_INVALID_RETURN_VALUE
}
} = require('internal/errors');
const { openSync, closeSync, readSync } = require('fs');
@@ -455,6 +456,11 @@ async function waitForActual(block) {
if (typeof block === 'function') {
// Return a rejected promise if `block` throws synchronously.
resultPromise = block();
// Fail in case no promise is returned.
if (!checkIsPromise(resultPromise)) {
throw new ERR_INVALID_RETURN_VALUE('instance of Promise',
'block', resultPromise);
}
} else if (checkIsPromise(block)) {
resultPromise = block;
} else {
@@ -904,6 +904,16 @@ E('ERR_INVALID_PROTOCOL',
TypeError);
E('ERR_INVALID_REPL_EVAL_CONFIG',
'Cannot specify both "breakEvalOnSigint" and "eval" for REPL', TypeError);
E('ERR_INVALID_RETURN_VALUE', (input, name, value) => {
let type;
if (value && value.constructor && value.constructor.name) {
type = `instance of ${value.constructor.name}`;
} else {
type = `type ${typeof value}`;
}
return `Expected ${input} to be returned from the "${name}"` +
` function but got ${type}.`;
}, TypeError);
E('ERR_INVALID_SYNC_FORK_INPUT',
'Asynchronous forks do not support Buffer, Uint8Array or string input: %s',
TypeError);
@@ -29,20 +29,25 @@ const promises = [];
`${err.name} is not instance of AssertionError`);
assert.strictEqual(err.code, 'ERR_ASSERTION');
assert.strictEqual(err.message,
'Missing expected rejection (handler).');
'Missing expected rejection (mustNotCall).');
assert.strictEqual(err.operator, 'rejects');
assert.ok(!err.stack.includes('at Function.rejects'));
return true;
};
let promise = assert.rejects(async () => {}, handler);
promises.push(assert.rejects(promise, handler));
let promise = assert.rejects(async () => {}, common.mustNotCall());
promises.push(assert.rejects(promise, common.mustCall(handler)));
promise = assert.rejects(() => {}, handler);
promises.push(assert.rejects(promise, handler));
promise = assert.rejects(() => {}, common.mustNotCall());
promises.push(assert.rejects(promise, {
name: 'TypeError [ERR_INVALID_RETURN_VALUE]',
code: 'ERR_INVALID_RETURN_VALUE',
message: 'Expected instance of Promise to be returned ' +
'from the "block" function but got type undefined.'
}));
promise = assert.rejects(Promise.resolve(), handler);
promises.push(assert.rejects(promise, handler));
promise = assert.rejects(Promise.resolve(), common.mustNotCall());
promises.push(assert.rejects(promise, common.mustCall(handler)));
}
{
@@ -67,7 +72,13 @@ promises.push(assert.rejects(
// Check `assert.doesNotReject`.
{
// `assert.doesNotReject` accepts a function or a promise as first argument.
promises.push(assert.doesNotReject(() => {}));
const promise = assert.doesNotReject(() => new Map(), common.mustNotCall());
promises.push(assert.rejects(promise, {
message: 'Expected instance of Promise to be returned ' +
'from the "block" function but got instance of Map.',
code: 'ERR_INVALID_RETURN_VALUE',
name: 'TypeError [ERR_INVALID_RETURN_VALUE]'
}));
promises.push(assert.doesNotReject(async () => {}));
promises.push(assert.doesNotReject(Promise.resolve()));
}
@@ -93,14 +104,14 @@ promises.push(assert.rejects(
const rejectingFn = async () => assert.fail();
let promise = assert.doesNotReject(rejectingFn, handler1);
promises.push(assert.rejects(promise, handler2));
let promise = assert.doesNotReject(rejectingFn, common.mustCall(handler1));
promises.push(assert.rejects(promise, common.mustCall(handler2)));
promise = assert.doesNotReject(rejectingFn(), handler1);
promises.push(assert.rejects(promise, handler2));
promise = assert.doesNotReject(rejectingFn(), common.mustCall(handler1));
promises.push(assert.rejects(promise, common.mustCall(handler2)));
promise = assert.doesNotReject(() => assert.fail(), common.mustNotCall());
promises.push(assert.rejects(promise, handler1));
promises.push(assert.rejects(promise, common.mustCall(handler1)));
}
promises.push(assert.rejects(

0 comments on commit 7007eee

Please sign in to comment.