Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

delegate to Node on non-Mocha unhandled rejections #4489

Merged
merged 2 commits into from Nov 2, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 13 additions & 0 deletions lib/errors.js
Expand Up @@ -129,6 +129,8 @@ var constants = {
INVALID_PLUGIN_DEFINITION: 'ERR_MOCHA_INVALID_PLUGIN_DEFINITION'
};

const MOCHA_ERRORS = new Set(Object.values(constants));

/**
* Creates an error object to be thrown when no files to be tested could be found using specified pattern.
*
Expand Down Expand Up @@ -419,6 +421,16 @@ function createInvalidPluginImplementationError(
return err;
}

/**
* Returns `true` if an error came out of Mocha.
* _Can suffer from false negatives, but not false positives._
* @public
* @param {*} err - Error, or anything
* @returns {boolean}
*/
const isMochaError = err =>
Boolean(err && typeof err === 'object' && MOCHA_ERRORS.has(err.code));

module.exports = {
constants,
createFatalError,
Expand All @@ -439,5 +451,6 @@ module.exports = {
createNoFilesMatchPatternError,
createUnsupportedError,
deprecate,
isMochaError,
warn
};
44 changes: 34 additions & 10 deletions lib/runner.js
Expand Up @@ -24,10 +24,13 @@ var sQuote = utils.sQuote;
var stackFilter = utils.stackTraceFilter();
var stringify = utils.stringify;

var errors = require('./errors');
var createInvalidExceptionError = errors.createInvalidExceptionError;
var createUnsupportedError = errors.createUnsupportedError;
var createFatalError = errors.createFatalError;
const {
createInvalidExceptionError,
createUnsupportedError,
createFatalError,
isMochaError,
constants: errorConstants
} = require('./errors');

/**
* Non-enumerable globals.
Expand Down Expand Up @@ -179,6 +182,29 @@ class Runner extends EventEmitter {
this.globals(this.globalProps());

this.uncaught = this._uncaught.bind(this);
this.unhandled = (reason, promise) => {
if (isMochaError(reason)) {
debug(
'trapped unhandled rejection coming out of Mocha; forwarding to uncaught handler:',
reason
);
this.uncaught(reason);
} else {
debug(
'trapped unhandled rejection from (probably) user code; re-emitting on process'
);
this._removeEventListener(
process,
'unhandledRejection',
this.unhandled
);
try {
process.emit('unhandledRejection', reason, promise);
} finally {
this._addEventListener(process, 'unhandledRejection', this.unhandled);
}
}
};
}
}

Expand Down Expand Up @@ -414,7 +440,7 @@ Runner.prototype.fail = function(test, err, force) {
return;
}
if (this.state === constants.STATE_STOPPED) {
if (err.code === errors.constants.MULTIPLE_DONE) {
if (err.code === errorConstants.MULTIPLE_DONE) {
throw err;
}
throw createFatalError(
Expand Down Expand Up @@ -1025,9 +1051,7 @@ Runner.prototype.run = function(fn, opts = {}) {
this.emit(constants.EVENT_RUN_BEGIN);
debug('run(): emitted %s', constants.EVENT_RUN_BEGIN);

this.runSuite(rootSuite, async () => {
end();
});
this.runSuite(rootSuite, end);
};

const prepare = () => {
Expand Down Expand Up @@ -1061,9 +1085,9 @@ Runner.prototype.run = function(fn, opts = {}) {
});

this._removeEventListener(process, 'uncaughtException', this.uncaught);
this._removeEventListener(process, 'unhandledRejection', this.uncaught);
this._removeEventListener(process, 'unhandledRejection', this.unhandled);
this._addEventListener(process, 'uncaughtException', this.uncaught);
this._addEventListener(process, 'unhandledRejection', this.uncaught);
this._addEventListener(process, 'unhandledRejection', this.unhandled);

if (this._delay) {
// for reporters, I guess.
Expand Down
7 changes: 7 additions & 0 deletions test/integration/fixtures/uncaught/unhandled.fixture.js
@@ -0,0 +1,7 @@
it('should emit an unhandled rejection', async function() {
setTimeout(() => {
Promise.resolve().then(() => {
throw new Error('yikes');
});
});
});
69 changes: 55 additions & 14 deletions test/integration/uncaught.spec.js
@@ -1,14 +1,17 @@
'use strict';

var helpers = require('./helpers');
var run = helpers.runMochaJSON;
var runMocha = helpers.runMocha;
var invokeNode = helpers.invokeNode;
const {
runMocha,
runMochaJSON: run,
invokeMochaAsync,
invokeNode,
resolveFixturePath
} = require('./helpers');
var args = [];

describe('uncaught exceptions', function() {
it('handles uncaught exceptions from hooks', function(done) {
run('uncaught/hook.fixture.js', args, function(err, res) {
run('uncaught/hook', args, function(err, res) {
if (err) {
return done(err);
}
Expand All @@ -24,7 +27,7 @@ describe('uncaught exceptions', function() {
});

it('handles uncaught exceptions from async specs', function(done) {
run('uncaught/double.fixture.js', args, function(err, res) {
run('uncaught/double', args, function(err, res) {
if (err) {
return done(err);
}
Expand All @@ -44,7 +47,7 @@ describe('uncaught exceptions', function() {
});

it('handles uncaught exceptions from which Mocha cannot recover', function(done) {
run('uncaught/fatal.fixture.js', args, function(err, res) {
run('uncaught/fatal', args, function(err, res) {
if (err) {
return done(err);
}
Expand All @@ -61,7 +64,7 @@ describe('uncaught exceptions', function() {
});

it('handles uncaught exceptions within pending tests', function(done) {
run('uncaught/pending.fixture.js', args, function(err, res) {
run('uncaught/pending', args, function(err, res) {
if (err) {
return done(err);
}
Expand All @@ -84,7 +87,7 @@ describe('uncaught exceptions', function() {
});

it('handles uncaught exceptions within open tests', function(done) {
run('uncaught/recover.fixture.js', args, function(err, res) {
run('uncaught/recover', args, function(err, res) {
if (err) {
return done(err);
}
Expand All @@ -111,8 +114,7 @@ describe('uncaught exceptions', function() {
});

it('removes uncaught exceptions handlers correctly', function(done) {
var path = require.resolve('./fixtures/uncaught/listeners.fixture.js');
invokeNode([path], function(err, res) {
invokeNode([resolveFixturePath('uncaught/listeners')], function(err, res) {
if (err) {
return done(err);
}
Expand All @@ -124,7 +126,7 @@ describe('uncaught exceptions', function() {

it("handles uncaught exceptions after runner's end", function(done) {
runMocha(
'uncaught/after-runner.fixture.js',
'uncaught/after-runner',
args,
function(err, res) {
if (err) {
Expand All @@ -145,7 +147,7 @@ describe('uncaught exceptions', function() {
});

it('issue-1327: should run the first test and then bail', function(done) {
run('uncaught/issue-1327.fixture.js', args, function(err, res) {
run('uncaught/issue-1327', args, function(err, res) {
if (err) {
return done(err);
}
Expand All @@ -159,7 +161,7 @@ describe('uncaught exceptions', function() {
});

it('issue-1417: uncaught exceptions from async specs', function(done) {
run('uncaught/issue-1417.fixture.js', args, function(err, res) {
run('uncaught/issue-1417', args, function(err, res) {
if (err) {
return done(err);
}
Expand All @@ -174,4 +176,43 @@ describe('uncaught exceptions', function() {
done();
});
});

describe('issue-4481: behavior of non-Mocha-originating unhandled rejections', function() {
describe('when Node is in "warn" mode', function() {
it('should warn', async function() {
const [, promise] = invokeMochaAsync(
[
resolveFixturePath('uncaught/unhandled'),
'--unhandled-rejections=warn'
],
{stdio: 'pipe'}
);

return expect(
promise,
'when fulfilled',
'to have passed with output',
/UnhandledPromiseRejectionWarning: Error: yikes/
);
});
});

describe('when Node is in "strict" mode', function() {
it('should fail with an uncaught exception', async function() {
const [, promise] = invokeMochaAsync(
[
resolveFixturePath('uncaught/unhandled'),
'--unhandled-rejections=strict'
],
{stdio: 'pipe'}
);
return expect(
promise,
'when fulfilled',
'to have failed with output',
/Error: yikes/
);
});
});
});
});
26 changes: 26 additions & 0 deletions test/unit/errors.spec.js
Expand Up @@ -2,6 +2,7 @@

var errors = require('../../lib/errors');
const sinon = require('sinon');
const {createNoFilesMatchPatternError} = require('../../lib/errors');

describe('Errors', function() {
afterEach(function() {
Expand Down Expand Up @@ -143,4 +144,29 @@ describe('Errors', function() {
expect(process.emitWarning, 'was not called');
});
});

describe('isMochaError()', function() {
describe('when provided an Error object having a known Mocha error code', function() {
it('should return true', function() {
expect(
errors.isMochaError(createNoFilesMatchPatternError('derp')),
'to be true'
);
});
});

describe('when provided an Error object with a non-Mocha error code', function() {
it('should return false', function() {
const err = new Error();
err.code = 'ENOTEA';
expect(errors.isMochaError(err), 'to be false');
});
});

describe('when provided a non-error', function() {
it('should return false', function() {
expect(errors.isMochaError(), 'to be false');
});
});
});
});