process: refactor the bootstrap mode branching for readability

This patch refactors the branches for choosing the mode to run
Node.js in `internal/bootstrap/node.js`. Instead of inlining the
decision making all in `startup`, we create a `startExecution()`
function which either detects and start the non-user-code mode,
or prepares for user code execution (worker setup, preloading modules)
and starts user code execution.
We use early returns when we decide the mode to run Node.js in for fewer
indentations and better readability.

This patch also adds a few comments about the command-line switches
and a few TODOs to remove underscore properties on `process` that
are mainly used for bootstrap mode branching. It also includes
a few other refactoring such as inlining functions/variables
that are not reused and removing the default argument of
`evalScript` for better clarity.

PR-URL: https://github.com/nodejs/node/pull/24673
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

lib/internal/bootstrap/node.js

@@ -104,18 +104,13 @@
     }
 
     const { getOptionValue } = NativeModule.require('internal/options');
-    const helpOption = getOptionValue('--help');
+
-    const completionBashOption = getOptionValue('--completion-bash');
+    if (getOptionValue('--help')) {
-    const experimentalModulesOption = getOptionValue('--experimental-modules');
-    const experimentalVMModulesOption =
-      getOptionValue('--experimental-vm-modules');
-    const experimentalWorkerOption = getOptionValue('--experimental-worker');
-    if (helpOption) {
       NativeModule.require('internal/print_help').print(process.stdout);
       return;
     }
 
-    if (completionBashOption) {
+    if (getOptionValue('--completion-bash')) {
       NativeModule.require('internal/bash_completion').print(process.stdout);
       return;
     }
@@ -135,7 +130,7 @@
       setupQueueMicrotask();
     }
 
-    if (experimentalWorkerOption) {
+    if (getOptionValue('--experimental-worker')) {
       setupDOMException();
     }
 
@@ -167,8 +162,10 @@
         'DeprecationWarning', 'DEP0062', startup, true);
     }
 
