Skip to content

Commit

Permalink
Address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@TimothyGu
TimothyGu committed Aug 6, 2017
1 parent 812e351 commit 7a4d57b
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 76 deletions.
16 changes: 7 additions & 9 deletions lib/internal/bootstrap_node.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -312,21 +312,19 @@
}

function setupInspectorCommandLineAPI() {
const inspector = process.binding('inspector');
const addCommandLineAPIMethod = inspector.addCommandLineAPIMethod;
if (!addCommandLineAPIMethod) return;
const { addCommandLineAPI } = process.binding('inspector');
if (!addCommandLineAPI) return;

const Module = NativeModule.require('module');
const { makeRequireFunction } = NativeModule.require('internal/module');
const path = NativeModule.require('path');
const cwd = tryGetCwd(path);

const consoleAPIModule = new Module('[consoleAPI]');
consoleAPIModule.filename = path.join(cwd, consoleAPIModule.id);
consoleAPIModule.paths = Module._nodeModulePaths(cwd);
const consoleAPIModule = new Module('<inspector console>');
consoleAPIModule.paths =
Module._nodeModulePaths(cwd).concat(Module.globalPaths);

addCommandLineAPIMethod('require', function require(request) {
return consoleAPIModule.require(request);
});
addCommandLineAPI('require', makeRequireFunction(consoleAPIModule));
}

