Skip to content

Commit

Permalink
src: consolidate exit codes in the code base
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
joyeecheung committed Oct 6, 2022
1 parent 66cedb4 commit be525d7
Show file tree
Hide file tree
Showing 36 changed files with 379 additions and 175 deletions.
4 changes: 3 additions & 1 deletion lib/internal/async_hooks.js
Expand Up @@ -8,6 +8,8 @@ const {
Symbol,
} = primordials;

const { exitCodes: { kGenericUserError } } = internalBinding('errors');

const promiseHooks = require('internal/promise_hooks');

const async_wrap = internalBinding('async_wrap');
Expand Down Expand Up @@ -171,7 +173,7 @@ function fatalError(e) {
if (getOptionValue('--abort-on-uncaught-exception')) {
process.abort();
}
process.exit(1);
process.exit(kGenericUserError);
}

function lookupPublicResource(resource) {
Expand Down
8 changes: 5 additions & 3 deletions lib/internal/cluster/child.js
Expand Up @@ -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();
Expand Down Expand Up @@ -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);
}
});

Expand Down Expand Up @@ -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));
}
};
17 changes: 12 additions & 5 deletions lib/internal/debugger/inspect.js
Expand Up @@ -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.
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -234,7 +240,7 @@ class NodeInspector {
}
}
this.stdout.write(' failed to connect, please retry\n');
process.exit(1);
process.exit(kGenericUserError);
}

clearLine() {
Expand Down Expand Up @@ -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;
}
Expand All @@ -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);
Expand All @@ -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);
Expand Down
5 changes: 4 additions & 1 deletion lib/internal/main/repl.js
Expand Up @@ -17,6 +17,8 @@ const console = require('internal/console/global');

const { getOptionValue } = require('internal/options');

const { exitCodes: { kGenericUserError } } = internalBinding('errors');

prepareMainThreadExecution();

markBootstrapComplete();
Expand All @@ -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(() => {
Expand Down
3 changes: 2 additions & 1 deletion lib/internal/main/test_runner.js
Expand Up @@ -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();
Expand All @@ -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;
});
8 changes: 5 additions & 3 deletions lib/internal/main/watch_mode.js
Expand Up @@ -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');
Expand All @@ -24,7 +27,6 @@ const { setTimeout, clearTimeout } = require('timers');
const { resolve } = require('path');
const { once, on } = require('events');


prepareMainThreadExecution(false, false);
markBootstrapComplete();

Expand Down Expand Up @@ -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'));
Expand Down
3 changes: 2 additions & 1 deletion lib/internal/main/worker_thread.js
Expand Up @@ -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();
Expand Down Expand Up @@ -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);
}
Expand Down
4 changes: 3 additions & 1 deletion 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 = {
Expand Down
4 changes: 3 additions & 1 deletion lib/internal/policy/manifest.js
Expand Up @@ -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

Expand Down Expand Up @@ -72,7 +74,7 @@ function REACTION_EXIT(error) {
if (shouldAbortOnUncaughtException) {
abort();
}
exit(1);
exit(kGenericUserError);
}

function REACTION_LOG(error) {
Expand Down
5 changes: 3 additions & 2 deletions lib/internal/process/execution.js
Expand Up @@ -14,6 +14,7 @@ const {
ERR_EVAL_ESM_CANNOT_PRINT,
},
} = require('internal/errors');
const { exitCodes: { kGenericUserError } } = internalBinding('errors');

const {
executionAsyncId,
Expand Down Expand Up @@ -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.
Expand Down
5 changes: 3 additions & 2 deletions lib/internal/process/per_thread.js
Expand Up @@ -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');

Expand Down Expand Up @@ -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) {
Expand Down
5 changes: 3 additions & 2 deletions lib/internal/process/promises.js
Expand Up @@ -24,7 +24,8 @@ const { deprecate } = require('internal/util');

const {
noSideEffectsToString,
triggerUncaughtException
triggerUncaughtException,
exitCodes: { kGenericUserError },
} = internalBinding('errors');

const {
Expand Down Expand Up @@ -294,7 +295,7 @@ function processPromiseRejections() {
const handled = emit(reason, promise, promiseInfo);
if (!handled) {
emitUnhandledRejectionWarning(uid, reason);
process.exitCode = 1;
process.exitCode = kGenericUserError;
}
break;
}
Expand Down
4 changes: 3 additions & 1 deletion lib/internal/test_runner/harness.js
Expand Up @@ -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');
Expand Down Expand Up @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion lib/internal/test_runner/runner.js
Expand Up @@ -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'];

Expand Down Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions node.gyp
Expand Up @@ -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',
Expand Down
16 changes: 12 additions & 4 deletions src/api/embed_helpers.cc
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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);
{
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit be525d7

Please sign in to comment.