Skip to content

Commit

Permalink
assert: stricter ifError
Browse files Browse the repository at this point in the history
This makes `assert.ifError` stricter by only accepting `null` and
`undefined` from now on. Before any truthy value was accepted.

PR-URL: #18247
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
BridgeAR committed Jan 24, 2018
1 parent 8e6e1c9 commit e65a6e8
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 9 deletions.
12 changes: 7 additions & 5 deletions doc/api/assert.md
Original file line number Diff line number Diff line change
Expand Up @@ -474,21 +474,23 @@ changes:
pr-url: https://github.com/nodejs/node/pull/18247
description: Instead of throwing the original error it is now wrapped into
a AssertionError that contains the full stack trace.
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/18247
description: Value may now only be `undefined` or `null`. Before any truthy
input was accepted.
-->
* `value` {any}

Throws `value` if `value` is truthy. This is useful when testing the `error`
argument in callbacks.
Throws `value` if `value` is not `undefined` or `null`. This is useful when
testing the `error` argument in callbacks.

```js
const assert = require('assert').strict;

assert.ifError(null);
// OK
assert.ifError(0);
// OK
assert.ifError(1);
// AssertionError [ERR_ASSERTION]: ifError got unwanted exception: 1
// AssertionError [ERR_ASSERTION]: ifError got unwanted exception: 0
assert.ifError('error');
// AssertionError [ERR_ASSERTION]: ifError got unwanted exception: 'error'
assert.ifError(new Error());
Expand Down
2 changes: 1 addition & 1 deletion lib/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ assert.doesNotThrow = function doesNotThrow(block, error, message) {
};

assert.ifError = function ifError(err) {
if (err) {
if (err !== null && err !== undefined) {
let message = 'ifError got unwanted exception: ';
if (typeof err === 'object' && typeof err.message === 'string') {
if (err.message.length === 0 && err.constructor) {
Expand Down
8 changes: 7 additions & 1 deletion test/parallel/test-assert-if-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,16 @@ assert.throws(
}
);

assert.throws(
() => { assert.ifError(false); },
{
message: 'ifError got unwanted exception: false'
}
);

assert.doesNotThrow(() => { assert.ifError(null); });
assert.doesNotThrow(() => { assert.ifError(); });
assert.doesNotThrow(() => { assert.ifError(undefined); });
assert.doesNotThrow(() => { assert.ifError(false); });

// https://github.com/nodejs/node-v0.x-archive/issues/2893
{
Expand Down
4 changes: 2 additions & 2 deletions test/sequential/test-inspector-port-zero.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ function test(arg, port = '') {
let stderr = '';
proc.stdout.on('data', (data) => stdout += data);
proc.stderr.on('data', (data) => stderr += data);
proc.stdout.on('close', assert.ifError);
proc.stderr.on('close', assert.ifError);
proc.stdout.on('close', (hadErr) => assert(!hadErr));
proc.stderr.on('close', (hadErr) => assert(!hadErr));
proc.stderr.on('data', () => {
if (!stderr.includes('\n')) return;
assert(/Debugger listening on (.+)/.test(stderr));
Expand Down

0 comments on commit e65a6e8

Please sign in to comment.