src: consolidate exit codes in the code base

Add an ExitCode enum class and use it throughout the code base instead of hard-coding the exit codes everywhere. At the moment, the exit codes used in many places do not actually conform to what the documentation describes. With the new enums (which are also available to the JS land as constants in an internal binding) we could migrate to a more consistent usage of the codes, and eventually expose the constants to the user land when they are stable enough. PR-URL: #44746 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com>
nodejs · Oct 6, 2022 · be525d7 · be525d7
1 parent 66cedb4
commit be525d7
Show file tree

Hide file tree

Showing 36 changed files with 379 additions and 175 deletions.
diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js
@@ -8,6 +8,8 @@ const {
   Symbol,
 } = primordials;
 
+const { exitCodes: { kGenericUserError } } = internalBinding('errors');
+
 const promiseHooks = require('internal/promise_hooks');
 
 const async_wrap = internalBinding('async_wrap');
@@ -171,7 +173,7 @@ function fatalError(e) {
   if (getOptionValue('--abort-on-uncaught-exception')) {
     process.abort();
   }
-  process.exit(1);
+  process.exit(kGenericUserError);
 }
 
 function lookupPublicResource(resource) {

diff --git a/lib/internal/cluster/child.js b/lib/internal/cluster/child.js
@@ -15,6 +15,8 @@ const EventEmitter = require('events');
 const { owner_symbol } = require('internal/async_hooks').symbols;
 const Worker = require('internal/cluster/worker');
 const { internal, sendHelper } = require('internal/cluster/utils');
+const { exitCodes: { kNoFailure } } = internalBinding('errors');
+
 const cluster = new EventEmitter();
 const handles = new SafeMap();
 const indexes = new SafeMap();
@@ -43,7 +45,7 @@ cluster._setupWorker = function() {
     if (!worker.exitedAfterDisconnect) {
       // Unexpected disconnect, primary exited, or some such nastiness, so
       // worker exits immediately.
-      process.exit(0);
+      process.exit(kNoFailure);
     }
   });
 
@@ -278,10 +280,10 @@ Worker.prototype.destroy = function() {
 
   this.exitedAfterDisconnect = true;
   if (!this.isConnected()) {
-    process.exit(0);
+    process.exit(kNoFailure);
   } else {
     this.state = 'destroying';
     send({ act: 'exitedAfterDisconnect' }, () => process.disconnect());
-    process.once('disconnect', () => process.exit(0));
+    process.once('disconnect', () => process.exit(kNoFailure));
   }
 };
diff --git a/lib/internal/debugger/inspect.js b/lib/internal/debugger/inspect.js
@@ -44,6 +44,12 @@ const { 0: InspectClient, 1: createRepl } =
 const debuglog = util.debuglog('inspect');
 
 const { ERR_DEBUGGER_STARTUP_ERROR } = require('internal/errors').codes;
+const {
+  exitCodes: {
+    kGenericUserError,
+    kNoFailure,
+  },
+} = internalBinding('errors');
 
 async function portIsFree(host, port, timeout = 3000) {
   if (port === 0) return; // Binding to a random port.
@@ -167,7 +173,7 @@ class NodeInspector {
 
     // Handle all possible exits
     process.on('exit', () => this.killChild());
-    const exitCodeZero = () => process.exit(0);
+    const exitCodeZero = () => process.exit(kNoFailure);
     process.once('SIGTERM', exitCodeZero);
     process.once('SIGHUP', exitCodeZero);
 
@@ -234,7 +240,7 @@ class NodeInspector {
       }
     }
     this.stdout.write(' failed to connect, please retry\n');
-    process.exit(1);
+    process.exit(kGenericUserError);
   }
 
   clearLine() {
@@ -314,7 +320,7 @@ function parseArgv(args) {
     } catch (e) {
       if (e.code === 'ESRCH') {
         console.error(`Target process: ${pid} doesn't exist.`);
-        process.exit(1);
+        process.exit(kGenericUserError);
       }
       throw e;
     }
@@ -337,7 +343,8 @@ function startInspect(argv = ArrayPrototypeSlice(process.argv, 2),
     console.error(`       ${invokedAs} <host>:<port>`);
     console.error(`       ${invokedAs} --port=<port>`);
     console.error(`       ${invokedAs} -p <pid>`);
-    process.exit(1);
+    // TODO(joyeecheung): should be kInvalidCommandLineArgument.
+    process.exit(kGenericUserError);
   }
 
   const options = parseArgv(argv);
@@ -355,7 +362,7 @@ function startInspect(argv = ArrayPrototypeSlice(process.argv, 2),
       console.error(e.message);
     }
     if (inspector.child) inspector.child.kill();
-    process.exit(1);
+    process.exit(kGenericUserError);
   }
 
   process.on('uncaughtException', handleUnexpectedError);

