Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src: consolidate exit codes in the code base #44746

Merged
merged 6 commits into from
Oct 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion lib/internal/async_hooks.js
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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);
ljharb marked this conversation as resolved.
Show resolved Hide resolved
} 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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Loading