From e65a6e81ef5e8c0afae4ffec852b662732114adb Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Tue, 23 Jan 2018 14:07:18 +0100 Subject: [PATCH] assert: stricter ifError This makes `assert.ifError` stricter by only accepting `null` and `undefined` from now on. Before any truthy value was accepted. PR-URL: https://github.com/nodejs/node/pull/18247 Reviewed-By: Matteo Collina Reviewed-By: James M Snell --- doc/api/assert.md | 12 +++++++----- lib/assert.js | 2 +- test/parallel/test-assert-if-error.js | 8 +++++++- test/sequential/test-inspector-port-zero.js | 4 ++-- 4 files changed, 17 insertions(+), 9 deletions(-) diff --git a/doc/api/assert.md b/doc/api/assert.md index 80b3019ad8a1ee..04f312aa6634fa 100644 --- a/doc/api/assert.md +++ b/doc/api/assert.md @@ -474,11 +474,15 @@ 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; @@ -486,9 +490,7 @@ 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()); diff --git a/lib/assert.js b/lib/assert.js index 20f9d0e47723f6..e25247dfed2579 100644 --- a/lib/assert.js +++ b/lib/assert.js @@ -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) { diff --git a/test/parallel/test-assert-if-error.js b/test/parallel/test-assert-if-error.js index bcee765f9afc17..3b070aac08f993 100644 --- a/test/parallel/test-assert-if-error.js +++ b/test/parallel/test-assert-if-error.js @@ -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 { diff --git a/test/sequential/test-inspector-port-zero.js b/test/sequential/test-inspector-port-zero.js index b7eb13ba13a68a..59027b5e30769d 100644 --- a/test/sequential/test-inspector-port-zero.js +++ b/test/sequential/test-inspector-port-zero.js @@ -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));