inspector: --debug* deprecation and invalidation

PR-URL: https://github.com/nodejs/node/pull/12949
Fixes: https://github.com/nodejs/node/issues/12364
Fixes: https://github.com/nodejs/node/issues/12630
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jan Krems <jan.krems@gmail.com>

3 files changed, 52 insertions, 30 deletions

diff --git a/src/node.cc b/src/node.cc
index 8972dd6135..f2688f2ecf 100644
--- a/src/node.cc
+++ b/src/node.cc
@@ -3391,9 +3391,29 @@ void SetupProcessObject(Environment* env,
     READONLY_PROPERTY(process, "traceDeprecation", True(env->isolate()));
   }
 
+  // TODO(refack): move the following 4 to `node_config`
+  // --inspect
+  if (debug_options.inspector_enabled()) {
+    READONLY_DONT_ENUM_PROPERTY(process,
+                                "_inspectorEnbale", True(env->isolate()));
+  }
+
   // --inspect-brk
   if (debug_options.wait_for_connect()) {
-    READONLY_PROPERTY(process, "_debugWaitConnect", True(env->isolate()));
+    READONLY_DONT_ENUM_PROPERTY(process,
+                                "_breakFirstLine", True(env->isolate()));
+  }
+
+  // --inspect --debug-brk
+  if (debug_options.deprecated_invocation()) {
+    READONLY_DONT_ENUM_PROPERTY(process,
+                                "_deprecatedDebugBrk", True(env->isolate()));
+  }
+
+  // --debug or, --debug-brk without --inspect
+  if (debug_options.invalid_invocation()) {
+    READONLY_DONT_ENUM_PROPERTY(process,
+                                "_invalidDebug", True(env->isolate()));
   }
 
   // --security-revert flags
diff --git a/src/node_debug_options.cc b/src/node_debug_options.cc
index 2e8f601401..3f1a2e56e8 100644
--- a/src/node_debug_options.cc
+++ b/src/node_debug_options.cc
@@ -8,9 +8,7 @@
 namespace node {
 
 namespace {
-#if HAVE_INSPECTOR
 const int default_inspector_port = 9229;
-#endif  // HAVE_INSPECTOR
 
 inline std::string remove_brackets(const std::string& host) {
   if (!host.empty() && host.front() == '[' && host.back() == ']')
@@ -56,14 +54,12 @@ std::pair<std::string, int> split_host_port(const std::string& arg) {
 }  // namespace
 
 DebugOptions::DebugOptions() :
-#if HAVE_INSPECTOR
                                inspector_enabled_(false),
-#endif  // HAVE_INSPECTOR
-                               wait_connect_(false), http_enabled_(false),
+                               deprecated_debug_(false),
+                               break_first_line_(false),
                                host_name_("127.0.0.1"), port_(-1) { }
 
 bool DebugOptions::ParseOption(const char* argv0, const std::string& option) {
-  bool enable_inspector = false;
   bool has_argument = false;
   std::string option_name;
   std::string argument;
@@ -81,11 +77,21 @@ bool DebugOptions::ParseOption(const char* argv0, const std::string& option) {
       argument.clear();
   }
 
+  // Note that --debug-port and --debug-brk in conjuction with --inspect
+  // work but are undocumented.
+  // --debug is no longer valid.
+  // Ref: https://github.com/nodejs/node/issues/12630
+  // Ref: https://github.com/nodejs/node/pull/12949
   if (option_name == "--inspect") {
-    enable_inspector = true;
+    inspector_enabled_ = true;
+  } else if (option_name == "--debug") {
+    deprecated_debug_ = true;
   } else if (option_name == "--inspect-brk") {
-    enable_inspector = true;
-    wait_connect_ = true;
+    inspector_enabled_ = true;
+    break_first_line_ = true;
+  } else if (option_name == "--debug-brk") {
+    break_first_line_ = true;
+    deprecated_debug_ = true;
   } else if (option_name == "--debug-port" ||
              option_name == "--inspect-port") {
     if (!has_argument) {
@@ -97,15 +103,14 @@ bool DebugOptions::ParseOption(const char* argv0, const std::string& option) {
     return false;
   }
 
-  if (enable_inspector) {
-#if HAVE_INSPECTOR
-    inspector_enabled_ = true;
-#else
+#if !HAVE_INSPECTOR
+  if (inspector_enabled_) {
     fprintf(stderr,
             "Inspector support is not available with this Node.js build\n");
-    return false;
-#endif
   }
+  inspector_enabled_ = false;
+  return false;
+#endif
 
   // argument can be specified for *any* option to specify host:port
   if (has_argument) {
@@ -124,9 +129,7 @@ bool DebugOptions::ParseOption(const char* argv0, const std::string& option) {
 int DebugOptions::port() const {
   int port = port_;
   if (port < 0) {
-#if HAVE_INSPECTOR
     port = default_inspector_port;
-#endif  // HAVE_INSPECTOR
   }
   return port;
 }
diff --git a/src/node_debug_options.h b/src/node_debug_options.h
index fb86060f43..6fdd30384f 100644
--- a/src/node_debug_options.h
+++ b/src/node_debug_options.h
@@ -10,25 +10,24 @@ class DebugOptions {
  public:
   DebugOptions();
   bool ParseOption(const char* argv0, const std::string& option);
-  bool inspector_enabled() const {
-#if HAVE_INSPECTOR
-    return inspector_enabled_;
-#else
-    return false;
-#endif  // HAVE_INSPECTOR
+  bool inspector_enabled() const { return inspector_enabled_; }
+  bool deprecated_invocation() const {
+    return deprecated_debug_ &&
+      inspector_enabled_ &&
+      break_first_line_;
   }
-  bool ToolsServerEnabled();
-  bool wait_for_connect() const { return wait_connect_; }
+  bool invalid_invocation() const {
+    return deprecated_debug_ && !inspector_enabled_;
+  }
+  bool wait_for_connect() const { return break_first_line_; }
   std::string host_name() const { return host_name_; }
   int port() const;
   void set_port(int port) { port_ = port; }
 
  private:
-#if HAVE_INSPECTOR
   bool inspector_enabled_;
-#endif  // HAVE_INSPECTOR
-  bool wait_connect_;
-  bool http_enabled_;
+  bool deprecated_debug_;
+  bool break_first_line_;
   std::string host_name_;
   int port_;
 };