From 06f89c2d15fa283d983e64f9775830795776d4bc Mon Sep 17 00:00:00 2001 From: Zirak Date: Thu, 26 Sep 2019 12:45:28 +0000 Subject: [PATCH] Catch unhandled promise rejections during tests Listen to the unhandledRejection event when running tests, catching promises who got rejected without being cared for, and failing the test in which said promises were left unhandled. This is only part of handling unhandled rejections, some things not yet included are: - Handling unhandled rejections outside spec tests (e.g. testhooks) - Making it work in a browser - Amend error messages which reference unhandled rejections - Testing the new signature of the `Runner.prototype.uncaught` method If this commit looks nice enough, a future commit will amend these shortcomings. --- lib/runner.js | 22 +++++++--- .../integration/fixtures/unhandled.fixture.js | 40 +++++++++++++++++++ test/integration/uncaught.spec.js | 19 +++++++++ 3 files changed, 75 insertions(+), 6 deletions(-) create mode 100644 test/integration/fixtures/unhandled.fixture.js diff --git a/lib/runner.js b/lib/runner.js index d8d8dd6fb8..9af6d8d99b 100644 --- a/lib/runner.js +++ b/lib/runner.js @@ -783,16 +783,19 @@ Runner.prototype.runSuite = function(suite, fn) { * Handle uncaught exceptions. * * @param {Error} err + * @param {string} unwhat It this an 'uncaught' exception, or 'unhandled' rejection * @private */ -Runner.prototype.uncaught = function(err) { +Runner.prototype.uncaught = function(err, unwhat) { + unwhat = unwhat || 'uncaught'; + if (err instanceof Pending) { return; } if (err) { - debug('uncaught exception %O', err); + debug('%s exception %O', unwhat, err); } else { - debug('uncaught undefined/falsy exception'); + debug('%s undefined/falsy exception', unwhat); err = createInvalidExceptionError( 'Caught falsy/undefined exception which would otherwise be uncaught. No stack trace found; try a debugger', err @@ -802,12 +805,13 @@ Runner.prototype.uncaught = function(err) { if (!isError(err)) { err = thrown2Error(err); } - err.uncaught = true; + err[unwhat] = true; var runnable = this.currentRunnable; if (!runnable) { - runnable = new Runnable('Uncaught error outside test suite'); + var capitalisedUnwhat = unwhat === 'uncaught' ? 'Uncaught' : 'Unhandled'; + runnable = new Runnable(capitalisedUnwhat + ' error outside test suite'); runnable.parent = this.suite; if (this.started) { @@ -879,7 +883,10 @@ Runner.prototype.run = function(fn) { fn = fn || function() {}; function uncaught(err) { - self.uncaught(err); + self.uncaught(err, 'uncaught'); + } + function unhandled(err) { + self.uncaught(err, 'unhandled'); } function start() { @@ -910,11 +917,14 @@ Runner.prototype.run = function(fn) { this.on(constants.EVENT_RUN_END, function() { debug(constants.EVENT_RUN_END); process.removeListener('uncaughtException', uncaught); + process.removeListener('unhandledRejection', unhandled); fn(self.failures); }); // uncaught exception process.on('uncaughtException', uncaught); + // unhandled rejection + process.on('unhandledRejection', unhandled); if (this._delay) { // for reporters, I guess. diff --git a/test/integration/fixtures/unhandled.fixture.js b/test/integration/fixtures/unhandled.fixture.js new file mode 100644 index 0000000000..eb5f9a9748 --- /dev/null +++ b/test/integration/fixtures/unhandled.fixture.js @@ -0,0 +1,40 @@ +'use strict'; + +function rejectWith(err) { + return new Promise(function (resolve, reject) { + reject(err); + }); +} + +// We wish to test unhandled rejections, which happen after a tick at the +// earliest, so we need a way some way to signal mocha our tests are async. One +// way is to accept the `done` parameter without ever using it. Another is +// returning a promise which never resolves. Since this tests promise handling, +// why don't we go all the way with the promises? +var unresolvedPromise = new Promise(function () {}); + +it('fails when faced with an unhandled rejection', function () { + rejectWith(new Error('rejection')); + + return unresolvedPromise; +}); + +it('fails exactly once when a global promise is rejected first', function () { + setTimeout(function () { + rejectWith(new Error('global error')); + }, 0); + + return unresolvedPromise; +}); + +it('fails exactly once when a global promise is rejected second', function () { + setTimeout(function () { + rejectWith(new Error('test error')); + }, 0); + + setTimeout(function () { + rejectWith(new Error('global error')); + }, 0); + + return unresolvedPromise; +}); diff --git a/test/integration/uncaught.spec.js b/test/integration/uncaught.spec.js index 42342923f0..51314558c1 100644 --- a/test/integration/uncaught.spec.js +++ b/test/integration/uncaught.spec.js @@ -71,3 +71,22 @@ describe('uncaught exceptions', function() { }); }); }); + +describe('unhandled rejections', function() { + it('handles unhandled rejections from async specs', function(done) { + run('unhandled.fixture.js', args, function(err, res) { + if (err) { + done(err); + return; + } + + assert.strictEqual(res.stats.pending, 0); + assert.strictEqual(res.stats.passes, 0); + assert.strictEqual(res.stats.failures, 3); + + assert.strictEqual(res.code, 3); + + done(); + }); + }); +});