fix: ensure that exit code for fatal errors is not overwritten (#17683)

* fix: ensure that exit code for fatal errors is not overwritten

* Add comments

* update test

bin/eslint.js

@@ -97,16 +97,22 @@ function getErrorMessage(error) {
  * same message once.
  * @type {Set<string>}
  */
-
 const displayedErrors = new Set();
 
+/**
+ * Tracks whether an unexpected error was caught
+ * @type {boolean}
+ */
+let hadFatalError = false;
+
 /**
  * Catch and report unexpected error.
  * @param {any} error The thrown error object.
  * @returns {void}
  */
 function onFatalError(error) {
     process.exitCode = 2;
+    hadFatalError = true;
 
     const { version } = require("../package.json");
     const message = `
@@ -143,9 +149,25 @@ ${getErrorMessage(error)}`;
     }
 
     // Otherwise, call the CLI.
-    process.exitCode = await require("../lib/cli").execute(
+    const exitCode = await require("../lib/cli").execute(
         process.argv,
         process.argv.includes("--stdin") ? await readStdin() : null,
         true
     );
+
+    /*
+     * If an uncaught exception or unhandled rejection was detected in the meantime,
+     * keep the fatal exit code 2 that is already assigned to `process.exitCode`.
+     * Without this condition, exit code 2 (unsuccessful execution) could be overwritten with
+     * 1 (successful execution, lint problems found) or even 0 (successful execution, no lint problems found).
+     * This ensures that unexpected errors that seemingly don't affect the success
+     * of the execution will still cause a non-zero exit code, as it's a common
+     * practice and the default behavior of Node.js to exit with non-zero
+     * in case of an uncaught exception or unhandled rejection.
+     *
+     * Otherwise, assign the exit code returned from CLI.
+     */
+    if (!hadFatalError) {
+        process.exitCode = exitCode;
+    }
 }()).catch(onFatalError);

tests/bin/eslint.js

@@ -387,6 +387,24 @@ describe("bin/eslint.js", () => {
             return Promise.all([exitCodeAssertion, outputAssertion]);
         });
 
+        it("does not exit with zero when there is an error in the next tick", () => {
+            const config = path.join(__dirname, "../fixtures/bin/eslint.config-promise-tick-throws.js");
+            const file = path.join(__dirname, "../fixtures/bin/empty.js");
+            const child = runESLint(["--config", config, file]);
+            const exitCodeAssertion = assertExitCode(child, 2);
+            const outputAssertion = getOutput(child).then(output => {
+
+                // ensure the expected error was printed
+                assert.include(output.stderr, "test_error_stack");
+
+                // ensure that linting the file did not cause an error
+                assert.notInclude(output.stderr, "empty.js");
+                assert.notInclude(output.stdout, "empty.js");
+            });
+
+            return Promise.all([exitCodeAssertion, outputAssertion]);
+        });
+
         // https://github.com/eslint/eslint/issues/17560
         describe("does not print duplicate errors in the event of a crash", () => {
 

tests/fixtures/bin/eslint.config-promise-tick-throws.js

@@ -0,0 +1,22 @@
+function throwError() {
+    const error = new Error();
+    error.stack = "test_error_stack";
+    throw error;
+}
+
+process.nextTick(throwError);
+
+function delay(ms) {
+    return new Promise(resolve => setTimeout(resolve, ms));
+}
+
+async function getConfig() {
+    await delay(100);
+    return [];
+}
+
+/*
+ * Exporting the config as an initially unsettled Promise should ensure that
+ * the error in next tick will be thrown before any linting is done
+ */
+module.exports = getConfig();