Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

inspector: throw error when activating an already active inspector #33015

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
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.
  • Loading branch information
joyeecheung committed Apr 23, 2020
commit 29123002964e56b525a7bc88a0ae3c898b83a99d
7 changes: 7 additions & 0 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -1210,6 +1210,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`

Expand Down
5 changes: 5 additions & 0 deletions lib/inspector.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const {
} = primordials;

const {
ERR_INSPECTOR_ALREADY_ACTIVATED,
ERR_INSPECTOR_ALREADY_CONNECTED,
ERR_INSPECTOR_CLOSED,
ERR_INSPECTOR_COMMAND,
Expand All @@ -33,6 +34,7 @@ const {
MainThreadConnection,
open,
url,
isEnabled,
waitForDebugger
} = internalBinding('inspector');

Expand Down Expand Up @@ -131,6 +133,9 @@ class Session extends EventEmitter {
}

function inspectorOpen(port, host, wait) {
if (isEnabled()) {
throw new ERR_INSPECTOR_ALREADY_ACTIVATED();
joyeecheung marked this conversation as resolved.
Show resolved Hide resolved
}
open(port, host);
if (wait)
waitForDebugger();
Expand Down
4 changes: 4 additions & 0 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -920,6 +920,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);
Expand Down
18 changes: 18 additions & 0 deletions test/sequential/test-inspector-already-activated-cli.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Flags: --inspect=0
'use strict';

const common = require('../common');
common.skipIfInspectorDisabled();

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);
22 changes: 18 additions & 4 deletions test/sequential/test-inspector-open.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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));
}));

Expand All @@ -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;
});
Expand Down Expand Up @@ -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));
}

Expand Down Expand Up @@ -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();
Expand Down