diff --git a/doc/api/assert.md b/doc/api/assert.md index 339351db8b22b1..80b3019ad8a1ee 100644 --- a/doc/api/assert.md +++ b/doc/api/assert.md @@ -469,6 +469,11 @@ suppressFrame(); ## assert.ifError(value) * `value` {any} @@ -483,11 +488,11 @@ assert.ifError(null); assert.ifError(0); // OK assert.ifError(1); -// Throws 1 +// AssertionError [ERR_ASSERTION]: ifError got unwanted exception: 1 assert.ifError('error'); -// Throws 'error' +// AssertionError [ERR_ASSERTION]: ifError got unwanted exception: 'error' assert.ifError(new Error()); -// Throws Error +// AssertionError [ERR_ASSERTION]: ifError got unwanted exception: Error ``` ## assert.notDeepEqual(actual, expected[, message]) diff --git a/lib/assert.js b/lib/assert.js index 9931ce9c222e6e..6ac3cf38ea99ae 100644 --- a/lib/assert.js +++ b/lib/assert.js @@ -452,7 +452,53 @@ assert.doesNotThrow = function doesNotThrow(block, error, message) { throw actual; }; -assert.ifError = function ifError(err) { if (err) throw err; }; +assert.ifError = function ifError(err) { + if (err) { + let message = 'ifError got unwanted exception: '; + if (typeof err === 'object' && typeof err.message === 'string') { + if (err.message.length === 0 && err.constructor) { + message += err.constructor.name; + } else { + message += err.message; + } + } else { + message += inspect(err); + } + + const newErr = new assert.AssertionError({ + actual: err, + expected: null, + operator: 'ifError', + message, + stackStartFn: ifError + }); + + // Make sure we actually have a stack trace! + const origStack = err.stack; + + if (typeof origStack === 'string') { + // This will remove any duplicated frames from the error frames taken + // from within `ifError` and add the original error frames to the newly + // created ones. + const tmp2 = origStack.split('\n'); + tmp2.shift(); + // Filter all frames existing in err.stack. + let tmp1 = newErr.stack.split('\n'); + for (var i = 0; i < tmp2.length; i++) { + // Find the first occurrence of the frame. + const pos = tmp1.indexOf(tmp2[i]); + if (pos !== -1) { + // Only keep new frames. + tmp1 = tmp1.slice(0, pos); + break; + } + } + newErr.stack = `${tmp1.join('\n')}\n${tmp2.join('\n')}`; + } + + throw newErr; + } +}; // Expose a strict only variant of assert function strict(...args) { diff --git a/test/fixtures/uncaught-exceptions/callbackify2.js b/test/fixtures/uncaught-exceptions/callbackify2.js index 9080da234f3c04..7b4ee4f565596d 100644 --- a/test/fixtures/uncaught-exceptions/callbackify2.js +++ b/test/fixtures/uncaught-exceptions/callbackify2.js @@ -8,13 +8,13 @@ const { callbackify } = require('util'); { const sentinel = new Error(__filename); process.once('uncaughtException', (err) => { - assert.strictEqual(err, sentinel); + assert.notStrictEqual(err, sentinel); // Calling test will use `stdout` to assert value of `err.message` console.log(err.message); }); - async function fn() { - return await Promise.reject(sentinel); + function fn() { + return Promise.reject(sentinel); } const cbFn = callbackify(fn); diff --git a/test/message/if-error-has-good-stack.js b/test/message/if-error-has-good-stack.js new file mode 100644 index 00000000000000..1db25d2fa55a1b --- /dev/null +++ b/test/message/if-error-has-good-stack.js @@ -0,0 +1,22 @@ +'use strict'; + +require('../common'); +const assert = require('assert'); + +let err; +// Create some random error frames. +(function a() { + (function b() { + (function c() { + err = new Error('test error'); + })(); + })(); +})(); + +(function x() { + (function y() { + (function z() { + assert.ifError(err); + })(); + })(); +})(); diff --git a/test/message/if-error-has-good-stack.out b/test/message/if-error-has-good-stack.out new file mode 100644 index 00000000000000..fa72322b446f6a --- /dev/null +++ b/test/message/if-error-has-good-stack.out @@ -0,0 +1,19 @@ +assert.js:* + throw newErr; + ^ + +AssertionError [ERR_ASSERTION]: ifError got unwanted exception: test error + at z (*/if-error-has-good-stack.js:*:* + at y (*/if-error-has-good-stack.js:*:*) + at x (*/if-error-has-good-stack.js:*:*) + at Object. (*/if-error-has-good-stack.js:*:*) + at c (*/if-error-has-good-stack.js:*:*) + at b (*/if-error-has-good-stack.js:*:*) + at a (*/if-error-has-good-stack.js:*:*) + at Object. (*/if-error-has-good-stack.js:*:*) + at Module._compile (module.js:*:*) + at Object.Module._extensions..js (module.js:*:*) + at Module.load (module.js:*:*) + at tryModuleLoad (module.js:*:*) + at Function.Module._load (module.js:*:*) + at Function.Module.runMain (module.js:*:*) diff --git a/test/parallel/test-assert-if-error.js b/test/parallel/test-assert-if-error.js new file mode 100644 index 00000000000000..bcee765f9afc17 --- /dev/null +++ b/test/parallel/test-assert-if-error.js @@ -0,0 +1,82 @@ +'use strict'; + +require('../common'); +const assert = require('assert').strict; +/* eslint-disable no-restricted-properties */ + +// Test that assert.ifError has the correct stack trace of both stacks. + +let err; +// Create some random error frames. +(function a() { + (function b() { + (function c() { + err = new Error('test error'); + })(); + })(); +})(); + +const msg = err.message; +const stack = err.stack; + +(function x() { + (function y() { + (function z() { + let threw = false; + try { + assert.ifError(err); + } catch (e) { + assert.equal(e.message, 'ifError got unwanted exception: test error'); + assert.equal(err.message, msg); + assert.equal(e.actual, err); + assert.equal(e.actual.stack, stack); + assert.equal(e.expected, null); + assert.equal(e.operator, 'ifError'); + threw = true; + } + assert(threw); + })(); + })(); +})(); + +assert.throws( + () => assert.ifError(new TypeError()), + { + message: 'ifError got unwanted exception: TypeError' + } +); + +assert.throws( + () => assert.ifError({ stack: false }), + { + message: 'ifError got unwanted exception: { stack: false }' + } +); + +assert.throws( + () => assert.ifError({ constructor: null, message: '' }), + { + message: 'ifError got unwanted exception: ' + } +); + +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 +{ + let threw = false; + try { + // eslint-disable-next-line no-restricted-syntax + assert.throws(() => { + assert.ifError(null); + }); + } catch (e) { + threw = true; + assert.strictEqual(e.message, 'Missing expected exception.'); + assert(!e.stack.includes('throws'), e.stack); + } + assert(threw); +} diff --git a/test/parallel/test-assert.js b/test/parallel/test-assert.js index e8bf7949322279..70ba887fee54e1 100644 --- a/test/parallel/test-assert.js +++ b/test/parallel/test-assert.js @@ -452,11 +452,6 @@ assert.throws(makeBlock(thrower, TypeError)); 'a.doesNotThrow is not catching type matching errors'); } -assert.throws(() => { assert.ifError(new Error('test error')); }, - /^Error: test error$/); -assert.doesNotThrow(() => { assert.ifError(null); }); -assert.doesNotThrow(() => { assert.ifError(); }); - common.expectsError( () => assert.doesNotThrow(makeBlock(thrower, Error), 'user message'), { @@ -602,22 +597,6 @@ testAssertionMessage({ a: undefined, b: null }, '{ a: undefined, b: null }'); testAssertionMessage({ a: NaN, b: Infinity, c: -Infinity }, '{ a: NaN, b: Infinity, c: -Infinity }'); -// https://github.com/nodejs/node-v0.x-archive/issues/2893 -{ - let threw = false; - try { - // eslint-disable-next-line no-restricted-syntax - assert.throws(() => { - assert.ifError(null); - }); - } catch (e) { - threw = true; - assert.strictEqual(e.message, 'Missing expected exception.'); - assert.ok(!e.stack.includes('throws'), e.stack); - } - assert.ok(threw); -} - // https://github.com/nodejs/node-v0.x-archive/issues/5292 try { assert.strictEqual(1, 2); diff --git a/test/parallel/test-domain-implicit-fs.js b/test/parallel/test-domain-implicit-fs.js index 6e4127763ce51c..f3695989502a77 100644 --- a/test/parallel/test-domain-implicit-fs.js +++ b/test/parallel/test-domain-implicit-fs.js @@ -34,9 +34,9 @@ d.on('error', common.mustCall(function(er) { assert.strictEqual(er.domain, d); assert.strictEqual(er.domainThrown, true); assert.ok(!er.domainEmitter); - assert.strictEqual(er.code, 'ENOENT'); - assert.ok(/\bthis file does not exist\b/i.test(er.path)); - assert.strictEqual(typeof er.errno, 'number'); + assert.strictEqual(er.actual.code, 'ENOENT'); + assert.ok(/\bthis file does not exist\b/i.test(er.actual.path)); + assert.strictEqual(typeof er.actual.errno, 'number'); })); diff --git a/test/parallel/test-util-callbackify.js b/test/parallel/test-util-callbackify.js index bc2154940cb405..d8b2de314c9c49 100644 --- a/test/parallel/test-util-callbackify.js +++ b/test/parallel/test-util-callbackify.js @@ -220,7 +220,9 @@ const values = [ [fixture], common.mustCall((err, stdout, stderr) => { assert.ifError(err); - assert.strictEqual(stdout.trim(), fixture); + assert.strictEqual( + stdout.trim(), + `ifError got unwanted exception: ${fixture}`); assert.strictEqual(stderr, ''); }) );