fix(reporter): isolate reporter errors so a hostile failure payload can't corrupt the run

Author: mostafaNazari702

lib/reporters/base.mjs

@@ -83,8 +83,12 @@ export class Base {
     });
 
     runner.on(EVENT_TEST_FAIL, function (test, err) {
-      if (Base.showDiff(err)) {
-        stringifyDiffObjs(err);
+      try {
+        if (Base.showDiff(err)) {
+          stringifyDiffObjs(err);
+        }
+      } catch (diffErr) {
+        warnReporterError("preparing the failure diff", diffErr);
       }
       // more than one error per test
       if (test.err && err instanceof Error) {
@@ -325,27 +329,52 @@ function estimateSize(val, maxDepth, seen) {
   return 50; // Default estimate for other types
 }
 
+/**
+ * Warn (on a best effort basis) that the reporter itself threw while handling a failure.
+ * Used so a hostile error payload cannot silently suppress the non-zero exit code.
+ *
+ * @private
+ * @param {string} context - what the reporter was doing when it threw
+ * @param {*} err - the error the reporter threw
+ */
+function warnReporterError(context, err) {
+  try {
+    process.stderr.write(
+      "\n[mocha] reporter error while " +
+        context +
+        ": " +
+        (err && err.stack ? err.stack : String(err)) +
+        "\n",
+    );
+  } catch {}
+}
+
 function stringifyDiffObjs(err) {
   if (!utils.isString(err.actual) || !utils.isString(err.expected)) {
     // Estimate size before stringifying to avoid hangs
     const maxSafeSize = Base.maxDiffSize || 8192;
-    const actualSize = estimateSize(err.actual, 10);
-    const expectedSize = estimateSize(err.expected, 10);
-
-    if (
-      actualSize === -1 ||
-      expectedSize === -1 ||
-      actualSize > maxSafeSize ||
-      expectedSize > maxSafeSize
-    ) {
-      // Values too large/complex - provide safe fallback
-      err.actual = "[object too large to diff]";
-      err.expected = "[object too large to diff]";
-      return;
-    }
+    try {
+      const actualSize = estimateSize(err.actual, 10);
+      const expectedSize = estimateSize(err.expected, 10);
+
+      if (
+        actualSize === -1 ||
+        expectedSize === -1 ||
+        actualSize > maxSafeSize ||
+        expectedSize > maxSafeSize
+      ) {
+        // Values too large/complex - provide safe fallback
+        err.actual = "[object too large to diff]";
+        err.expected = "[object too large to diff]";
+        return;
+      }
 
-    err.actual = utils.stringify(err.actual);
-    err.expected = utils.stringify(err.expected);
+      err.actual = utils.stringify(err.actual);
+      err.expected = utils.stringify(err.expected);
+    } catch {
+      err.actual = "[object could not be stringified]";
+      err.expected = "[object could not be stringified]";
+    }
   }
 }
 
@@ -454,6 +483,19 @@ Base.list = function (failures) {
   var multipleErr, multipleTest;
   Base.consoleLog();
   failures.forEach(function (test, i) {
+    try {
+      renderOneFailure(test, i);
+    } catch (renderErr) {
+      Base.consoleLog(
+        "  %s) %s:\n     [mocha] failed to render error: %s",
+        i + 1,
+        test && test.titlePath ? test.titlePath().join(" ") : "<unknown>",
+        renderErr && renderErr.message ? renderErr.message : renderErr,
+      );
+    }
+  });
+
+  function renderOneFailure(test, i) {
     // format
     var fmt =
       Base.color("error title", "  %s) %s:\n") +
@@ -506,7 +548,7 @@ Base.list = function (failures) {
     });
 
     Base.consoleLog(fmt, i + 1, testTitle, msg, stack);
-  });
+  }
 };
 
 /**

test/integration/fixtures/reporters/listener-throws.fixture.js

@@ -0,0 +1,25 @@
+"use strict";
+
+const assert = require("node:assert");
+
+it("listener-throws", () => {
+  const poison = new Proxy(
+    {},
+    {
+      ownKeys() {
+        throw new TypeError("poisoned ownKeys");
+      },
+      getOwnPropertyDescriptor() {
+        throw new TypeError("poisoned descriptor");
+      },
+      get() {
+        throw new TypeError("poisoned get");
+      },
+    },
+  );
+  throw new assert.AssertionError({
+    actual: poison,
+    expected: { a: 1 },
+    message: "boom",
+  });
+});

test/integration/reporter-crash.spec.js

@@ -0,0 +1,34 @@
+"use strict";
+
+const { invokeMochaAsync, resolveFixturePath } = require("./helpers");
+
+describe("reporter resilience", function () {
+  this.timeout(10000);
+
+  let res;
+
+  before(async function () {
+    const [, promise] = invokeMochaAsync(
+      [resolveFixturePath("reporters/listener-throws")],
+      { stdio: "pipe" },
+    );
+    res = await promise;
+  });
+
+  it("should exit non-zero when the reporter throws while handling a failure", function () {
+    expect(res, "to have failed");
+  });
+
+  it("should warn that the reporter threw instead of crashing", function () {
+    expect(
+      res.output,
+      "to match",
+      /\[mocha\] (reporter error|failed to render)/,
+    );
+  });
+
+  it("should report the failure exactly once, not double-count the reporter throw", function () {
+    expect(res.output, "to match", /1 failing/);
+    expect(res.output, "not to contain", "2 failing");
+  });
+});

test/reporters/base.spec.js

@@ -593,6 +593,59 @@ describe("Base reporter", function () {
     });
   });
 
+  describe("reporter resilience", function () {
+    it("should not throw out of Base.list when rendering a failure throws", function () {
+      var poison = new Proxy(
+        {},
+        {
+          ownKeys: function () {
+            throw new TypeError("poisoned ownKeys");
+          },
+          getOwnPropertyDescriptor: function () {
+            throw new TypeError("poisoned descriptor");
+          },
+          get: function () {
+            throw new TypeError("poisoned get");
+          },
+        },
+      );
+      var err = new AssertionError({
+        actual: poison,
+        expected: { a: 1 },
+        message: "boom",
+      });
+      var test = makeTest(err);
+
+      expect(function () {
+        list([test]);
+      }, "not to throw");
+
+      var errOut = stdout.join("\n");
+      expect(errOut, "to contain", "test title");
+      expect(errOut, "to contain", "failed to render error");
+    });
+
+    it("should fall back to a placeholder when stringifyDiffObjs throws", function () {
+      var utils = require("../../lib/utils");
+      sinon
+        .stub(utils, "stringify")
+        .throws(new Error("simulated stringify failure"));
+      var err = new AssertionError({
+        actual: { a: 1 },
+        expected: { a: 2 },
+        message: "boom",
+      });
+      var test = makeTest(err);
+
+      expect(function () {
+        list([test]);
+      }, "not to throw");
+      var errOut = stdout.join("\n");
+      expect(errOut, "to contain", "test title");
+      expect(errOut, "not to contain", "failed to render error");
+    });
+  });
+
   it("should list multiple Errors per test", function () {
     var err = new Error("First Error");
     err.multiple = [new Error("Second Error - same test")];