-
Notifications
You must be signed in to change notification settings - Fork 29.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
assert: validate the block return type #19886
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
This makes sure the returned value when calling `block` is actually of type promise in case `assert.rejects` or `assert.doesNotReject` is called.
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -387,8 +387,13 @@ 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 | ||
without checking the error handler. | ||
|
||
If `block` is a function and it does not return a promise, an | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't this redundant with the previous paragraph? |
||
[`ERR_INVALID_RETURN_VALUE`][] error is going to be rejected. | ||
|
||
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,7 +934,9 @@ 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 | ||
`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 without | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto. |
||
checking the error handler. | ||
|
||
Besides the async nature to await the completion behaves identically to | ||
|
@@ -1138,6 +1145,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 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1173,6 +1173,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 a expected value on execution. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: an expected value |
||
For example when a function is expected to return a promise. | ||
|
||
<a id="ERR_INVALID_SYNC_FORK_INPUT"></a> | ||
### ERR_INVALID_SYNC_FORK_INPUT | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sentence fragment seems a bit unexpected. Should we add a verb to it?