mocha: unhandledRejection

+1 We were completely caught by surprise that mocha catches thrown errors but not unhandled rejections. Our tests passed when actually they should have failed.

describe("normal test", () => {
  it("bar", () => {
    Promise.reject(new Error("baz"));    
  });
  it("bar", () => {
    // nothing
  });
});

This is a synchronous test which happens to result in an unhandled rejection (suppose e.g. the code under test had an unintended floating promise). In this case, testing results in “foo” and “bar” succeeding and exit code 0.

You cannot prevent “foo” from succeeding since node will need a couple of ticks to decide that a promise is unhandled. However I would never expect exit code 0. If necessary, manually go through a couple of node cycles at the end of a mocha execution to wait for any unhandled rejections and then exit with an error message and non-zero code

Current work-around:

let unhandledRejectionExitCode = 0;

process.on("unhandledRejection", (reason) => {
	console.log("unhandled rejection:", reason);
	unhandledRejectionExitCode = 1;
	throw reason;
});

process.prependListener("exit", (code) => {
	if (code === 0) {
		process.exit(unhandledRejectionExitCode);
	}
});

Was debugging this issue. All Mocha does on unhandled rejections is just

process.emit('unhandledRejection', reason, promise);

This is useless and the issue must be reopen.

I could try to mitigate this by throwing myself in process.on('unhandledRejection'), which will lead to Mocha going into uncaught route, but it is a hack.

@juergba The same faulty/unwanted behavior described in this issue and fixed in 8.2.0 or earlier; i.e, that mocha silently ignores unhandled promise rejections that occur in tests instead of failing the tests.

@juergba Please reopen this issue. The faulty behavior was reintroduced in 8.2.1 - https://github.com/mochajs/mocha/pull/4489

@juergba To get unhandledRejection in the xxxx.test.ts at the beginning I need to add the following rows in order to grab the Exception raised in any async function.

process.on('unhandledRejection', (reason) => {
  throw reason;
});

E.G. In the following example mocha doesn’t print anything. Mocha@8.3.0

private async XXX() {
    throw new Error('fooo error');

It seems Mocha doesn’t handle this type of event and silently hides them. Is the expected behavior?

In Mocha v8.2.0 we added an unhandledRejection listener. Going to close this issue.

@eyalroth You would have to be more precise in order to know what is your “faulty behavior”. I have not implemented PR4489.

I have a WIP commit that implements unhandled rejection support: https://github.com/ofrobots/mocha/commit/712678ccd3605ed95b60072a1d6059052c4f9a6b. @boneskull If this is in line with what you had in mind in this comment I can go ahead and finish this off, write tests and open a PR.

This would be a semver major change IMO, so perhaps a --disallow-unhandled makes more sense than --allow-unhandled?

This is what I think needs to happen:

1. An unhandled rejection needs to result in the same (or similar) behavior as when an uncaught exception is thrown
2. A new flag, --allow-unhandled, should be added (if we piggybacked on --allow-uncaught, it would be more difficult to pin down either one in particular)
3. Plenty of tests.

Since Mocha doesn’t use Promises internally, this shouldn’t be a particularly severe change to the codebase. I think we can get away with basically copy-and-pasting the majority of the “uncaught exception” behavior, as well as the test suites.

There are likely some edge cases here I’m not considering, so this is best-case scenario. 😇

I seem to have this same issue in 10.2.0

In 1 week this shouldn’t matter as this is the default behavior in Node 15 🎉 :]

Note that this problem is even worse if you want it to be cross-browser. For example

new Promise((resolve, reject) => {
	throw new Error("bad");
});

logs the unhandled rejection in the console, while

Promise.resolve().then(() => {
	throw new Error("bad");
});

does not do anything in all of the browsers except Chrome and Opera, which are Chromium based. Both can be caught with .catch(), so the rejection is there, it just does not arrive to the console. Not even in Edge, which supports unhandled rejections according to MDN. I already sent a bug report about this to Firefox, but it feels like if we were 20 years ago, the core js implementation is breaking in different browsers again. :S I really hate client side js because of the constant bugfixing, workarounds and shims. :S

edit:

A simple fix for Chrome until we get unhandled rejection support in Mocha:

window.addEventListener("unhandledrejection", function (event){
    throw event.reason;
});

Here is another example of an UnhandledPromiseRejectionWarning that didn’t fail the build.

Using a combination of Mocha, Chai, and Sinon, we had an expect(...).to.be.rejectedWith(error) expression that wasn’t returned (return expect(...), and therefore logging an UnhandledPromiseRejectionWarning.

Once I added the return, I got an Cannot read property 'equal' of undefined error which was due to completely invalid usage in the then() block.

expect(fun.callCount.to.equal(1));

should have been:

expect(fun.callCount).to.equal(1);

Not sure how long we had an invalid test, but it was only by looking through the test log that we came upon UnhandledPromiseRejectionWarning.

For now, I’ve added this small library in the test suite to exit the process for UnhandledPromiseRejectionWarning. https://github.com/mcollina/make-promises-safe

It would be nice if the test suite continued running, so that other test failures would be shown, while still failing the build. Thanks!

Not sure if this is a separate issue or not, but I see similar behaviour without promises when deferring an error with setImmediate or process.nextTick.

// swallow.js
it("swallows deferred error", function() {
  setImmediate(() => { throw new Error("thrown error"); });
});

Running the test I get success with exit code 0.

$ node bin/mocha swallow.js


  ✓ swallows deferred error

  1 passing (5ms)

Whereas running the same code with node v8.9.0 gives prints the stack trace and exits non-zero:

$ node -e 'setImmediate(() => { throw new Error("thrown error"); });'
[eval]:1
setImmediate(() => { throw new Error("thrown error"); });
                     ^

Error: thrown error
    at Immediate.setImmediate [as _onImmediate] ([eval]:1:28)
    at runCallback (timers.js:789:20)
    at tryOnImmediate (timers.js:751:5)
    at processImmediate [as _immediateCallback] (timers.js:722:5)

Yes, I think that’s true.

This is definitely something we should add, since the current timeout behavior is quite uninformative and generally promise implementations support this hook; but let’s make sure the resulting test failure message indicates that these errors would have been swallowed by the promise in production and should be somehow handled or communicated to the caller for handling (at which point it’s easy for the test to communicate the failure back to Mocha without relying on unhandled rejection functions) – a lot of the questions we get about the current behavior come from lack of awareness of the need for catch.