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: added inspector.waitForDebugger() #28453

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
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
5 changes: 5 additions & 0 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -1187,6 +1187,11 @@ after the session had already closed.

An error occurred while issuing a command via the `inspector` module.

<a id="ERR_INSPECTOR_NOT_ACTIVE"></a>
### ERR_INSPECTOR_NOT_ACTIVE

The `inspector` is not active when `inspector.waitForDebugger()` is called.

<a id="ERR_INSPECTOR_NOT_AVAILABLE"></a>
### ERR_INSPECTOR_NOT_AVAILABLE

Expand Down
10 changes: 10 additions & 0 deletions doc/api/inspector.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,16 @@ parameter usage.

Return the URL of the active inspector, or `undefined` if there is none.

## inspector.waitForDebugger()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should This include a timeout to prevent blocking forever?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding timeout for this (as well as inspector.open) is definitely something we should do. I think it should be a separate PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds reasonable to me. I prefer to do this work in separate PR if you do not mind.

alexkozy marked this conversation as resolved.
Show resolved Hide resolved
<!-- YAML
added: REPLACEME
-->

Blocks until a client (existing or connected later) has sent
`Runtime.runIfWaitingForDebugger` command.

An exception will be thrown if there is no active inspector.

## Class: inspector.Session

The `inspector.Session` is used for dispatching messages to the V8 inspector
Expand Down
22 changes: 20 additions & 2 deletions lib/inspector.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const {
ERR_INSPECTOR_COMMAND,
ERR_INSPECTOR_NOT_AVAILABLE,
ERR_INSPECTOR_NOT_CONNECTED,
ERR_INSPECTOR_NOT_ACTIVE,
ERR_INVALID_ARG_TYPE,
ERR_INVALID_CALLBACK
} = require('internal/errors').codes;
Expand All @@ -19,7 +20,12 @@ if (!hasInspector)
const EventEmitter = require('events');
const { validateString } = require('internal/validators');
const util = require('util');
const { Connection, open, url } = internalBinding('inspector');
const {
Connection,
open,
url,
waitForDebugger
} = internalBinding('inspector');

const connectionSymbol = Symbol('connectionProperty');
const messageCallbacksSymbol = Symbol('messageCallbacks');
Expand Down Expand Up @@ -105,10 +111,22 @@ class Session extends EventEmitter {
}
}

function inspectorOpen(port, host, wait) {
open(port, host);
if (wait)
waitForDebugger();
}

function inspectorWaitForDebugger() {
if (!waitForDebugger())
throw new ERR_INSPECTOR_NOT_ACTIVE();
}

module.exports = {
open: (port, host, wait) => open(port, host, !!wait),
open: inspectorOpen,
close: process._debugEnd,
url: url,
waitForDebugger: inspectorWaitForDebugger,
// This is dynamically added during bootstrap,
// where the console from the VM is still available
console: require('internal/util/inspector').consoleFromVM,
Expand Down
1 change: 1 addition & 0 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -841,6 +841,7 @@ E('ERR_INPUT_TYPE_NOT_ALLOWED', '--input-type can only be used with string ' +
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);
E('ERR_INSPECTOR_NOT_ACTIVE', 'Inspector is not active', Error);
E('ERR_INSPECTOR_NOT_AVAILABLE', 'Inspector is not available', Error);
E('ERR_INSPECTOR_NOT_CONNECTED', 'Session is not connected', Error);
E('ERR_INTERNAL_ASSERTION', (message) => {
Expand Down
13 changes: 8 additions & 5 deletions src/inspector_js_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,6 @@ void IsEnabled(const FunctionCallbackInfo<Value>& args) {
void Open(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
Agent* agent = env->inspector_agent();
bool wait_for_connect = false;
eugeneo marked this conversation as resolved.
Show resolved Hide resolved

if (args.Length() > 0 && args[0]->IsUint32()) {
uint32_t port = args[0].As<Uint32>()->Value();
Expand All @@ -249,12 +248,15 @@ void Open(const FunctionCallbackInfo<Value>& args) {
agent->host_port()->set_host(*host);
}

if (args.Length() > 2 && args[2]->IsBoolean()) {
wait_for_connect = args[2]->BooleanValue(args.GetIsolate());
}
agent->StartIoThread();
if (wait_for_connect)
}

void WaitForDebugger(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
Agent* agent = env->inspector_agent();
if (agent->IsActive())
agent->WaitForConnect();
args.GetReturnValue().Set(agent->IsActive());
}

void Url(const FunctionCallbackInfo<Value>& args) {
Expand Down Expand Up @@ -291,6 +293,7 @@ void Initialize(Local<Object> target, Local<Value> unused,
env->SetMethod(target, "callAndPauseOnStart", CallAndPauseOnStart);
env->SetMethod(target, "open", Open);
env->SetMethodNoSideEffect(target, "url", Url);
env->SetMethod(target, "waitForDebugger", WaitForDebugger);

env->SetMethod(target, "asyncTaskScheduled", AsyncTaskScheduledWrapper);
env->SetMethod(target, "asyncTaskCanceled",
Expand Down
63 changes: 63 additions & 0 deletions test/parallel/test-inspector-wait-for-connection.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// Flags: --expose-internals

'use strict';
const common = require('../common');

common.skipIfInspectorDisabled();

const assert = require('assert');
const { NodeInstance } = require('../common/inspector-helper.js');

async function runTests() {
const child = new NodeInstance(['-e', `(${main.toString()})()`], '', '');
const session = await child.connectInspectorSession();
await session.send({ method: 'Runtime.enable' });
// Check that there is only one console message received.
await session.waitForConsoleOutput('log', 'before wait for debugger');
assert.ok(!session.unprocessedNotifications()
.some((n) => n.method === 'Runtime.consoleAPICalled'));
// Check that inspector.url() is available between inspector.open() and
// inspector.waitForDebugger()
const { result: { value } } = await session.send({
method: 'Runtime.evaluate',
params: {
expression: 'process._ws',
includeCommandLineAPI: true
}
});
assert.ok(value.startsWith('ws://'));
session.send({ method: 'Runtime.runIfWaitingForDebugger' });
// Check that messages after first and before second waitForDebugger are
// received
await session.waitForConsoleOutput('log', 'after wait for debugger');
await session.waitForConsoleOutput('log', 'before second wait for debugger');
assert.ok(!session.unprocessedNotifications()
.some((n) => n.method === 'Runtime.consoleAPICalled'));
const secondSession = await child.connectInspectorSession();
// Check that inspector.waitForDebugger can be resumed from another session
secondSession.send({ method: 'Runtime.runIfWaitingForDebugger' });
await session.waitForConsoleOutput('log', 'after second wait for debugger');
assert.ok(!session.unprocessedNotifications()
.some((n) => n.method === 'Runtime.consoleAPICalled'));
secondSession.disconnect();
session.disconnect();

function main(prefix) {
const inspector = require('inspector');
inspector.open(0, undefined, false);
process._ws = inspector.url();
console.log('before wait for debugger');
inspector.waitForDebugger();
console.log('after wait for debugger');
console.log('before second wait for debugger');
inspector.waitForDebugger();
console.log('after second wait for debugger');
}

// Check that inspector.waitForDebugger throws if there is no active
// inspector
const re = /^Error \[ERR_INSPECTOR_NOT_ACTIVE\]: Inspector is not active$/;
assert.throws(() => require('inspector').waitForDebugger(), re);
}

runTests();