🐛 Bug: unhandledRejection in the default Runner kills process with 0 status code

[alxndrsn]

Bug Report Checklist

• I have read and agree to Mocha's Code of Conduct and Contributing Guidelines
• I have searched for related issues and issues with the faq label, but none matched my issue.
• I have 'smoke tested' the code to be tested by running it outside the real test suite to get a better sense of whether the problem is in the code under test, my usage of Mocha, or Mocha itself.
• I want to provide a PR to resolve this

Expected

Tests should fail if there's an unhandled rejection with no process-level listener.

Actual

As demonstrated in #5505, a test which throws an unhandled promise rejection inside runner code will cause the mocha process to exit with status code 0.

Minimal, Reproducible Example

$ echo 'it("t", async () => { require("node:assert").deepEqual(Object.create(null), { a:1 }); });' > demo.js; npx mocha demo.js; echo $?


0

Versions

$ node --version
v22.14.0

$ npx mocha --version
11.7.4

Additional Info

No response

[alxndrsn]

Looks like a missing try/catch for event handler:

mocha/lib/reporters/base.js

Lines 386 to 398 in f4e7e54

	runner.on(EVENT_TEST_FAIL, function (test, err) {
	if (showDiff(err)) {
	stringifyDiffObjs(err);
	}
	// more than one error per test
	if (test.err && err instanceof Error) {
	test.err.multiple = (test.err.multiple || []).concat(err);
	} else {
	test.err = err;
	}
	failures.push(test);
	});
	}

[alxndrsn]

Maybe something like:

--- a/lib/reporters/base.js
+++ b/lib/reporters/base.js
@@ -19,6 +19,9 @@ var utils = require('../utils');
 var supportsColor = require('supports-color');
 var symbols = require('log-symbols');
 var constants = require('../runner').constants;
+const {
+  createFatalError,
+} = require('../errors');
 var EVENT_TEST_PASS = constants.EVENT_TEST_PASS;
 var EVENT_TEST_FAIL = constants.EVENT_TEST_FAIL;
 
@@ -384,16 +387,23 @@ function Base(runner, options) {
   });
 
   runner.on(EVENT_TEST_FAIL, function (test, err) {
-    if (showDiff(err)) {
-      stringifyDiffObjs(err);
-    }
-    // more than one error per test
-    if (test.err && err instanceof Error) {
-      test.err.multiple = (test.err.multiple || []).concat(err);
-    } else {
-      test.err = err;
+    try {
+      if (showDiff(err)) {
+        stringifyDiffObjs(err);
+      }
+      // more than one error per test
+      if (test.err && err instanceof Error) {
+        test.err.multiple = (test.err.multiple || []).concat(err);
+      } else {
+        test.err = err;
+      }
+      failures.push(test);
+    } catch(err2) {
+      throw createFatalError(
+        `Error handling event: '${EVENT_TEST_FAIL}'`,
+        err2
+      );
     }
-    failures.push(test);
   });
 }

[JoshuaKGoldberg]

While we're on the subject, do you have an opinion on #5251?

[alxndrsn]

While we're on the subject, do you have an opinion on #5251?

Interesting! I've added some thoughts on that ticket, but I definitely think they're separate issues (rejections occurring in user-supplied test code vs mocha's internal code).

[alxndrsn]

Maybe something like: ...

It's not actually as simple as the code suggested in #5507 (comment)

Converting the error to a mocha fatal error switches the path followed in

mocha/lib/runner.js

Lines 217 to 239 in 681e843

	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);
	}
	}
	};

This means the error is now passed to _uncaught(). However, as the current test has already failed, it ends up being ignored at

mocha/lib/runner.js

Lines 1045 to 1049 in 681e843

	if (runnable.isFailed()) {
	debug('uncaught(): Runnable has already failed');
	// Ignore error if already failed
	return;
	} else if (runnable.isPending()) {

This causes the failure to be even more silent:

$ echo 'it("t", async () => { require("node:assert").deepEqual(Object.create(null), { a:1 }); });' > demo.js; npx mocha demo.js; echo $?


0

---

One more fail-safe approach would be to abort() the runner, either from inside the event handling at

mocha/lib/reporters/base.js

Line 386 in 681e843

runner.on(EVENT_TEST_FAIL, function (test, err) {

. However, this gives zero information to the user:

$ echo 'it("t", async () => { require("node:assert").deepEqual(Object.create(null), { a:1 }); });' > demo.js; npx mocha demo.js; echo $?


  1) t

  0 passing (3ms)
  1 failing



1

[alxndrsn]

One more fail-safe approach would be to abort() the runner, either from inside the event handling

Maybe this could be done, with an additional line of logging.

Adding:

Base.consoleLog(`Fatal error in handling for event '${EVENT_TEST_FAIL}':`, err2);

gives output:

$ echo 'it("t", async () => { require("node:assert").deepEqual(Object.create(null), { a:1 }); });' > demo.js; npx mocha demo.js; echo $?


Fatal error in handling for event 'fail': TypeError: value.toString is not a function
    at emptyRepresentation (lib/utils.js:116:20)
    at exports.stringify (lib/utils.js:251:10)
    at stringifyDiffObjs (lib/reporters/base.js:190:24)
    at Runner.<anonymous> (lib/reporters/base.js:392:9)
    at Runner.emit (node:events:520:35)
    at Runner.fail (lib/runner.js:506:8)
    at lib/runner.js:867:18
    at done (lib/runnable.js:310:5)
    at lib/runnable.js:377:11
  1) t

  0 passing (5ms)
  1 failing



1