Skip to content

Commit

Permalink
assert: throw without args in ok
Browse files Browse the repository at this point in the history
`assert.ok()` should always receive a value. Otherwise there
might be a bug or it was intended to use `assert.fail()`.

PR-URL: #17581
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ron Korving <ron@ronkorving.nl>
  • Loading branch information
BridgeAR committed Jan 16, 2018
1 parent f76ef50 commit d07c6f9
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 0 deletions.
5 changes: 5 additions & 0 deletions doc/api/assert.md
Original file line number Diff line number Diff line change
Expand Up @@ -645,6 +645,11 @@ parameter is an instance of an [`Error`][] then it will be thrown instead of the
## assert.ok(value[, message])
<!-- YAML
added: v0.1.21
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/17581
description: assert.ok() will throw a `ERR_MISSING_ARGS` error.
Use assert.fail() instead.
-->
* `value` {any}
* `message` {any}
Expand Down
3 changes: 3 additions & 0 deletions lib/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,9 @@ function getBuffer(fd, assertLine) {
function innerOk(args, fn) {
var [value, message] = args;

if (args.length === 0)
throw new errors.TypeError('ERR_MISSING_ARGS', 'value');

if (!value) {
if (message == null) {
// Use the call as error message if possible.
Expand Down
14 changes: 14 additions & 0 deletions test/parallel/test-assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -754,6 +754,20 @@ common.expectsError(
assert.equal(Object.keys(assert).length, Object.keys(a).length);
/* eslint-enable no-restricted-properties */
assert(7);
common.expectsError(
() => assert(),
{
code: 'ERR_MISSING_ARGS',
type: TypeError
}
);
common.expectsError(
() => a(),
{
code: 'ERR_MISSING_ARGS',
type: TypeError
}
);

// Test setting the limit to zero and that assert.strict works properly.
const tmpLimit = Error.stackTraceLimit;
Expand Down

0 comments on commit d07c6f9

Please sign in to comment.