Skip to content

Commit

Permalink
Catch unhandled promise rejections during tests
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Zirak committed Sep 26, 2019
1 parent eed38d7 commit 06f89c2
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 6 deletions.
22 changes: 16 additions & 6 deletions lib/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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.
Expand Down
40 changes: 40 additions & 0 deletions test/integration/fixtures/unhandled.fixture.js
Original file line number Diff line number Diff line change
@@ -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;
});
19 changes: 19 additions & 0 deletions test/integration/uncaught.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});
});

0 comments on commit 06f89c2

Please sign in to comment.