inspector: throw error when activating an already active inspector

When the user tries to activate the inspector that is already active
on a different port and host, we previously just silently reset
the port and host stored in the Environment without actually doing
anything for that to be effective. After this patch, we throw
an error telling the user to close the active inspector before invoking
`inspector.open()` again.

PR-URL: #33015
Fixes: #33012
Reviewed-By: Yongsheng Zhang <[email protected]>
Reviewed-By: Eugene Ostroukhov <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>

doc/api/errors.md

@@ -1224,6 +1224,13 @@ time.
 The `--input-type` flag was used to attempt to execute a file. This flag can
 only be used with input via `--eval`, `--print` or `STDIN`.
 
+<a id="ERR_INSPECTOR_ALREADY_ACTIVATED"></a>
+### `ERR_INSPECTOR_ALREADY_ACTIVATED`
+
+While using the `inspector` module, an attempt was made to activate the
+inspector when it already started to listen on a port. Use `inspector.close()`
+before activating it on a different address.
+
 <a id="ERR_INSPECTOR_ALREADY_CONNECTED"></a>
 ### `ERR_INSPECTOR_ALREADY_CONNECTED`
 

lib/inspector.js

@@ -8,6 +8,7 @@ const {
 } = primordials;
 
 const {
+  ERR_INSPECTOR_ALREADY_ACTIVATED,
   ERR_INSPECTOR_ALREADY_CONNECTED,
   ERR_INSPECTOR_CLOSED,
   ERR_INSPECTOR_COMMAND,
@@ -33,6 +34,7 @@ const {
   MainThreadConnection,
   open,
   url,
+  isEnabled,
   waitForDebugger
 } = internalBinding('inspector');
 
@@ -131,6 +133,9 @@ class Session extends EventEmitter {
 }
 
 function inspectorOpen(port, host, wait) {
+  if (isEnabled()) {
+    throw new ERR_INSPECTOR_ALREADY_ACTIVATED();
+  }
   open(port, host);
   if (wait)
     waitForDebugger();

lib/internal/errors.js

@@ -941,6 +941,10 @@ E('ERR_INCOMPATIBLE_OPTION_PAIR',
   'Option "%s" cannot be used in combination with option "%s"', TypeError);
 E('ERR_INPUT_TYPE_NOT_ALLOWED', '--input-type can only be used with string ' +
   'input via --eval, --print, or STDIN', Error);
+E('ERR_INSPECTOR_ALREADY_ACTIVATED',
+  'Inspector is already activated. Close it with inspector.close() ' +
+  'before activating it again.',
+  Error);
 E('ERR_INSPECTOR_ALREADY_CONNECTED', '%s is already connected', Error);
 E('ERR_INSPECTOR_CLOSED', 'Session was closed', Error);
 E('ERR_INSPECTOR_COMMAND', 'Inspector error %d: %s', Error);

test/sequential/test-inspector-already-activated-cli.js

@@ -0,0 +1,19 @@
+// Flags: --inspect=0
+'use strict';
+
+const common = require('../common');
+common.skipIfInspectorDisabled();
+common.skipIfWorker();
+
+const assert = require('assert');
+const inspector = require('inspector');
+const wsUrl = inspector.url();
+assert(wsUrl.startsWith('ws://'));
+assert.throws(() => {
+  inspector.open(0, undefined, false);
+}, {
+  code: 'ERR_INSPECTOR_ALREADY_ACTIVATED'
+});
+assert.strictEqual(inspector.url(), wsUrl);
+inspector.close();
+assert.strictEqual(inspector.url(), undefined);

test/sequential/test-inspector-open.js

@@ -10,6 +10,10 @@ const fork = require('child_process').fork;
 const net = require('net');
 const url = require('url');
 
+const kFirstOpen = 0;
+const kOpenWhileOpen = 1;
+const kReOpen = 2;
+
 if (process.env.BE_CHILD)
   return beChild();
 
@@ -19,7 +23,7 @@ const child = fork(__filename,
 child.once('message', common.mustCall((msg) => {
   assert.strictEqual(msg.cmd, 'started');
 
-  child.send({ cmd: 'open', args: [0] });
+  child.send({ cmd: 'open', args: [kFirstOpen] });
   child.once('message', common.mustCall(firstOpen));
 }));
 
@@ -31,7 +35,7 @@ function firstOpen(msg) {
   ping(port, (err) => {
     assert.ifError(err);
     // Inspector is already open, and won't be reopened, so args don't matter.
-    child.send({ cmd: 'open', args: [] });
+    child.send({ cmd: 'open', args: [kOpenWhileOpen] });
     child.once('message', common.mustCall(tryToOpenWhenOpen));
     firstPort = port;
   });
@@ -62,7 +66,7 @@ function closeWhenOpen(msg) {
 function tryToCloseWhenClosed(msg) {
   assert.strictEqual(msg.cmd, 'url');
   assert.strictEqual(msg.url, undefined);
-  child.send({ cmd: 'open', args: [] });
+  child.send({ cmd: 'open', args: [kReOpen] });
   child.once('message', common.mustCall(reopenAfterClose));
 }
 
@@ -93,7 +97,17 @@ function beChild() {
 
   process.on('message', (msg) => {
     if (msg.cmd === 'open') {
-      inspector.open(...msg.args);
+      if (msg.args[0] === kFirstOpen) {
+        inspector.open(0, false, undefined);
+      } else if (msg.args[0] === kOpenWhileOpen) {
+        assert.throws(() => {
+          inspector.open(0, false, undefined);
+        }, {
+          code: 'ERR_INSPECTOR_ALREADY_ACTIVATED'
+        });
+      } else if (msg.args[0] === kReOpen) {
+        inspector.open(0, false, undefined);
+      }
     }
     if (msg.cmd === 'close') {
       inspector.close();