Skip to content

Commit be525d7

Browse files
authored
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: nodejs#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>
1 parent 66cedb4 commit be525d7

36 files changed

+379
-175
lines changed

lib/internal/async_hooks.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ const {
88
Symbol,
99
} = primordials;
1010

11+
const { exitCodes: { kGenericUserError } } = internalBinding('errors');
12+
1113
const promiseHooks = require('internal/promise_hooks');
1214

1315
const async_wrap = internalBinding('async_wrap');
@@ -171,7 +173,7 @@ function fatalError(e) {
171173
if (getOptionValue('--abort-on-uncaught-exception')) {
172174
process.abort();
173175
}
174-
process.exit(1);
176+
process.exit(kGenericUserError);
175177
}
176178

177179
function lookupPublicResource(resource) {

lib/internal/cluster/child.js

+5-3
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ const EventEmitter = require('events');
1515
const { owner_symbol } = require('internal/async_hooks').symbols;
1616
const Worker = require('internal/cluster/worker');
1717
const { internal, sendHelper } = require('internal/cluster/utils');
18+
const { exitCodes: { kNoFailure } } = internalBinding('errors');
19+
1820
const cluster = new EventEmitter();
1921
const handles = new SafeMap();
2022
const indexes = new SafeMap();
@@ -43,7 +45,7 @@ cluster._setupWorker = function() {
4345
if (!worker.exitedAfterDisconnect) {
4446
// Unexpected disconnect, primary exited, or some such nastiness, so
4547
// worker exits immediately.
46-
process.exit(0);
48+
process.exit(kNoFailure);
4749
}
4850
});
4951

@@ -278,10 +280,10 @@ Worker.prototype.destroy = function() {
278280

279281
this.exitedAfterDisconnect = true;
280282
if (!this.isConnected()) {
281-
process.exit(0);
283+
process.exit(kNoFailure);
282284
} else {
283285
this.state = 'destroying';
284286
send({ act: 'exitedAfterDisconnect' }, () => process.disconnect());
285-
process.once('disconnect', () => process.exit(0));
287+
process.once('disconnect', () => process.exit(kNoFailure));
286288
}
287289
};

lib/internal/debugger/inspect.js

+12-5
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,12 @@ const { 0: InspectClient, 1: createRepl } =
4444
const debuglog = util.debuglog('inspect');
4545

4646
const { ERR_DEBUGGER_STARTUP_ERROR } = require('internal/errors').codes;
47+
const {
48+
exitCodes: {
49+
kGenericUserError,
50+
kNoFailure,
51+
},
52+
} = internalBinding('errors');
4753

4854
async function portIsFree(host, port, timeout = 3000) {
4955
if (port === 0) return; // Binding to a random port.
@@ -167,7 +173,7 @@ class NodeInspector {
167173

168174
// Handle all possible exits
169175
process.on('exit', () => this.killChild());
170-
const exitCodeZero = () => process.exit(0);
176+
const exitCodeZero = () => process.exit(kNoFailure);
171177
process.once('SIGTERM', exitCodeZero);
172178
process.once('SIGHUP', exitCodeZero);
173179

@@ -234,7 +240,7 @@ class NodeInspector {
234240
}
235241
}
236242
this.stdout.write(' failed to connect, please retry\n');
237-
process.exit(1);
243+
process.exit(kGenericUserError);
238244
}
239245

240246
clearLine() {
@@ -314,7 +320,7 @@ function parseArgv(args) {
314320
} catch (e) {
315321
if (e.code === 'ESRCH') {
316322
console.error(`Target process: ${pid} doesn't exist.`);
317-
process.exit(1);
323+
process.exit(kGenericUserError);
318324
}
319325
throw e;
320326
}
@@ -337,7 +343,8 @@ function startInspect(argv = ArrayPrototypeSlice(process.argv, 2),
337343
console.error(` ${invokedAs} <host>:<port>`);
338344
console.error(` ${invokedAs} --port=<port>`);
339345
console.error(` ${invokedAs} -p <pid>`);
340-
process.exit(1);
346+
// TODO(joyeecheung): should be kInvalidCommandLineArgument.
347+
process.exit(kGenericUserError);
341348
}
342349

343350
const options = parseArgv(argv);
@@ -355,7 +362,7 @@ function startInspect(argv = ArrayPrototypeSlice(process.argv, 2),
355362
console.error(e.message);
356363
}
357364
if (inspector.child) inspector.child.kill();
358-
process.exit(1);
365+
process.exit(kGenericUserError);
359366
}
360367