function setupProcessFatal() {
Expand Down
52 changes: 29 additions & 23 deletions src/inspector_agent.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
namespace node {
namespace inspector {
namespace {
using v8::Array;
using v8::Context;
using v8::External;
using v8::Function;
Expand Down Expand Up @@ -554,6 +555,20 @@ class NodeInspectorClient : public V8InspectorClient {
return env_->context();
}

void installAdditionalCommandLineAPI(Local<Context> context,
Local<Object> target) override {
Local<Object> console_api = env_->inspector_console_api_object();

Local<Array> properties =
console_api->GetOwnPropertyNames(context).ToLocalChecked();
for (uint32_t i = 0; i < properties->Length(); ++i) {
Local<Value> key = properties->Get(context, i).ToLocalChecked();
target->Set(context,
key,
console_api->Get(context, key).ToLocalChecked()).FromJust();
}
}

void FatalException(Local<Value> error, Local<v8::Message> message) {
Local<Context> context = env_->context();

Expand Down Expand Up @@ -587,20 +602,6 @@ class NodeInspectorClient : public V8InspectorClient {
return channel_.get();
}

void installAdditionalCommandLineAPI(v8::Local<v8::Context> context,
v8::Local<v8::Object> target) {
v8::Local<v8::Object> console_api = env_->inspector_console_api_object();

v8::Local<v8::Array> properties =
console_api->GetOwnPropertyNames(context).ToLocalChecked();
for (uint32_t i = 0; i < properties->Length(); ++i) {
v8::Local<v8::Value> key = properties->Get(context, i).ToLocalChecked();
target->Set(context,
key,
console_api->Get(context, key).ToLocalChecked()).FromJust();
}
}

void startRepeatingTimer(double interval_s,
TimerCallback callback,
void* data) override {
Expand Down Expand Up @@ -696,17 +697,17 @@ bool Agent::StartIoThread(bool wait_for_connect) {
return true;
}

static void AddCommandLineAPIMethod(
const v8::FunctionCallbackInfo<v8::Value>& info) {
static void AddCommandLineAPI(
const FunctionCallbackInfo<Value>& info) {
auto env = Environment::GetCurrent(info);
v8::Local<v8::Context> context = env->context();
Local<Context> context = env->context();

if (info.Length() != 2 || !info[0]->IsString() || !info[1]->IsFunction()) {
return env->ThrowTypeError("inspector.addCommandLineAPIMethod takes "
"exactly 2 arguments: a string and a function.");
if (info.Length() != 2 || !info[0]->IsString()) {
return env->ThrowTypeError("inspector.addCommandLineAPI takes "
"exactly 2 arguments: a string and a value.");
}

v8::Local<v8::Object> console_api = env->inspector_console_api_object();
Local<Object> console_api = env->inspector_console_api_object();
console_api->Set(context, info[0], info[1]).FromJust();
}

Expand Down Expand Up @@ -812,11 +813,16 @@ void Url(const FunctionCallbackInfo<Value>& args) {
void Agent::InitInspector(Local<Object> target, Local<Value> unused,
Local<Context> context, void* priv) {
Environment* env = Environment::GetCurrent(context);
env->set_inspector_console_api_object(Object::New(env->isolate()));
{
auto obj = Object::New(env->isolate());
auto null = Null(env->isolate());
CHECK(obj->SetPrototype(context, null).FromJust());
env->set_inspector_console_api_object(obj);
}

Agent* agent = env->inspector_agent();
env->SetMethod(target, "consoleCall", InspectorConsoleCall);
env->SetMethod(target, "addCommandLineAPIMethod", AddCommandLineAPIMethod);
env->SetMethod(target, "addCommandLineAPI", AddCommandLineAPI);
if (agent->debug_options_.wait_for_connect())
env->SetMethod(target, "callAndPauseOnStart", CallAndPauseOnStart);
env->SetMethod(target, "connect", ConnectJSBindingsSession);
Expand Down
113 changes: 69 additions & 44 deletions test/inspector/test-inspector.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ function checkBadPath(err, response) {
assert(/WebSockets request was expected/.test(err.response));
}

function checkException(message) {
assert.strictEqual(message['exceptionDetails'], undefined,
'An exception occurred during execution');
}

function expectMainScriptSource(result) {
const expected = helper.mainScriptSource();
const source = result['scriptSource'];
Expand Down Expand Up @@ -212,33 +217,36 @@ function testI18NCharacters(session) {
function testCommandLineAPI(session) {
const testModulePath = require.resolve('../fixtures/empty.js');
const testModuleStr = JSON.stringify(testModulePath);
const printModulePath = require.resolve('../fixtures/printA.js');
const printModuleStr = JSON.stringify(printModulePath);
const printAModulePath = require.resolve('../fixtures/printA.js');
const printAModuleStr = JSON.stringify(printAModulePath);
const printBModulePath = require.resolve('../fixtures/printB.js');
const printBModuleStr = JSON.stringify(printBModulePath);
session.sendInspectorCommands([
[ // we can use `require` outside of a callframe with require in scope
{
'method': 'Runtime.evaluate', 'params': {
'expression': 'typeof require("fs").readFile === "function"',
'includeCommandLineAPI': true
}
}, (message) => assert.strictEqual(true, message['result']['value'])
}, (message) => {
checkException(message);
assert.strictEqual(message['result']['value'], true);
}
],
[ // the global require does not have require.cache
[ // the global require has the same properties as a normal `require`
{
'method': 'Runtime.evaluate', 'params': {
'expression': 'require.cache === undefined',
'includeCommandLineAPI': true
}
}, (message) => assert.strictEqual(true, message['result']['value'])
],
[ // the `require` in the module shadows the command line API's `require`
{
'method': 'Debugger.evaluateOnCallFrame', 'params': {
'callFrameId': '{"ordinal":0,"injectedScriptId":1}',
'expression': 'typeof require.cache',
'expression': [
'typeof require.resolve === "function"',
'typeof require.extensions === "object"',
'typeof require.cache === "object"'
].join(' && '),
'includeCommandLineAPI': true
}
}, (message) => assert.strictEqual('object', message['result']['value'])
}, (message) => {
checkException(message);
assert.strictEqual(message['result']['value'], true);
}
],
[ // `require` twice returns the same value
{
Expand All @@ -252,74 +260,91 @@ function testCommandLineAPI(session) {
) === require(${testModuleStr})`,
'includeCommandLineAPI': true
}
}, (message) => assert.strictEqual(true, message['result']['value'])
}, (message) => {
checkException(message);
assert.strictEqual(message['result']['value'], true);
}
],
[ // after require the module appears in require.cache
{
'method': 'Debugger.evaluateOnCallFrame', 'params': {
'callFrameId': '{"ordinal":0,"injectedScriptId":1}',
'expression': `require.cache[${testModuleStr}].exports`,
'generatePreview': true,
'method': 'Runtime.evaluate', 'params': {
'expression': `JSON.stringify(
require.cache[${testModuleStr}].exports
)`,
'includeCommandLineAPI': true
}
}, (message) => {
const { properties } = message.result.preview;
assert.strictEqual('old', properties[0].name);
assert.strictEqual('yes', properties[0].value);
checkException(message);
assert.deepStrictEqual(JSON.parse(message['result']['value']),
{ old: 'yes' });
}
],
[ // remove module from require.cache
{
'method': 'Debugger.evaluateOnCallFrame', 'params': {
'callFrameId': '{"ordinal":0,"injectedScriptId":1}',
'method': 'Runtime.evaluate', 'params': {
'expression': `delete require.cache[${testModuleStr}]`,
'includeCommandLineAPI': true
}
},
}, (message) => {
checkException(message);
assert.strictEqual(message['result']['value'], true);
}
],
[ // require again, should get fresh (empty) exports
{
'method': 'Runtime.evaluate', 'params': {
'callFrameId': '{"ordinal":0,"injectedScriptId":1}',
'expression': `require(${testModuleStr})`,
'generatePreview': true,
'expression': `JSON.stringify(require(${testModuleStr}))`,
'includeCommandLineAPI': true
}
}, (message) => {
const { properties } = message.result.preview;
assert.strictEqual(0, properties.length);
checkException(message);
assert.deepStrictEqual(JSON.parse(message['result']['value']), {});
}
],
[ // require 2nd module, exports an empty object
{
'method': 'Runtime.evaluate', 'params': {
// 'callFrameId': '{"ordinal":0,"injectedScriptId":1}',
'expression': `require(${printModuleStr})`,
'generatePreview': true,
'expression': `JSON.stringify(require(${printAModuleStr}))`,
'includeCommandLineAPI': true
}
}, (message) => {
const { properties } = message.result.preview;
assert.strictEqual(0, properties.length);
checkException(message);
assert.deepStrictEqual(JSON.parse(message['result']['value']), {});
}
],
[ // both modules end up with the same module.parent
{
'method': 'Debugger.evaluateOnCallFrame', 'params': {
'callFrameId': '{"ordinal":0,"injectedScriptId":1}',
'expression': `({
'method': 'Runtime.evaluate', 'params': {
'expression': `JSON.stringify({
parentsEqual:
require.cache[${testModuleStr}].parent ===
require.cache[${printModuleStr}].parent,
require.cache[${printAModuleStr}].parent,
parentId: require.cache[${testModuleStr}].parent.id,
})`,
'generatePreview': true,
'includeCommandLineAPI': true
}
}, (message) => {
const { properties } = message.result.preview;
assert.strictEqual('[consoleAPI]', properties[1].value);
assert.strictEqual('true', properties[0].value);
checkException(message);
assert.deepStrictEqual(JSON.parse(message['result']['value']), {
parentsEqual: true,
parentId: '<inspector console>'
});
}
],
[ // the `require` in the module shadows the command line API's `require`
{
'method': 'Debugger.evaluateOnCallFrame', 'params': {
'callFrameId': '{"ordinal":0,"injectedScriptId":1}',
'expression': `(
require(${printBModuleStr}),
require.cache[${printBModuleStr}].parent.id
)`,
'includeCommandLineAPI': true
}
}, (message) => {
checkException(message);
assert.notStrictEqual(message['result']['value'],
'<inspector console>');
}
],
]);
Expand Down

0 comments on commit 7a4d57b

Please sign in to comment.