debugger: fix --debug-brk interaction with -e

The command line flag `--debug-brk` was ignored when the `-e` flag was
also present. This change allows the flags to both be honored when they
are used in a single command line.

PR-URL: #7089
Fixes: #3589
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>

Author: Trott

lib/internal/bootstrap_node.js

@@ -109,7 +109,9 @@
 
         const internalModule = NativeModule.require('internal/module');
         internalModule.addBuiltinLibsToObject(global);
-        evalScript('[eval]');
+        run(() => {
+          evalScript('[eval]');
+        });
       } else if (process.argv[1]) {
         // make process.argv[1] into a full path
         var path = NativeModule.require('path');
@@ -135,31 +137,7 @@
         }
 
         preloadModules();
-
-        if (process._debugWaitConnect &&
-            process.execArgv.some(function(arg) {
-              return arg.match(/^--debug-brk(=[0-9]*)?$/);
-            })) {
-
-          // XXX Fix this terrible hack!
-          //
-          // Give the client program a few ticks to connect.
-          // Otherwise, there's a race condition where `node debug foo.js`
-          // will not be able to connect in time to catch the first
-          // breakpoint message on line 1.
-          //
-          // A better fix would be to somehow get a message from the
-          // V8 debug object about a connection, and runMain when
-          // that occurs.  --isaacs
-
-          var debugTimeout = +process.env.NODE_DEBUG_TIMEOUT || 50;
-          setTimeout(Module.runMain, debugTimeout);
-
-        } else {
-          // Main entry point into most programs:
-          Module.runMain();
-        }
-
+        run(Module.runMain);
       } else {
         preloadModules();
         // If -i or --interactive were passed, or stdin is a TTY.
@@ -342,6 +320,35 @@
     }
   }
 
+  function isDebugBreak() {
+    return process.execArgv.some((arg) => {
+      return arg.match(/^--debug-brk(=[0-9]*)?$/);
+    });
+  }
+
+  function run(entryFunction) {
+    if (process._debugWaitConnect && isDebugBreak()) {
+
+      // XXX Fix this terrible hack!
+      //
+      // Give the client program a few ticks to connect.
+      // Otherwise, there's a race condition where `node debug foo.js`
+      // will not be able to connect in time to catch the first
+      // breakpoint message on line 1.
+      //
+      // A better fix would be to somehow get a message from the
+      // V8 debug object about a connection, and runMain when
+      // that occurs.  --isaacs
+
+      var debugTimeout = +process.env.NODE_DEBUG_TIMEOUT || 50;
+      setTimeout(entryFunction, debugTimeout);
+
+    } else {
+      // Main entry point into most programs:
+      entryFunction();
+    }
+  }
+
   // Below you find a minimal module system, which is used to load the node
   // core modules found in lib/*.js. All core modules are compiled into the
   // node binary, so they can be loaded faster.

test/message/core_line_numbers.out

@@ -11,5 +11,5 @@ RangeError: Invalid input
     at Module.load (module.js:*:*)
     at tryModuleLoad (module.js:*:*)
     at Function.Module._load (module.js:*:*)
-    at Function.Module.runMain (module.js:*:*)
-    at startup (node.js:*:*)
+    at Module.runMain (module.js:*:*)
+    at run (node.js:*:*)

test/message/error_exit.out

@@ -10,6 +10,7 @@ AssertionError: 1 == 2
     at Module.load (module.js:*:*)
     at tryModuleLoad (module.js:*:*)
     at Function.Module._load (module.js:*:*)
-    at Function.Module.runMain (module.js:*:*)
+    at Module.runMain (module.js:*:*)
+    at run (node.js:*:*)
     at startup (node.js:*:*)
     at node.js:*:*

test/message/nexttick_throw.out

@@ -6,6 +6,7 @@ ReferenceError: undefined_reference_error_maker is not defined
     at *test*message*nexttick_throw.js:*:*
     at _combinedTickCallback (internal/process/next_tick.js:*:*)
     at process._tickCallback (internal/process/next_tick.js:*:*)