361368
process.on('uncaughtException', handleUnexpectedError);

lib/internal/main/repl.js

+4-1
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ const console = require('internal/console/global');
1717

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

20+
const { exitCodes: { kGenericUserError } } = internalBinding('errors');
21+
2022
prepareMainThreadExecution();
2123

2224
markBootstrapComplete();
@@ -31,7 +33,8 @@ if (process.env.NODE_REPL_EXTERNAL_MODULE) {
3133
// If we can't write to stderr, we'd like to make this a noop,
3234
// so use console.error.
3335
console.error('Cannot specify --input-type for REPL');
34-
process.exit(1);
36+
// TODO(joyeecheung): should be kInvalidCommandLineArgument.
37+
process.exit(kGenericUserError);
3538
}
3639

3740
esmLoader.loadESM(() => {

lib/internal/main/test_runner.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ const {
55
} = require('internal/process/pre_execution');
66
const { isUsingInspector } = require('internal/util/inspector');
77
const { run } = require('internal/test_runner/runner');
8+
const { exitCodes: { kGenericUserError } } = internalBinding('errors');
89

910
prepareMainThreadExecution(false);
1011
markBootstrapComplete();
@@ -22,5 +23,5 @@ if (isUsingInspector()) {
2223
const tapStream = run({ concurrency, inspectPort });
2324
tapStream.pipe(process.stdout);
2425
tapStream.once('test:fail', () => {
25-
process.exitCode = 1;
26+
process.exitCode = kGenericUserError;
2627
});

lib/internal/main/watch_mode.js

+5-3
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,10 @@ const {
1212
prepareMainThreadExecution,
1313
markBootstrapComplete
1414
} = require('internal/process/pre_execution');
15-
const { triggerUncaughtException } = internalBinding('errors');
15+
const {
16+
triggerUncaughtException,
17+
exitCodes: { kNoFailure },
18+
} = internalBinding('errors');
1619
const { getOptionValue } = require('internal/options');
1720
const { emitExperimentalWarning } = require('internal/util');
1821
const { FilesWatcher } = require('internal/watch_mode/files_watcher');
@@ -24,7 +27,6 @@ const { setTimeout, clearTimeout } = require('timers');
2427
const { resolve } = require('path');
2528
const { once, on } = require('events');
2629

27-
2830
prepareMainThreadExecution(false, false);
2931
markBootstrapComplete();
3032

@@ -125,7 +127,7 @@ function signalHandler(signal) {
125127
return async () => {
126128
watcher.clear();
127129
const exitCode = await killAndWait(signal, true);
128-
process.exit(exitCode ?? 0);
130+
process.exit(exitCode ?? kNoFailure);
129131
};
130132
}
131133
process.on('SIGTERM', signalHandler('SIGTERM'));

lib/internal/main/worker_thread.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ let debug = require('internal/util/debuglog').debuglog('worker', (fn) => {
6666
});
6767

6868
const assert = require('internal/assert');
69+
const { exitCodes: { kGenericUserError } } = internalBinding('errors');
6970

7071
patchProcessObject();
7172
setupInspectorHooks();
@@ -234,7 +235,7 @@ function workerOnGlobalUncaughtException(error, fromPromise) {
234235
if (!process._exiting) {
235236
try {
236237
process._exiting = true;
237-
process.exitCode = 1;
238+
process.exitCode = kGenericUserError;
238239
if (!handlerThrew) {
239240
process.emit('exit', process.exitCode);
240241
}

lib/internal/modules/esm/handle_process_exit.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
'use strict';
22

3+
const { exitCodes: { kUnfinishedTopLevelAwait } } = internalBinding('errors');
4+
35
// Handle a Promise from running code that potentially does Top-Level Await.
46
// In that case, it makes sense to set the exit code to a specific non-zero
57
// value if the main code never finishes running.
68
function handleProcessExit() {
7-
process.exitCode ??= 13;
9+
process.exitCode ??= kUnfinishedTopLevelAwait;
810
}
911

1012
module.exports = {

lib/internal/policy/manifest.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ const { getOptionValue } = require('internal/options');
4040
const shouldAbortOnUncaughtException = getOptionValue(
4141
'--abort-on-uncaught-exception'
4242
);
43+
const { exitCodes: { kGenericUserError } } = internalBinding('errors');
44+
4345
const { abort, exit, _rawDebug } = process;
4446
// #endregion
4547

@@ -72,7 +74,7 @@ function REACTION_EXIT(error) {
7274
if (shouldAbortOnUncaughtException) {
7375
abort();
7476
}
75-
exit(1);
77+
exit(kGenericUserError);
7678
}
7779

7880
function REACTION_LOG(error) {

lib/internal/process/execution.js

+3-2
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ const {
1414
ERR_EVAL_ESM_CANNOT_PRINT,
1515
},
1616
} = require('internal/errors');
17+
const { exitCodes: { kGenericUserError } } = internalBinding('errors');
1718

1819
const {
1920
executionAsyncId,
@@ -161,8 +162,8 @@ function createOnGlobalUncaughtException() {
161162
try {
162163
if (!process._exiting) {
163164
process._exiting = true;
164-
process.exitCode = 1;
165-
process.emit('exit', 1);
165+
process.exitCode = kGenericUserError;
166+
process.emit('exit', kGenericUserError);
166167
}
167168
} catch {
168169
// Nothing to be done about it at this point.

lib/internal/process/per_thread.js

+3-2
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ const kInternal = Symbol('internal properties');
5858
function assert(x, msg) {
5959
if (!x) throw new ERR_ASSERTION(msg || 'assertion error');
6060
}
61+
const { exitCodes: { kNoFailure } } = internalBinding('errors');
6162

6263
const binding = internalBinding('process_methods');
6364

@@ -187,12 +188,12 @@ function wrapProcessMethods(binding) {
187188

188189
if (!process._exiting) {
189190
process._exiting = true;
190-
process.emit('exit', process.exitCode || 0);
191+
process.emit('exit', process.exitCode || kNoFailure);
191192
}
192193
// FIXME(joyeecheung): This is an undocumented API that gets monkey-patched
193194
// in the user land. Either document it, or deprecate it in favor of a
194195
// better public alternative.
195-
process.reallyExit(process.exitCode || 0);
196+
process.reallyExit(process.exitCode || kNoFailure);
196197
}
197198

198199
function kill(pid, sig) {

lib/internal/process/promises.js

+3-2
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@ const { deprecate } = require('internal/util');
2424

2525
const {
2626
noSideEffectsToString,
27-
triggerUncaughtException
27+
triggerUncaughtException,
28+
exitCodes: { kGenericUserError },
2829
} = internalBinding('errors');
2930

3031
const {
@@ -294,7 +295,7 @@ function processPromiseRejections() {
294295
const handled = emit(reason, promise, promiseInfo);
295296
if (!handled) {
296297
emitUnhandledRejectionWarning(uid, reason);
297-
process.exitCode = 1;
298+
process.exitCode = kGenericUserError;
298299
}
299300
break;
300301
}

lib/internal/test_runner/harness.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ const {
1313
ERR_TEST_FAILURE,
1414
},
1515
} = require('internal/errors');
16+
const { exitCodes: { kGenericUserError } } = internalBinding('errors');
17+
1618
const { kEmptyObject } = require('internal/util');
1719
const { getOptionValue } = require('internal/options');
1820
const { kCancelledByParent, Test, ItTest, Suite } = require('internal/test_runner/test');
@@ -118,7 +120,7 @@ function getGlobalRoot() {
118120
globalRoot = createTestTree();
119121
globalRoot.reporter.pipe(process.stdout);
120122
globalRoot.reporter.once('test:fail', () => {
121-
process.exitCode = 1;
123+
process.exitCode = kGenericUserError;
122124
});
123125
}
124126
return globalRoot;

lib/internal/test_runner/runner.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ const {
3434
} = require('internal/test_runner/utils');
3535
const { basename, join, resolve } = require('path');
3636
const { once } = require('events');
37+
const { exitCodes: { kGenericUserError } } = internalBinding('errors');
3738

3839
const kFilterArgs = ['--test'];
3940

@@ -90,7 +91,7 @@ function createTestFileList() {
9091
} catch (err) {
9192
if (err?.code === 'ENOENT') {
9293
console.error(`Could not find '${err.path}'`);
93-
process.exit(1);
94+
process.exit(kGenericUserError);
9495
}
9596

9697
throw err;

node.gyp

+1
Original file line numberDiff line numberDiff line change
@@ -600,6 +600,7 @@
600600
'src/node_contextify.h',
601601
'src/node_dir.h',
602602
'src/node_errors.h',
603+
'src/node_exit_code.h',
603604
'src/node_external_reference.h',
604605
'src/node_file.h',
605606
'src/node_file-inl.h',

src/api/embed_helpers.cc

+12-4
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ using v8::Function;
77
using v8::Global;
88
using v8::HandleScope;
99
using v8::Isolate;
10+
using v8::Just;
1011
using v8::Local;
1112
using v8::Locker;
1213
using v8::Maybe;
@@ -15,7 +16,7 @@ using v8::SealHandleScope;
1516

1617
namespace node {
1718

18-
Maybe<int> SpinEventLoop(Environment* env) {
19+
Maybe<ExitCode> SpinEventLoopInternal(Environment* env) {
1920
CHECK_NOT_NULL(env);
2021
MultiIsolatePlatform* platform = GetMultiIsolatePlatform(env);
2122
CHECK_NOT_NULL(platform);
@@ -25,7 +26,7 @@ Maybe<int> SpinEventLoop(Environment* env) {
2526
Context::Scope context_scope(env->context());
2627
SealHandleScope seal(isolate);
2728

28-
if (env->is_stopping()) return Nothing<int>();
29+
if (env->is_stopping()) return Nothing<ExitCode>();
2930

3031
env->set_trace_sync_io(env->options()->trace_sync_io);
3132
{
@@ -59,7 +60,7 @@ Maybe<int> SpinEventLoop(Environment* env) {
5960
env->performance_state()->Mark(
6061
node::performance::NODE_PERFORMANCE_MILESTONE_LOOP_EXIT);
6162
}
62-
if (env->is_stopping()) return Nothing<int>();
63+
if (env->is_stopping()) return Nothing<ExitCode>();
6364

6465
env->set_trace_sync_io(false);
6566
// Clear the serialize callback even though the JS-land queue should
@@ -69,7 +70,7 @@ Maybe<int> SpinEventLoop(Environment* env) {
6970

7071
env->PrintInfoForSnapshotIfDebug();
7172
env->ForEachRealm([](Realm* realm) { realm->VerifyNoStrongBaseObjects(); });
72-
return EmitProcessExit(env);
73+
return EmitProcessExitInternal(env);
7374
}
7475

7576
struct CommonEnvironmentSetup::Impl {
@@ -155,6 +156,13 @@ CommonEnvironmentSetup::~CommonEnvironmentSetup() {
155156
delete impl_;
156157
}
157158

159+
Maybe<int> SpinEventLoop(Environment* env) {
160+
Maybe<ExitCode> result = SpinEventLoopInternal(env);
161+
if (result.IsNothing()) {
162+
return Nothing<int>();
163+
}
164+
return Just(static_cast<int>(result.FromJust()));
165+
}
158166

159167
uv_loop_t* CommonEnvironmentSetup::event_loop() const {
160168
return &impl_->loop;

0 commit comments

Comments
 (0)