-    if (experimentalModulesOption || experimentalVMModulesOption) {
+    const experimentalModules = getOptionValue('--experimental-modules');
-      if (experimentalModulesOption) {
+    const experimentalVMModules = getOptionValue('--experimental-vm-modules');
+    if (experimentalModules || experimentalVMModules) {
+      if (experimentalModules) {
         process.emitWarning(
           'The ESM module loader is experimental.',
           'ExperimentalWarning', undefined);
@@ -228,22 +225,33 @@
 
     setupAllowedFlags();
 
+    startExecution();
+  }
+
   // There are various modes that Node can run in. The most common two
   // are running from a script and running the REPL - but there are a few
   // others like the debugger or running --eval arguments. Here we decide
   // which mode we run in.
-    if (internalBinding('worker').getEnvMessagePort() !== undefined) {
+  function startExecution() {
     // This means we are in a Worker context, and any script execution
     // will be directed by the worker module.
+    if (internalBinding('worker').getEnvMessagePort() !== undefined) {
       NativeModule.require('internal/worker').setupChild(evalScript);
-    } else if (NativeModule.exists('_third_party_main')) {
+      return;
+    }
+
     // To allow people to extend Node in different ways, this hook allows
     // one to drop a file lib/_third_party_main.js into the build
     // directory which will be executed instead of Node's normal loading.
+    if (NativeModule.exists('_third_party_main')) {
       process.nextTick(() => {
         NativeModule.require('_third_party_main');
       });
-    } else if (process.argv[1] === 'inspect' || process.argv[1] === 'debug') {
+      return;
+    }
+
+    // `node inspect ...` or `node debug ...`
+    if (process.argv[1] === 'inspect' || process.argv[1] === 'debug') {
       if (process.argv[1] === 'debug') {
         process.emitWarning(
           '`node debug` is deprecated. Please use `node inspect` instead.',
@@ -254,12 +262,22 @@
       process.nextTick(() => {
         NativeModule.require('internal/deps/node-inspect/lib/_inspect').start();
       });
+      return;
+    }
 
-    } else if (process.profProcess) {
+    // `node --prof-process`
+    // TODO(joyeecheung): use internal/options instead of process.profProcess
+    if (process.profProcess) {
       NativeModule.require('internal/v8_prof_processor');
-    } else {
+      return;
-      // There is user code to be run.
+    }
 
+    // There is user code to be run.
+    prepareUserCodeExecution();
+    executeUserCode();
+  }
+
+  function prepareUserCodeExecution() {
     // If this is a worker in cluster mode, start up the communication
     // channel. This needs to be done before any user code gets executed
     // (including preload modules).
@@ -270,25 +288,45 @@
       delete process.env.NODE_UNIQUE_ID;
     }
 
-      if (process._eval != null && !process._forceRepl) {
+    // For user code, we preload modules if `-r` is passed
-        // User passed '-e' or '--eval' arguments to Node without '-i' or
+    // TODO(joyeecheung): use internal/options instead of
-        // '--interactive'.
+    // process._preload_modules
-        preloadModules();
+    if (process._preload_modules) {
+      const {
+        _preloadModules
+      } = NativeModule.require('internal/modules/cjs/loader');
+      _preloadModules(process._preload_modules);
+    }
+  }
 
+  function executeUserCode() {
+    // User passed `-e` or `--eval` arguments to Node without `-i` or
+    // `--interactive`.
+    // Note that the name `forceRepl` is merely an alias of `interactive`
+    // in code.
+    // TODO(joyeecheung): use internal/options instead of
+    // process._eval/process._forceRepl
+    if (process._eval != null && !process._forceRepl) {
       const {
         addBuiltinLibsToObject
       } = NativeModule.require('internal/modules/cjs/helpers');
       addBuiltinLibsToObject(global);
-        evalScript('[eval]');
+      evalScript('[eval]', wrapForBreakOnFirstLine(process._eval));
-      } else if (process.argv[1] && process.argv[1] !== '-') {
+      return;
-        // Make process.argv[1] into a full path.
+    }
+
+    // If the first argument is a file name, run it as a main script
+    if (process.argv[1] && process.argv[1] !== '-') {
+      // Expand process.argv[1] into a full path.
       const path = NativeModule.require('path');
       process.argv[1] = path.resolve(process.argv[1]);
 
       const CJSModule = NativeModule.require('internal/modules/cjs/loader');
 
-        preloadModules();
+      // If user passed `-c` or `--check` arguments to Node, check its syntax
-        // Check if user passed `-c` or `--check` arguments to Node.
+      // instead of actually running the file.
+      // TODO(joyeecheung): use internal/options instead of
+      // process._syntax_check_only
       if (process._syntax_check_only != null) {
         const fs = NativeModule.require('fs');
         // Read the source.
@@ -297,12 +335,20 @@
         checkScriptSyntax(source, filename);
         process.exit(0);
       }
+
+      // Note: this actually tries to run the module as a ESM first if
+      // --experimental-modules is on.
+      // TODO(joyeecheung): can we move that logic to here? Note that this
+      // is an undocumented method available via `require('module').runMain`
       CJSModule.runMain();
-      } else {
+      return;
-        preloadModules();
+    }
-        // If -i or --interactive were passed, or stdin is a TTY.
+
+    // Create the REPL if `-i` or `--interactive` is passed, or if
+    // stdin is a TTY.
+    // Note that the name `forceRepl` is merely an alias of `interactive`
+    // in code.
     if (process._forceRepl || NativeModule.require('tty').isatty(0)) {
-          // REPL
       const cliRepl = NativeModule.require('internal/repl');
       cliRepl.createInternalRepl(process.env, (err, repl) => {
         if (err) {
@@ -319,12 +365,18 @@
         });
       });
 
+      // User passed '-e' or '--eval' along with `-i` or `--interactive`
       if (process._eval != null) {
-            // User passed '-e' or '--eval'
+        evalScript('[eval]', wrapForBreakOnFirstLine(process._eval));
-            evalScript('[eval]');
       }
-        } else {
+      return;
-          // Read all of stdin - execute it.
+    }
+
+    // Stdin is not a TTY, we will read it and execute it.
+    readAndExecuteStdin();
+  }
+
+  function readAndExecuteStdin() {
     process.stdin.setEncoding('utf8');
 
     let code = '';
@@ -332,18 +384,15 @@
       code += d;
     });
 
-          process.stdin.on('end', function() {
+    process.stdin.on('end', () => {
       if (process._syntax_check_only != null) {
         checkScriptSyntax(code, '[stdin]');
       } else {
         process._eval = code;
-              evalScript('[stdin]');
+        evalScript('[stdin]', wrapForBreakOnFirstLine(process._eval));
       }
     });
   }
-      }
-    }
-  }
 
   function setupTraceCategoryState() {
     const { traceCategoryState } = internalBinding('trace_events');
@@ -651,7 +700,7 @@
     return `process.binding('inspector').callAndPauseOnStart(${fn}, {})`;
   }
 
-  function evalScript(name, body = wrapForBreakOnFirstLine(process._eval)) {
+  function evalScript(name, body) {
     const CJSModule = NativeModule.require('internal/modules/cjs/loader');
     const path = NativeModule.require('path');
     const cwd = tryGetCwd(path);
@@ -673,16 +722,6 @@
     process._tickCallback();
   }
 
-  // Load preload modules.
-  function preloadModules() {
-    if (process._preload_modules) {
-      const {
-        _preloadModules
-      } = NativeModule.require('internal/modules/cjs/loader');
-      _preloadModules(process._preload_modules);
-    }
-  }
-
   function checkScriptSyntax(source, filename) {
     const CJSModule = NativeModule.require('internal/modules/cjs/loader');
     const vm = NativeModule.require('vm');

test/message/assert_throws_stack.out

@@ -18,3 +18,4 @@ AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
     at *
     at *
     at *
+    at *

test/message/core_line_numbers.out

@@ -12,4 +12,4 @@ RangeError: Invalid input
     at tryModuleLoad (internal/modules/cjs/loader.js:*:*)
     at Function.Module._load (internal/modules/cjs/loader.js:*:*)
     at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
-    at startup (internal/bootstrap/node.js:*:*)
+    at executeUserCode (internal/bootstrap/node.js:*:*)

test/message/error_exit.out

@@ -14,5 +14,6 @@ AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
     at tryModuleLoad (internal/modules/cjs/loader.js:*:*)
     at Function.Module._load (internal/modules/cjs/loader.js:*:*)
     at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
+    at executeUserCode (internal/bootstrap/node.js:*:*)
+    at startExecution (internal/bootstrap/node.js:*:*)
     at startup (internal/bootstrap/node.js:*:*)
-    at bootstrapNodeJSCore (internal/bootstrap/node.js:*:*)

test/message/eval_messages.out

@@ -9,6 +9,8 @@ SyntaxError: Strict mode code may not include a with statement
     at Object.<anonymous> ([eval]-wrapper:*:*)
     at Module._compile (internal/modules/cjs/loader.js:*:*)
     at evalScript (internal/bootstrap/node.js:*:*)
+    at executeUserCode (internal/bootstrap/node.js:*:*)
+    at startExecution (internal/bootstrap/node.js:*:*)
     at startup (internal/bootstrap/node.js:*:*)
     at bootstrapNodeJSCore (internal/bootstrap/node.js:*:*)
 42
@@ -24,6 +26,8 @@ Error: hello
     at Object.<anonymous> ([eval]-wrapper:*:*)
     at Module._compile (internal/modules/cjs/loader.js:*:*)
     at evalScript (internal/bootstrap/node.js:*:*)
+    at executeUserCode (internal/bootstrap/node.js:*:*)
+    at startExecution (internal/bootstrap/node.js:*:*)
     at startup (internal/bootstrap/node.js:*:*)
     at bootstrapNodeJSCore (internal/bootstrap/node.js:*:*)
 
@@ -38,6 +42,8 @@ Error: hello
     at Object.<anonymous> ([eval]-wrapper:*:*)
     at Module._compile (internal/modules/cjs/loader.js:*:*)
     at evalScript (internal/bootstrap/node.js:*:*)
+    at executeUserCode (internal/bootstrap/node.js:*:*)
+    at startExecution (internal/bootstrap/node.js:*:*)
     at startup (internal/bootstrap/node.js:*:*)
     at bootstrapNodeJSCore (internal/bootstrap/node.js:*:*)
 100
@@ -52,6 +58,8 @@ ReferenceError: y is not defined
     at Object.<anonymous> ([eval]-wrapper:*:*)
     at Module._compile (internal/modules/cjs/loader.js:*:*)
     at evalScript (internal/bootstrap/node.js:*:*)
+    at executeUserCode (internal/bootstrap/node.js:*:*)
+    at startExecution (internal/bootstrap/node.js:*:*)
     at startup (internal/bootstrap/node.js:*:*)
     at bootstrapNodeJSCore (internal/bootstrap/node.js:*:*)
 

test/message/events_unhandled_error_common_trace.out

@@ -12,11 +12,11 @@ Error: foo:bar
     at tryModuleLoad (internal/modules/cjs/loader.js:*:*)
     at Function.Module._load (internal/modules/cjs/loader.js:*:*)
     at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
-    at startup (internal/bootstrap/node.js:*:*)
+    at executeUserCode (internal/bootstrap/node.js:*:*)
 Emitted 'error' event at:
     at quux (*events_unhandled_error_common_trace.js:*:*)
     at Object.<anonymous> (*events_unhandled_error_common_trace.js:*:*)
     at Module._compile (internal/modules/cjs/loader.js:*:*)
     [... lines matching original stack trace ...]
-    at startup (internal/bootstrap/node.js:*:*)
+    at executeUserCode (internal/bootstrap/node.js:*:*)
-    at bootstrapNodeJSCore (internal/bootstrap/node.js:*:*)
+    at startExecution (internal/bootstrap/node.js:*:*)

test/message/events_unhandled_error_nexttick.out

@@ -10,12 +10,15 @@ Error
     at tryModuleLoad (internal/modules/cjs/loader.js:*:*)
     at Function.Module._load (internal/modules/cjs/loader.js:*:*)
     at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
+    at executeUserCode (internal/bootstrap/node.js:*:*)
+    at startExecution (internal/bootstrap/node.js:*:*)
     at startup (internal/bootstrap/node.js:*:*)
-    at bootstrapNodeJSCore (internal/bootstrap/node.js:*:*)
 Emitted 'error' event at:
     at process.nextTick (*events_unhandled_error_nexttick.js:*:*)
     at internalTickCallback (internal/process/next_tick.js:*:*)
     at process._tickCallback (internal/process/next_tick.js:*:*)
     at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
+    at executeUserCode (internal/bootstrap/node.js:*:*)
+    at startExecution (internal/bootstrap/node.js:*:*)
     at startup (internal/bootstrap/node.js:*:*)
     at bootstrapNodeJSCore (internal/bootstrap/node.js:*:*)

test/message/events_unhandled_error_sameline.out

@@ -10,10 +10,11 @@ Error
     at tryModuleLoad (internal/modules/cjs/loader.js:*:*)
     at Function.Module._load (internal/modules/cjs/loader.js:*:*)
     at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
+    at executeUserCode (internal/bootstrap/node.js:*:*)
+    at startExecution (internal/bootstrap/node.js:*:*)
     at startup (internal/bootstrap/node.js:*:*)
-    at bootstrapNodeJSCore (internal/bootstrap/node.js:*:*)
 Emitted 'error' event at:
     at Object.<anonymous> (*events_unhandled_error_sameline.js:*:*)
     at Module._compile (internal/modules/cjs/loader.js:*:*)
     [... lines matching original stack trace ...]
-    at bootstrapNodeJSCore (internal/bootstrap/node.js:*:*)
+    at startup (internal/bootstrap/node.js:*:*)

test/message/nexttick_throw.out

@@ -7,5 +7,7 @@ ReferenceError: undefined_reference_error_maker is not defined
     at internalTickCallback (internal/process/next_tick.js:*:*)
     at process._tickCallback (internal/process/next_tick.js:*:*)
     at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
+    at executeUserCode (internal/bootstrap/node.js:*:*)
+    at startExecution (internal/bootstrap/node.js:*:*)
     at startup (internal/bootstrap/node.js:*:*)
     at bootstrapNodeJSCore (internal/bootstrap/node.js:*:*)

test/message/stdin_messages.out

@@ -9,7 +9,7 @@ SyntaxError: Strict mode code may not include a with statement
     at Object.<anonymous> ([stdin]-wrapper:*:*)
     at Module._compile (internal/modules/cjs/loader.js:*:*)
     at evalScript (internal/bootstrap/node.js:*:*)
-    at Socket.<anonymous> (internal/bootstrap/node.js:*:*)
+    at Socket.process.stdin.on (internal/bootstrap/node.js:*:*)
     at Socket.emit (events.js:*:*)
     at endReadableNT (_stream_readable.js:*:*)
     at process.internalTickCallback (internal/process/next_tick.js:*:*)
@@ -26,7 +26,7 @@ Error: hello
     at Object.<anonymous> ([stdin]-wrapper:*:*)
     at Module._compile (internal/modules/cjs/loader.js:*:*)
     at evalScript (internal/bootstrap/node.js:*:*)
-    at Socket.<anonymous> (internal/bootstrap/node.js:*:*)
+    at Socket.process.stdin.on (internal/bootstrap/node.js:*:*)
     at Socket.emit (events.js:*:*)
     at endReadableNT (_stream_readable.js:*:*)
     at process.internalTickCallback (internal/process/next_tick.js:*:*)
@@ -41,7 +41,7 @@ Error: hello
     at Object.<anonymous> ([stdin]-wrapper:*:*)
     at Module._compile (internal/modules/cjs/loader.js:*:*)
     at evalScript (internal/bootstrap/node.js:*:*)
-    at Socket.<anonymous> (internal/bootstrap/node.js:*:*)
+    at Socket.process.stdin.on (internal/bootstrap/node.js:*:*)
     at Socket.emit (events.js:*:*)
     at endReadableNT (_stream_readable.js:*:*)
     at process.internalTickCallback (internal/process/next_tick.js:*:*)
@@ -57,7 +57,7 @@ ReferenceError: y is not defined
     at Object.<anonymous> ([stdin]-wrapper:*:*)
     at Module._compile (internal/modules/cjs/loader.js:*:*)
     at evalScript (internal/bootstrap/node.js:*:*)
-    at Socket.<anonymous> (internal/bootstrap/node.js:*:*)
+    at Socket.process.stdin.on (internal/bootstrap/node.js:*:*)
     at Socket.emit (events.js:*:*)
     at endReadableNT (_stream_readable.js:*:*)
     at process.internalTickCallback (internal/process/next_tick.js:*:*)

test/message/unhandled_promise_trace_warnings.out

@@ -15,6 +15,9 @@
     at *
     at *
     at *
+    at *
+    at *
+    at *
 (node:*) Error: This was rejected
     at * (*test*message*unhandled_promise_trace_warnings.js:*)
     at *
@@ -25,6 +28,7 @@
     at *
     at *
     at *
+    at *
 (node:*) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
     at *
     at *
@@ -34,6 +38,8 @@
     at *
     at *
     at *
+    at *
+    at *
 (node:*) PromiseRejectionHandledWarning: Promise rejection was handled asynchronously (rejection id: 1)
     at handledRejection (internal/process/promises.js:*)
     at handler (internal/process/promises.js:*)

test/message/util_inspect_error.out

@@ -10,6 +10,7 @@
        at *
        at *
        at *
+       at *
   nested:
    { err:
       Error: foo
@@ -22,6 +23,7 @@
           at *
           at *
           at *
+          at *
           at * } }
 {
   err: Error: foo
@@ -34,6 +36,7 @@
       at *
       at *
       at *
+      at *
       at *,
   nested: {
     err: Error: foo
@@ -47,6 +50,7 @@
         at *
         at *
         at *
+        at *
   }
 }
 { Error: foo
@@ -60,4 +64,5 @@ bar
     at *
     at *
     at *
+    at *
   foo: 'bar' }