-    at Function.Module.runMain (module.js:*:*)
+    at Module.runMain (module.js:*:*)
+    at run (node.js:*:*)
     at startup (node.js:*:*)
     at node.js:*:*

test/message/undefined_reference_in_new_context.out

@@ -13,4 +13,4 @@ ReferenceError: foo is not defined
     at Module.load (module.js:*)
     at tryModuleLoad (module.js:*:*)
     at Function.Module._load (module.js:*:*)
-    at Function.Module.runMain (module.js:*:*)
+    at Module.runMain (module.js:*:*)

test/message/vm_display_runtime_error.out

@@ -12,5 +12,5 @@ Error: boo!
     at Module.load (module.js:*)
     at tryModuleLoad (module.js:*:*)
     at Function.Module._load (module.js:*)
-    at Function.Module.runMain (module.js:*)
-    at startup (node.js:*)
+    at Module.runMain (module.js:*)
+    at run (node.js:*)

test/message/vm_display_syntax_error.out

@@ -11,9 +11,9 @@ SyntaxError: Unexpected number
     at Module.load (module.js:*)
     at tryModuleLoad (module.js:*:*)
     at Function.Module._load (module.js:*)
-    at Function.Module.runMain (module.js:*)
+    at Module.runMain (module.js:*)
+    at run (node.js:*)
     at startup (node.js:*)
-    at node.js:*
 test.vm:1
 var 5;
     ^
@@ -25,6 +25,6 @@ SyntaxError: Unexpected number
     at Module.load (module.js:*)
     at tryModuleLoad (module.js:*:*)
     at Function.Module._load (module.js:*)
-    at Function.Module.runMain (module.js:*)
+    at Module.runMain (module.js:*)
+    at run (node.js:*)
     at startup (node.js:*)
-    at node.js:*

test/message/vm_dont_display_runtime_error.out

@@ -12,5 +12,5 @@ Error: boo!
     at Module.load (module.js:*)
     at tryModuleLoad (module.js:*:*)
     at Function.Module._load (module.js:*)
-    at Function.Module.runMain (module.js:*)
-    at startup (node.js:*)
+    at Module.runMain (module.js:*)
+    at run (node.js:*)

test/message/vm_dont_display_syntax_error.out

@@ -11,6 +11,6 @@ SyntaxError: Unexpected number
     at Module.load (module.js:*)
     at tryModuleLoad (module.js:*:*)
     at Function.Module._load (module.js:*)
-    at Function.Module.runMain (module.js:*)
+    at Module.runMain (module.js:*)
+    at run (node.js:*)
     at startup (node.js:*)
-    at node.js:*

test/parallel/test-debug-brk.js

@@ -1,9 +1,35 @@
 'use strict';
 
 const common = require('../common');
-const assert = require('assert');
-const spawnSync = require('child_process').spawnSync;
+const spawn = require('child_process').spawn;
 
-const args = [`--debug-brk=${common.PORT}`, '-e', '0'];
-const proc = spawnSync(process.execPath, args, {encoding: 'utf8'});
-assert(/Debugger listening on/.test(proc.stderr));
+var procStderr = '';
+var agentStdout = '';
+var needToSpawnAgent = true;
+var needToExit = true;
+
+const procArgs = [`--debug-brk=${common.PORT}`, '-e', '0'];
+const proc = spawn(process.execPath, procArgs);
+proc.stderr.setEncoding('utf8');
+
+const exitAll = common.mustCall((processes) => {
+  processes.forEach((myProcess) => { myProcess.kill(); });
+});
+
+proc.stderr.on('data', (chunk) => {
+  procStderr += chunk;
+  if (/Debugger listening on/.test(procStderr) && needToSpawnAgent) {
+    needToSpawnAgent = false;
+    const agentArgs = ['debug', `localhost:${common.PORT}`];
+    const agent = spawn(process.execPath, agentArgs);
+    agent.stdout.setEncoding('utf8');
+
+    agent.stdout.on('data', (chunk) => {
+      agentStdout += chunk;
+      if (/connecting to .+ ok/.test(agentStdout) && needToExit) {
+        needToExit = false;
+        exitAll([proc, agent]);
+      }
+    });
+  }
+});