diff --git a/lib/internal/main/repl.js b/lib/internal/main/repl.js
@@ -17,6 +17,8 @@ const console = require('internal/console/global');
 
 const { getOptionValue } = require('internal/options');
 
+const { exitCodes: { kGenericUserError } } = internalBinding('errors');
+
 prepareMainThreadExecution();
 
 markBootstrapComplete();
@@ -31,7 +33,8 @@ if (process.env.NODE_REPL_EXTERNAL_MODULE) {
     // If we can't write to stderr, we'd like to make this a noop,
     // so use console.error.
     console.error('Cannot specify --input-type for REPL');
-    process.exit(1);
+    // TODO(joyeecheung): should be kInvalidCommandLineArgument.
+    process.exit(kGenericUserError);
   }
 
   esmLoader.loadESM(() => {

diff --git a/lib/internal/main/test_runner.js b/lib/internal/main/test_runner.js
@@ -5,6 +5,7 @@ const {
 } = require('internal/process/pre_execution');
 const { isUsingInspector } = require('internal/util/inspector');
 const { run } = require('internal/test_runner/runner');
+const { exitCodes: { kGenericUserError } } = internalBinding('errors');
 
 prepareMainThreadExecution(false);
 markBootstrapComplete();
@@ -22,5 +23,5 @@ if (isUsingInspector()) {
 const tapStream = run({ concurrency, inspectPort });
 tapStream.pipe(process.stdout);
 tapStream.once('test:fail', () => {
-  process.exitCode = 1;
+  process.exitCode = kGenericUserError;
 });
diff --git a/lib/internal/main/watch_mode.js b/lib/internal/main/watch_mode.js
@@ -12,7 +12,10 @@ const {
   prepareMainThreadExecution,
   markBootstrapComplete
 } = require('internal/process/pre_execution');
-const { triggerUncaughtException } = internalBinding('errors');
+const {
+  triggerUncaughtException,
+  exitCodes: { kNoFailure },
+} = internalBinding('errors');
 const { getOptionValue } = require('internal/options');
 const { emitExperimentalWarning } = require('internal/util');
 const { FilesWatcher } = require('internal/watch_mode/files_watcher');
@@ -24,7 +27,6 @@ const { setTimeout, clearTimeout } = require('timers');
 const { resolve } = require('path');
 const { once, on } = require('events');
 
-
 prepareMainThreadExecution(false, false);
 markBootstrapComplete();
 
@@ -125,7 +127,7 @@ function signalHandler(signal) {
   return async () => {
     watcher.clear();
     const exitCode = await killAndWait(signal, true);
-    process.exit(exitCode ?? 0);
+    process.exit(exitCode ?? kNoFailure);
   };
 }
 process.on('SIGTERM', signalHandler('SIGTERM'));

diff --git a/lib/internal/main/worker_thread.js b/lib/internal/main/worker_thread.js
@@ -66,6 +66,7 @@ let debug = require('internal/util/debuglog').debuglog('worker', (fn) => {
 });
 
 const assert = require('internal/assert');
+const { exitCodes: { kGenericUserError } } = internalBinding('errors');
 
 patchProcessObject();
 setupInspectorHooks();
@@ -234,7 +235,7 @@ function workerOnGlobalUncaughtException(error, fromPromise) {
   if (!process._exiting) {
     try {
       process._exiting = true;
-      process.exitCode = 1;
+      process.exitCode = kGenericUserError;
       if (!handlerThrew) {
         process.emit('exit', process.exitCode);
       }

diff --git a/lib/internal/modules/esm/handle_process_exit.js b/lib/internal/modules/esm/handle_process_exit.js
@@ -1,10 +1,12 @@
 'use strict';
 
+const { exitCodes: { kUnfinishedTopLevelAwait } } = internalBinding('errors');
+
 // Handle a Promise from running code that potentially does Top-Level Await.
 // In that case, it makes sense to set the exit code to a specific non-zero
 // value if the main code never finishes running.
 function handleProcessExit() {
-  process.exitCode ??= 13;
+  process.exitCode ??= kUnfinishedTopLevelAwait;
 }
 
 module.exports = {

diff --git a/lib/internal/policy/manifest.js b/lib/internal/policy/manifest.js
@@ -40,6 +40,8 @@ const { getOptionValue } = require('internal/options');
 const shouldAbortOnUncaughtException = getOptionValue(
   '--abort-on-uncaught-exception'
 );
+const { exitCodes: { kGenericUserError } } = internalBinding('errors');
+
 const { abort, exit, _rawDebug } = process;
 // #endregion
 
@@ -72,7 +74,7 @@ function REACTION_EXIT(error) {
   if (shouldAbortOnUncaughtException) {
     abort();
   }
-  exit(1);
+  exit(kGenericUserError);
 }
 
 function REACTION_LOG(error) {

diff --git a/lib/internal/process/execution.js b/lib/internal/process/execution.js
@@ -14,6 +14,7 @@ const {
     ERR_EVAL_ESM_CANNOT_PRINT,
   },
 } = require('internal/errors');
+const { exitCodes: { kGenericUserError } } = internalBinding('errors');
 
 const {
   executionAsyncId,
@@ -161,8 +162,8 @@ function createOnGlobalUncaughtException() {
       try {
         if (!process._exiting) {
           process._exiting = true;
-          process.exitCode = 1;
-          process.emit('exit', 1);
+          process.exitCode = kGenericUserError;
+          process.emit('exit', kGenericUserError);
         }
       } catch {
         // Nothing to be done about it at this point.

diff --git a/lib/internal/process/per_thread.js b/lib/internal/process/per_thread.js
@@ -58,6 +58,7 @@ const kInternal = Symbol('internal properties');
 function assert(x, msg) {
   if (!x) throw new ERR_ASSERTION(msg || 'assertion error');
 }
+const { exitCodes: { kNoFailure } } = internalBinding('errors');
 
 const binding = internalBinding('process_methods');
 
@@ -187,12 +188,12 @@ function wrapProcessMethods(binding) {
 
     if (!process._exiting) {
       process._exiting = true;
-      process.emit('exit', process.exitCode || 0);
+      process.emit('exit', process.exitCode || kNoFailure);
     }
     // FIXME(joyeecheung): This is an undocumented API that gets monkey-patched
     // in the user land. Either document it, or deprecate it in favor of a
     // better public alternative.
-    process.reallyExit(process.exitCode || 0);
+    process.reallyExit(process.exitCode || kNoFailure);
   }
 
   function kill(pid, sig) {

diff --git a/lib/internal/process/promises.js b/lib/internal/process/promises.js
@@ -24,7 +24,8 @@ const { deprecate } = require('internal/util');
 
 const {
   noSideEffectsToString,
-  triggerUncaughtException
+  triggerUncaughtException,
+  exitCodes: { kGenericUserError },
 } = internalBinding('errors');
 
 const {
@@ -294,7 +295,7 @@ function processPromiseRejections() {
           const handled = emit(reason, promise, promiseInfo);
           if (!handled) {
             emitUnhandledRejectionWarning(uid, reason);
-            process.exitCode = 1;
+            process.exitCode = kGenericUserError;
           }
           break;
         }

diff --git a/lib/internal/test_runner/harness.js b/lib/internal/test_runner/harness.js
@@ -13,6 +13,8 @@ const {
     ERR_TEST_FAILURE,
   },
 } = require('internal/errors');
+const { exitCodes: { kGenericUserError } } = internalBinding('errors');
+
 const { kEmptyObject } = require('internal/util');
 const { getOptionValue } = require('internal/options');
 const { kCancelledByParent, Test, ItTest, Suite } = require('internal/test_runner/test');
@@ -118,7 +120,7 @@ function getGlobalRoot() {
     globalRoot = createTestTree();
     globalRoot.reporter.pipe(process.stdout);
     globalRoot.reporter.once('test:fail', () => {
-      process.exitCode = 1;
+      process.exitCode = kGenericUserError;
     });
   }
   return globalRoot;

diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js
@@ -34,6 +34,7 @@ const {
 } = require('internal/test_runner/utils');
 const { basename, join, resolve } = require('path');
 const { once } = require('events');
+const { exitCodes: { kGenericUserError } } = internalBinding('errors');
 
 const kFilterArgs = ['--test'];
 
@@ -90,7 +91,7 @@ function createTestFileList() {
   } catch (err) {
     if (err?.code === 'ENOENT') {
       console.error(`Could not find '${err.path}'`);
-      process.exit(1);
+      process.exit(kGenericUserError);
     }
 
     throw err;

diff --git a/node.gyp b/node.gyp
@@ -600,6 +600,7 @@
         'src/node_contextify.h',
         'src/node_dir.h',
         'src/node_errors.h',
+        'src/node_exit_code.h',
         'src/node_external_reference.h',
         'src/node_file.h',
         'src/node_file-inl.h',

diff --git a/src/api/embed_helpers.cc b/src/api/embed_helpers.cc
@@ -7,6 +7,7 @@ using v8::Function;
 using v8::Global;
 using v8::HandleScope;
 using v8::Isolate;
+using v8::Just;
 using v8::Local;
 using v8::Locker;
 using v8::Maybe;
@@ -15,7 +16,7 @@ using v8::SealHandleScope;
 
 namespace node {
 
-Maybe<int> SpinEventLoop(Environment* env) {
+Maybe<ExitCode> SpinEventLoopInternal(Environment* env) {
   CHECK_NOT_NULL(env);
   MultiIsolatePlatform* platform = GetMultiIsolatePlatform(env);
   CHECK_NOT_NULL(platform);
@@ -25,7 +26,7 @@ Maybe<int> SpinEventLoop(Environment* env) {
   Context::Scope context_scope(env->context());
   SealHandleScope seal(isolate);
 
-  if (env->is_stopping()) return Nothing<int>();
+  if (env->is_stopping()) return Nothing<ExitCode>();
 
   env->set_trace_sync_io(env->options()->trace_sync_io);
   {
@@ -59,7 +60,7 @@ Maybe<int> SpinEventLoop(Environment* env) {
     env->performance_state()->Mark(
         node::performance::NODE_PERFORMANCE_MILESTONE_LOOP_EXIT);
   }
-  if (env->is_stopping()) return Nothing<int>();
+  if (env->is_stopping()) return Nothing<ExitCode>();
 
   env->set_trace_sync_io(false);
   // Clear the serialize callback even though the JS-land queue should
@@ -69,7 +70,7 @@ Maybe<int> SpinEventLoop(Environment* env) {
 
   env->PrintInfoForSnapshotIfDebug();
   env->ForEachRealm([](Realm* realm) { realm->VerifyNoStrongBaseObjects(); });
-  return EmitProcessExit(env);
+  return EmitProcessExitInternal(env);
 }
 
 struct CommonEnvironmentSetup::Impl {
@@ -155,6 +156,13 @@ CommonEnvironmentSetup::~CommonEnvironmentSetup() {
   delete impl_;
 }
 
+Maybe<int> SpinEventLoop(Environment* env) {
+  Maybe<ExitCode> result = SpinEventLoopInternal(env);
+  if (result.IsNothing()) {
+    return Nothing<int>();
+  }
+  return Just(static_cast<int>(result.FromJust()));
+}
 
 uv_loop_t* CommonEnvironmentSetup::event_loop() const {
   return &impl_->loop;