From 1ad6257744d63281235fcc33288394b1d69b34ce Mon Sep 17 00:00:00 2001
From: Milos Djermanovic <milos.djermanovic@gmail.com>
Date: Fri, 27 Oct 2023 13:36:58 +0200
Subject: [PATCH] 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                                 | 26 +++++++++++++++++--
 tests/bin/eslint.js                           | 18 +++++++++++++
 tests/fixtures/bin/empty.js                   |  0
 .../bin/eslint.config-promise-tick-throws.js  | 22 ++++++++++++++++
 4 files changed, 64 insertions(+), 2 deletions(-)
 create mode 100644 tests/fixtures/bin/empty.js
 create mode 100644 tests/fixtures/bin/eslint.config-promise-tick-throws.js

diff --git a/bin/eslint.js b/bin/eslint.js
index 5c7972cc086..eeb4647e70b 100755
--- a/bin/eslint.js
+++ b/bin/eslint.js
@@ -97,9 +97,14 @@ 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.
@@ -107,6 +112,7 @@ const displayedErrors = new Set();
  */
 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);
diff --git a/tests/bin/eslint.js b/tests/bin/eslint.js
index dca8955d038..430f7112e96 100644
--- a/tests/bin/eslint.js
+++ b/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", () => {
 
diff --git a/tests/fixtures/bin/empty.js b/tests/fixtures/bin/empty.js
new file mode 100644
index 00000000000..e69de29bb2d
diff --git a/tests/fixtures/bin/eslint.config-promise-tick-throws.js b/tests/fixtures/bin/eslint.config-promise-tick-throws.js
new file mode 100644
index 00000000000..6ea53d25386
--- /dev/null
+++ b/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();