From 62193ba333f5ff16efed33ae6c5f2bc806cade22 Mon Sep 17 00:00:00 2001 From: Eugene Ostroukhov Date: Mon, 30 Oct 2017 16:32:16 -0700 Subject: [PATCH 1/5] inspector: add support for protocol extension This change introduces internal API for extending inspector protocol through JS API. Ref: https://github.com/nodejs/node/issues/16627 --- doc/guides/extending-inspector-protocol.md | 77 +++++++ lib/internal/bootstrap_node.js | 1 + lib/internal/inspector_node_domains.js | 120 ++++++++++ node.gyp | 1 + src/async_wrap.h | 3 +- src/inspector_agent.cc | 191 +++++++++------- src/inspector_agent.h | 46 +++- src/inspector_io.cc | 20 +- src/inspector_io.h | 4 +- src/inspector_js_api.cc | 207 ++++++++++++++++-- test/parallel/test-inspector-custom-domain.js | 95 ++++++++ test/sequential/test-async-wrap-getasyncid.js | 7 + 12 files changed, 655 insertions(+), 117 deletions(-) create mode 100644 doc/guides/extending-inspector-protocol.md create mode 100644 lib/internal/inspector_node_domains.js create mode 100644 test/parallel/test-inspector-custom-domain.js diff --git a/doc/guides/extending-inspector-protocol.md b/doc/guides/extending-inspector-protocol.md new file mode 100644 index 000000000000..270e9989fb3c --- /dev/null +++ b/doc/guides/extending-inspector-protocol.md @@ -0,0 +1,77 @@ +# Extending Node.js Inspector Protocol + +Node.js Inspector Protocol (based on the [Chrome DevTools](https://chromedevtools.github.io/devtools-protocol/v8/)) +is designed to be easily extensible by the implementers. This document provides +details on such extension. + +## Terminology + +* *Inspector session* - message exchange between protocol client and Node.js +* *Protocol handler* - object with a liftime of the inspector session that +interacts with the protocol client. +* *Protocol domain* - 'API unit' of the inspector protocol. Defines message +formats for specific and related functionality. Node.js exposes protocol +domains provided by the V8. These include `Runtime`, `Debugger`, `Profiler` and +`HeapProfiler`. +* *Message* - a JSON string passed between Node.js backend and a inspector +protocol client. Message types are: request, response and notification. Protocol +client only sends out requests. Messages are asynchronous and client should not +assume response will be sent immidiately after serving the request. +* *Method field* - a mandatory field in request and notification messages. +This field value includes *domain* and *method* strings separated by a dot +(e.g. `Debugger.pause`). + +## API + +`internal/inspector_node_domains` module provides API for extending inspector +protocol exposed by the Node.js + +### inspector_node_domains.registerDomainHandlerClass(domain, constructor) + +* domain {string} - a JavaScript identifier string that serves as a unique +identifier for the messages that target this domain. Domains should be unique. +It is not permitted to override built-in Node.js domains. +* constructor {ES6 class|constructor function} - creates a protocol handler that +will receive messages for the specified domain. This function should accept a +single callback argument. This callback can be used by the handler to send +notifications to protocol clients. Callback accepts two arguments - *method* and +optional *parameters* object. This callback can be called multiple times. + +### Handler method + +Protocol dispatcher will call handler method with the name that was specified in +the *method field*. Handler method will receive message *params* object as an +argument. Handler method can return one of the following: +* Nothing. Empty response will be sent to the client. +* Object. This object will be sent to the client as *params* field of +the response. +* Throw exception. Error response will be sent to the client. Exception message +will be included in the response. In the case thrown object does not have the +message field, entire object will be included in the response. +* Function that accepts a callback argument. This function will be invoked +immediately by the dispatcher and enables asynchronous processing of +the inspector message. + +### [SessionTerminatedSymbol] handler method + +This method is invoked when inspector session is terminated and should perform +all necessary cleanup. + +## Things to keep in mind + +Regular event loop is interrupted to process the protocol message. This is to +ensure that tooling is available even while the application is running a +continuous body of code (e.g. infinite loop) or is waiting for IO events. + +This means that the handler implementation should be careful when relying on +async APIs (e.g. promise resolution) as those events may never happen if +the user code is stuck in the infinite loop or if the JavaScript VM is paused on +a breakpoint. + +## Being a good citizen + +* Any state should not outlive the session state. E.g. event collection should +be disabled, buffers should be freed. +* Handlers should never be initiating the exchange. E.g. handlers should not +send any notifications before the client explicitly requests them. Convention +is to only send notification after the client invokes '*Domain*.enable' method. diff --git a/lib/internal/bootstrap_node.js b/lib/internal/bootstrap_node.js index d8ebd9af968c..a837bf19e6f3 100644 --- a/lib/internal/bootstrap_node.js +++ b/lib/internal/bootstrap_node.js @@ -62,6 +62,7 @@ NativeModule.require('internal/trace_events_async_hooks').setup(); NativeModule.require('internal/inspector_async_hook').setup(); + NativeModule.require('internal/inspector_node_domains').setup(); // Do not initialize channel in debugger agent, it deletes env variable // and the main thread won't see it. diff --git a/lib/internal/inspector_node_domains.js b/lib/internal/inspector_node_domains.js new file mode 100644 index 000000000000..36c3dc14fb49 --- /dev/null +++ b/lib/internal/inspector_node_domains.js @@ -0,0 +1,120 @@ +'use strict'; + +const assert = require('assert'); +const util = require('util'); + +const { registerDispatcherFactory } = process.binding('inspector'); + +const domainHandlerClasses = new Map(); +const SessionTerminatedSymbol = Symbol('DisposeAgent'); + +function createNotificationChannel(session, domain) { + return function(method, params) { + session.sendMessageToFrontend(JSON.stringify({ + method: `${domain}.${method}`, + params + })); + }; +} + +function createResponseCallback(session, id) { + let first_call = true; + return function(error, result) { + assert.ok(first_call, 'Callback should only be called once'); + first_call = false; + const response = { id }; + if (error) + response['error'] = error; + else + response['result'] = result || {}; + session.sendMessageToFrontend(JSON.stringify(response)); + }; +} + +function parseSuppressingError(message) { + try { + return JSON.parse(message); + } catch (error) { + process.emitWarning(error); + return {}; + } +} + +class Dispatcher { + constructor(session) { + this._session = session; + this._domainHandlers = new Map(); + } + + dispatch(message) { + if (domainHandlerClasses.size === 0 && this._domainHandlers.size === 0) { + return false; + } + + if (message === null) { + this._shutdown(); + return; + } + const parsed = parseSuppressingError(message); + const [ domain, method ] = parsed.method ? parsed.method.split('.') : []; + + if (!domain || !method) + return false; + + const handler = this._getOrCreateHandler(domain); + + if (!handler || !util.isFunction(handler[method])) { + return false; + } + + const callback = createResponseCallback(this._session, parsed.id); + try { + const result = handler[method](parsed.params); + if (util.isFunction(result)) { + result(callback); + } else { + callback(null, result); + } + } catch (e) { + callback(e.message || e, null); + } + return true; + } + + _getOrCreateHandler(domain) { + let handler = this._domainHandlers.get(domain); + if (handler) + return handler; + var ctor = domainHandlerClasses.get(domain); + if (!ctor) + return null; + handler = new ctor(createNotificationChannel(this._session, domain)); + this._domainHandlers.set(domain, handler); + return handler; + } + + _shutdown() { + for (const handler of this._domainHandlers.values()) { + if (handler[SessionTerminatedSymbol]) + handler[SessionTerminatedSymbol](); + } + this._domainHandlers.clear(); + } + + static create(session) { + const dispatcher = new Dispatcher(session); + return dispatcher.dispatch.bind(dispatcher); + } +} + +function registerDomainHandlerClass(domain, factory) { + domainHandlerClasses.set(domain, factory); +} + +function setup() { + registerDispatcherFactory(Dispatcher.create); +} + +module.exports = { + registerDomainHandlerClass, setup, SessionTerminatedSymbol +}; diff --git a/node.gyp b/node.gyp index ed28b4a6ce19..608541fffd13 100644 --- a/node.gyp +++ b/node.gyp @@ -100,6 +100,7 @@ 'lib/internal/fs.js', 'lib/internal/http.js', 'lib/internal/inspector_async_hook.js', + 'lib/internal/inspector_node_domains.js', 'lib/internal/linkedlist.js', 'lib/internal/loader/Loader.js', 'lib/internal/loader/ModuleMap.js', diff --git a/src/async_wrap.h b/src/async_wrap.h index ec9e162ca79b..d1e444e719aa 100644 --- a/src/async_wrap.h +++ b/src/async_wrap.h @@ -76,7 +76,8 @@ namespace node { #if HAVE_INSPECTOR #define NODE_ASYNC_INSPECTOR_PROVIDER_TYPES(V) \ - V(INSPECTORJSBINDING) + V(INSPECTORJSBINDING) \ + V(INSPECTORJSDISPATCHER) #else #define NODE_ASYNC_INSPECTOR_PROVIDER_TYPES(V) #endif // HAVE_INSPECTOR diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc index 875c12efa023..709aa32152bb 100644 --- a/src/inspector_agent.cc +++ b/src/inspector_agent.cc @@ -186,55 +186,6 @@ static int StartDebugSignalHandler() { const int NANOS_PER_MSEC = 1000000; const int CONTEXT_GROUP_ID = 1; -class ChannelImpl final : public v8_inspector::V8Inspector::Channel { - public: - explicit ChannelImpl(V8Inspector* inspector, - InspectorSessionDelegate* delegate) - : delegate_(delegate) { - session_ = inspector->connect(1, this, StringView()); - } - - virtual ~ChannelImpl() {} - - void dispatchProtocolMessage(const StringView& message) { - session_->dispatchProtocolMessage(message); - } - - bool waitForFrontendMessage() { - return delegate_->WaitForFrontendMessageWhilePaused(); - } - - void schedulePauseOnNextStatement(const std::string& reason) { - std::unique_ptr buffer = Utf8ToStringView(reason); - session_->schedulePauseOnNextStatement(buffer->string(), buffer->string()); - } - - InspectorSessionDelegate* delegate() { - return delegate_; - } - - private: - void sendResponse( - int callId, - std::unique_ptr message) override { - sendMessageToFrontend(message->string()); - } - - void sendNotification( - std::unique_ptr message) override { - sendMessageToFrontend(message->string()); - } - - void flushProtocolNotifications() override { } - - void sendMessageToFrontend(const StringView& message) { - delegate_->SendMessageToFrontend(message); - } - - InspectorSessionDelegate* const delegate_; - std::unique_ptr session_; -}; - class InspectorTimer { public: InspectorTimer(uv_loop_t* loop, @@ -296,6 +247,74 @@ class InspectorTimerHandle { }; } // namespace +class ChannelImpl final : public v8_inspector::V8Inspector::Channel { + public: + ChannelImpl(V8Inspector* inspector, InspectorSessionDelegate* delegate) + : delegate_(delegate), dispatcher_factory_(nullptr) { + session_ = inspector->connect(1, this, StringView()); + } + + ~ChannelImpl() override { + resetDispatcher(); + } + + void dispatchProtocolMessage(const StringView& message) { + if (v8_inspector::V8InspectorSession::canDispatchMethod(message) || + !dispatchToNodeDispatcher(message)) { + session_->dispatchProtocolMessage(message); + } + } + + bool waitForFrontendMessage() { + return delegate_->WaitForFrontendMessageWhilePaused(); + } + + void schedulePauseOnNextStatement(const std::string& reason) { + std::unique_ptr buffer = Utf8ToStringView(reason); + session_->schedulePauseOnNextStatement(buffer->string(), buffer->string()); + } + + void setDispatcherFactory(MessageDispatcherFactory* factory) { + resetDispatcher(); + dispatcher_factory_ = factory; + } + + void resetDispatcher() { + dispatcher_.reset(); + } + + void sendMessageToFrontend(const StringView& message) { + delegate_->SendMessageToFrontend(message); + } + + private: + bool dispatchToNodeDispatcher(const StringView& message) { + if (dispatcher_ == nullptr && dispatcher_factory_ != nullptr) { + dispatcher_ = dispatcher_factory_->Create( + std::unique_ptr(new DispatcherSession(this))); + } + return dispatcher_ != nullptr && dispatcher_->HandleMessage(message); + } + + void sendResponse( + int callId, + std::unique_ptr message) override { + sendMessageToFrontend(message->string()); + } + + void sendNotification( + std::unique_ptr message) override { + sendMessageToFrontend(message->string()); + } + + void flushProtocolNotifications() override { } + + InspectorSessionDelegate* const delegate_; + std::unique_ptr session_; + std::unique_ptr dispatcher_; + MessageDispatcherFactory* dispatcher_factory_; +}; + class NodeInspectorClient : public V8InspectorClient { public: NodeInspectorClient(node::Environment* env, node::NodePlatform* platform) @@ -346,22 +365,23 @@ class NodeInspectorClient : public V8InspectorClient { terminated_ = true; } - void connectFrontend(InspectorSessionDelegate* delegate) { - CHECK_EQ(channel_, nullptr); - channel_ = std::unique_ptr( - new ChannelImpl(client_.get(), delegate)); + std::unique_ptr connectFrontend( + InspectorSessionDelegate* delegate) { + if (channel_) + return nullptr; + channel_.reset(new ChannelImpl(client_.get(), delegate)); + channel_->setDispatcherFactory(dispatcher_factory_.get()); + return std::unique_ptr( + new InspectorSession(this, channel_.get())); } - void disconnectFrontend() { + void disconnectFrontend(ChannelImpl* channel) { + if (channel_.get() != channel) + return; quitMessageLoopOnPause(); channel_.reset(); } - void dispatchMessageFromFrontend(const StringView& message) { - CHECK_NE(channel_, nullptr); - channel_->dispatchProtocolMessage(message); - } - Local ensureDefaultContextInGroup(int contextGroupId) override { return env_->context(); } @@ -447,6 +467,13 @@ class NodeInspectorClient : public V8InspectorClient { client_->allAsyncTasksCanceled(); } + void RegisterDispatcherFactory( + std::unique_ptr factory) { + dispatcher_factory_ = std::move(factory); + if (channel_) + channel_->setDispatcherFactory(dispatcher_factory_.get()); + } + private: node::Environment* env_; node::NodePlatform* platform_; @@ -455,8 +482,26 @@ class NodeInspectorClient : public V8InspectorClient { std::unique_ptr client_; std::unique_ptr channel_; std::unordered_map timers_; + std::unique_ptr dispatcher_factory_; }; +InspectorSession::~InspectorSession() { + client_->disconnectFrontend(channel_); +} + +void InspectorSession::Dispatch(const v8_inspector::StringView& message) { + channel_->dispatchProtocolMessage(message); +} + +DispatcherSession::~DispatcherSession() { + channel_->resetDispatcher(); +} + +void DispatcherSession::SendMessageToFrontend( + const v8_inspector::StringView& message) { + channel_->sendMessageToFrontend(message); +} + Agent::Agent(Environment* env) : parent_env_(env), client_(nullptr), platform_(nullptr), @@ -542,9 +587,10 @@ void Agent::Stop() { } } -void Agent::Connect(InspectorSessionDelegate* delegate) { +std::unique_ptr Agent::Connect( + InspectorSessionDelegate* delegate) { enabled_ = true; - client_->connectFrontend(delegate); + return client_->connectFrontend(delegate); } bool Agent::IsConnected() { @@ -566,29 +612,11 @@ void Agent::FatalException(Local error, Local message) { WaitForDisconnect(); } -void Agent::Dispatch(const StringView& message) { - CHECK_NE(client_, nullptr); - client_->dispatchMessageFromFrontend(message); -} - -void Agent::Disconnect() { - CHECK_NE(client_, nullptr); - client_->disconnectFrontend(); -} - void Agent::RunMessageLoop() { CHECK_NE(client_, nullptr); client_->runMessageLoopOnPause(CONTEXT_GROUP_ID); } -InspectorSessionDelegate* Agent::delegate() { - CHECK_NE(client_, nullptr); - ChannelImpl* channel = client_->channel(); - if (channel == nullptr) - return nullptr; - return channel->delegate(); -} - void Agent::PauseOnNextJavascriptStatement(const std::string& reason) { ChannelImpl* channel = client_->channel(); if (channel != nullptr) @@ -690,6 +718,11 @@ bool Agent::IsWaitingForConnect() { return debug_options_.wait_for_connect(); } +void Agent::RegisterDispatcherFactory( + std::unique_ptr factory) { + client_->RegisterDispatcherFactory(std::move(factory)); +} + } // namespace inspector } // namespace node diff --git a/src/inspector_agent.h b/src/inspector_agent.h index 7211f5a2a49a..c07e8b2040bb 100644 --- a/src/inspector_agent.h +++ b/src/inspector_agent.h @@ -23,6 +23,21 @@ class Environment; namespace inspector { +class ChannelImpl; +class InspectorIo; +class NodeInspectorClient; + +class InspectorSession { + public: + InspectorSession(NodeInspectorClient* client, ChannelImpl* channel) + : channel_(channel), client_(client) { } + ~InspectorSession(); + void Dispatch(const v8_inspector::StringView& message); + private: + ChannelImpl* channel_; + NodeInspectorClient* client_; +}; + class InspectorSessionDelegate { public: virtual ~InspectorSessionDelegate() = default; @@ -31,8 +46,27 @@ class InspectorSessionDelegate { = 0; }; -class InspectorIo; -class NodeInspectorClient; +class DispatcherSession { + public: + explicit DispatcherSession(ChannelImpl* channel) : channel_(channel) {} + ~DispatcherSession(); + void SendMessageToFrontend(const v8_inspector::StringView& message); + private: + ChannelImpl* channel_; +}; + +class MessageDispatcher { + public: + virtual ~MessageDispatcher() = default; + virtual bool HandleMessage(const v8_inspector::StringView& message) = 0; +}; + +class MessageDispatcherFactory { + public: + virtual ~MessageDispatcherFactory() = default; + virtual std::unique_ptr Create( + std::unique_ptr session) = 0; +}; class Agent { public: @@ -67,13 +101,13 @@ class Agent { v8::Local enable_function, v8::Local disable_function); + void RegisterDispatcherFactory( + std::unique_ptr factory); + // These methods are called by the WS protocol and JS binding to create // inspector sessions. The inspector responds by using the delegate to send // messages back. - void Connect(InspectorSessionDelegate* delegate); - void Disconnect(); - void Dispatch(const v8_inspector::StringView& message); - InspectorSessionDelegate* delegate(); + std::unique_ptr Connect(InspectorSessionDelegate* delegate); void RunMessageLoop(); bool enabled() { return enabled_; } diff --git a/src/inspector_io.cc b/src/inspector_io.cc index 538cbab3c9fe..dc4d513951f9 100644 --- a/src/inspector_io.cc +++ b/src/inspector_io.cc @@ -121,11 +121,18 @@ std::unique_ptr Utf8ToStringView(const std::string& message) { class IoSessionDelegate : public InspectorSessionDelegate { public: - explicit IoSessionDelegate(InspectorIo* io) : io_(io) { } + explicit IoSessionDelegate(Agent* agent, InspectorIo* io) + : io_(io), session_(agent->Connect(this)) { } bool WaitForFrontendMessageWhilePaused() override; void SendMessageToFrontend(const v8_inspector::StringView& message) override; + bool IsConnected() { return !!session_; } + void Dispatch(const StringView& message) { + CHECK_NE(session_, nullptr); + session_->Dispatch(message); + } private: InspectorIo* io_; + std::unique_ptr session_; }; // Passed to InspectorSocketServer to handle WS inspector protocol events, @@ -382,9 +389,10 @@ void InspectorIo::DispatchMessages() { session_id_ = std::get<1>(task); state_ = State::kConnected; fprintf(stderr, "Debugger attached.\n"); - session_delegate_ = std::unique_ptr( - new IoSessionDelegate(this)); - parent_env_->inspector_agent()->Connect(session_delegate_.get()); + session_delegate_.reset( + new IoSessionDelegate(parent_env_->inspector_agent(), this)); + if (!session_delegate_->IsConnected()) + session_delegate_.reset(); break; case InspectorAction::kEndSession: CHECK_NE(session_delegate_, nullptr); @@ -393,11 +401,11 @@ void InspectorIo::DispatchMessages() { } else { state_ = State::kAccepting; } - parent_env_->inspector_agent()->Disconnect(); session_delegate_.reset(); break; case InspectorAction::kSendMessage: - parent_env_->inspector_agent()->Dispatch(message); + if (session_delegate_) + session_delegate_->Dispatch(message); break; } } diff --git a/src/inspector_io.h b/src/inspector_io.h index 7c15466eed91..76a99d03dc64 100644 --- a/src/inspector_io.h +++ b/src/inspector_io.h @@ -47,6 +47,8 @@ enum class TransportAction { kStop }; +class IoSessionDelegate; + class InspectorIo { public: InspectorIo(node::Environment* env, v8::Platform* platform, @@ -143,7 +145,7 @@ class InspectorIo { // Note that this will live while the async is being closed - likely, past // the parent object lifespan std::pair* main_thread_req_; - std::unique_ptr session_delegate_; + std::unique_ptr session_delegate_; v8::Platform* platform_; // Message queues diff --git a/src/inspector_js_api.cc b/src/inspector_js_api.cc index 428d8391f258..d4d1611f6810 100644 --- a/src/inspector_js_api.cc +++ b/src/inspector_js_api.cc @@ -32,6 +32,162 @@ std::unique_ptr ToProtocolString(Isolate* isolate, return StringBuffer::create(StringView(*buffer, buffer.length())); } +class JSDispatcher; + +class JSDispatcherInterface : private AsyncWrap { + public: + JSDispatcherInterface(Environment* env, Local object) + : AsyncWrap(env, object, PROVIDER_INSPECTORJSDISPATCHER), + env_(env), + dispatcher_(nullptr) { + Wrap(object, this); + env->SetMethod(object, "sendMessageToFrontend", + JSDispatcherInterface::SendMessageToFrontend); + } + + ~JSDispatcherInterface() override; + + bool HandleMessage(const v8_inspector::StringView& message) { + Isolate* isolate = env_->isolate(); + HandleScope handle_scope(isolate); + if (env_->context().IsEmpty()) { + // Post-shutdown cleanup, should not be any real remaining messages + CHECK_EQ(message.length(), 0); + return false; + } + Context::Scope context_scope(env_->context()); + Local argument; + if (message.length() > 0) { + MaybeLocal v8string = + String::NewFromTwoByte(isolate, message.characters16(), + NewStringType::kNormal, message.length()); + argument = v8string.ToLocalChecked().As(); + } else { + argument = Null(isolate); + } + MaybeLocal result = + MakeCallback(callback_.Get(env()->isolate()), 1, &argument); + Local r; + return result.ToLocal(&r) && r->IsTrue(); + } + + void SetCallback(Local callback) { + callback_.Reset(env_->isolate(), callback); + } + + void SetDispatcher(JSDispatcher* dispatcher) { + if (dispatcher == nullptr && dispatcher_ != nullptr) { + HandleMessage(v8_inspector::StringView()); + } + dispatcher_ = dispatcher; + } + + size_t self_size() const override { + return sizeof(*this); + } + + static void SendMessageToFrontend(const FunctionCallbackInfo& info); + + private: + Environment* env_; + JSDispatcher* dispatcher_; + Persistent callback_; +}; + +class JSDispatcher : public MessageDispatcher { + public: + JSDispatcher(std::unique_ptr&& session, + JSDispatcherInterface* interface) + : js_interface_(interface), session_(std::move(session)) { + interface->SetDispatcher(this); + } + + ~JSDispatcher() override { + if (js_interface_ != nullptr) { + js_interface_->SetDispatcher(nullptr); + } + } + + bool HandleMessage(const v8_inspector::StringView& message) override { + return js_interface_ != nullptr && js_interface_->HandleMessage(message); + } + + void ResetInterface() { + js_interface_ = nullptr; + } + + void SendMessageToFrontend(const StringView& message) { + session_->SendMessageToFrontend(message); + } + + private: + JSDispatcherInterface* js_interface_; + std::unique_ptr session_; +}; + +JSDispatcherInterface::~JSDispatcherInterface() { + callback_.Reset(); + if (dispatcher_ != nullptr) { + dispatcher_->ResetInterface(); + } +} + +void JSDispatcherInterface::SendMessageToFrontend( + const FunctionCallbackInfo& info) { + JSDispatcherInterface* interface; + ASSIGN_OR_RETURN_UNWRAP(&interface, info.Holder()); + Environment* env = interface->env_; + JSDispatcher* dispatcher = interface->dispatcher_; + if (info.Length() != 1 || !info[0]->IsString()) { + env->ThrowTypeError("Protocol message should be a string"); + return; + } + if (dispatcher == nullptr) { + return; + } + dispatcher->SendMessageToFrontend( + ToProtocolString(env->isolate(), info[0])->string()); +} + +class JSDispatcherFactory : public MessageDispatcherFactory { + public: + JSDispatcherFactory(Environment* env, Local js_factory) + : env_(env), js_factory_(env->isolate(), js_factory) {} + + std::unique_ptr Create( + std::unique_ptr session) + override { + Isolate* isolate = env_->isolate(); + Local context = env_->context(); + HandleScope handle_scope(isolate); + Context::Scope context_scope(context); + + Local tmpl = env_->NewFunctionTemplate(nullptr); + tmpl->InstanceTemplate()->SetInternalFieldCount(1); + tmpl->SetClassName( + FIXED_ONE_BYTE_STRING(env_->isolate(), + "InspectorDispatcherConnection")); + AsyncWrap::AddWrapMethods(env_, tmpl); + Local object = + tmpl->GetFunction()->NewInstance(context).ToLocalChecked(); + JSDispatcherInterface* dispatcher = new JSDispatcherInterface(env_, object); + Local arg = object.As(); + // Note that this code is meant to be internal so it is ok to assume + // correct arguments + Local callback = + js_factory_.Get(isolate)->Call(context, Null(isolate), 1, &arg) + .ToLocalChecked(); + CHECK(callback->IsFunction()); + dispatcher->SetCallback(callback.As()); + return std::unique_ptr( + new JSDispatcher(std::move(session), dispatcher)); + } + + private: + Environment* env_; + Persistent js_factory_; +}; + class JSBindingsConnection : public AsyncWrap { public: class JSBindingsSessionDelegate : public InspectorSessionDelegate { @@ -58,12 +214,6 @@ class JSBindingsConnection : public AsyncWrap { connection_->OnMessage(argument); } - void Disconnect() { - Agent* agent = env_->inspector_agent(); - if (agent->delegate() == this) - agent->Disconnect(); - } - private: Environment* env_; JSBindingsConnection* connection_; @@ -78,11 +228,11 @@ class JSBindingsConnection : public AsyncWrap { Wrap(wrap, this); Agent* inspector = env->inspector_agent(); - if (inspector->delegate() != nullptr) { + session_ = inspector->Connect(&delegate_); + if (!session_) { env->ThrowTypeError("Session is already attached"); return; } - inspector->Connect(&delegate_); } ~JSBindingsConnection() override { @@ -93,11 +243,6 @@ class JSBindingsConnection : public AsyncWrap { MakeCallback(callback_.Get(env()->isolate()), 1, &value); } - void CheckIsCurrent() { - Agent* inspector = env()->inspector_agent(); - CHECK_EQ(&delegate_, inspector->delegate()); - } - static void New(const FunctionCallbackInfo& info) { Environment* env = Environment::GetCurrent(info); if (!info[0]->IsFunction()) { @@ -109,7 +254,8 @@ class JSBindingsConnection : public AsyncWrap { } void Disconnect() { - delegate_.Disconnect(); + if (session_) + session_.reset(); if (!persistent().IsEmpty()) { ClearWrap(object()); persistent().Reset(); @@ -125,16 +271,14 @@ class JSBindingsConnection : public AsyncWrap { static void Dispatch(const FunctionCallbackInfo& info) { Environment* env = Environment::GetCurrent(info); - JSBindingsConnection* session; - ASSIGN_OR_RETURN_UNWRAP(&session, info.Holder()); if (!info[0]->IsString()) { env->ThrowTypeError("Inspector message must be a string"); return; } - - session->CheckIsCurrent(); - Agent* inspector = env->inspector_agent(); - inspector->Dispatch(ToProtocolString(env->isolate(), info[0])->string()); + JSBindingsConnection* session; + ASSIGN_OR_RETURN_UNWRAP(&session, info.Holder()); + session->session_->Dispatch( + ToProtocolString(env->isolate(), info[0])->string()); } size_t self_size() const override { return sizeof(*this); } @@ -142,6 +286,7 @@ class JSBindingsConnection : public AsyncWrap { private: JSBindingsSessionDelegate delegate_; Persistent callback_; + std::unique_ptr session_; }; @@ -213,7 +358,7 @@ void InspectorConsoleCall(const FunctionCallbackInfo& info) { call_args.data()).FromMaybe(Local()); } -static void* GetAsyncTask(int64_t asyncId) { +void* GetAsyncTask(int64_t asyncId) { // The inspector assumes that when other clients use its asyncTask* API, // they use real pointers, or at least something aligned like real pointer. // In general it means that our task_id should always be even. @@ -226,14 +371,14 @@ static void* GetAsyncTask(int64_t asyncId) { } template -static void InvokeAsyncTaskFnWithId(const FunctionCallbackInfo& args) { +void InvokeAsyncTaskFnWithId(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); CHECK(args[0]->IsNumber()); int64_t task_id = args[0]->IntegerValue(env->context()).FromJust(); (env->inspector_agent()->*asyncTaskFn)(GetAsyncTask(task_id)); } -static void AsyncTaskScheduledWrapper(const FunctionCallbackInfo& args) { +void AsyncTaskScheduledWrapper(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); CHECK(args[0]->IsString()); @@ -251,7 +396,7 @@ static void AsyncTaskScheduledWrapper(const FunctionCallbackInfo& args) { env->inspector_agent()->AsyncTaskScheduled(task_name_view, task, recurring); } -static void RegisterAsyncHookWrapper(const FunctionCallbackInfo& args) { +void RegisterAsyncHookWrapper(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); CHECK(args[0]->IsFunction()); @@ -267,6 +412,17 @@ void IsEnabled(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(env->inspector_agent()->enabled()); } +void RegisterDispatcherFactory(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + if (args.Length() == 1 || (args[0]->IsNull() || args[0]->IsFunction())) { + env->inspector_agent()->RegisterDispatcherFactory( + std::unique_ptr( + new JSDispatcherFactory(env, args[0].As()))); + } else { + env->ThrowTypeError("Factory should be a function"); + } +} + void Open(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); Agent* agent = env->inspector_agent(); @@ -333,6 +489,9 @@ void InitInspectorBindings(Local target, Local unused, env->SetMethod(target, "registerAsyncHook", RegisterAsyncHookWrapper); env->SetMethod(target, "isEnabled", IsEnabled); + env->SetMethod(target, "registerDispatcherFactory", + RegisterDispatcherFactory); + auto conn_str = FIXED_ONE_BYTE_STRING(env->isolate(), "Connection"); Local tmpl = env->NewFunctionTemplate(JSBindingsConnection::New); diff --git a/test/parallel/test-inspector-custom-domain.js b/test/parallel/test-inspector-custom-domain.js new file mode 100644 index 000000000000..6882b45f442d --- /dev/null +++ b/test/parallel/test-inspector-custom-domain.js @@ -0,0 +1,95 @@ +// Flags: --expose-internals +'use strict'; +const common = require('../common'); +common.skipIfInspectorDisabled(); +const assert = require('assert'); +const { Session } = require('inspector'); +const { registerDomainHandlerClass, SessionTerminatedSymbol } = + require('internal/inspector_node_domains'); +const util = require('util'); + +let handler; + +class TestDomainHandler { + constructor(notificationsChannel) { + this.notificationsChannel = notificationsChannel; + this.field = 0; + this.done = false; + handler = this; + } + + methodA(params) { + return params.a + params.b; + } + + methodB(params) { + throw new Error('Booom!'); + } + + methodC(params) { + return (callback) => this.cb = callback; + } + + methodD() { + const a = this.field++; + return { a }; + } + + [SessionTerminatedSymbol]() { + this.done = true; + } +} + +async function test() { + const session = new Session(); + session.connect(); + const post = util.promisify(session.post.bind(session)); + try { + await post('Test.methodA'); + assert.fails('Exception must have been thrown'); + } catch (e) { + assert.strictEqual(-32601, e.code); + } + + registerDomainHandlerClass('Test', TestDomainHandler); + assert.strictEqual(3, await post('Test.methodA', { a: 1, b: 2 })); + try { + await post('Test.methodB'); + assert.fails(); + } catch (e) { + assert.strictEqual('Booom!', e); + } + let result = null; + session.post('Test.methodC', (error, params) => { + result = { error, params }; + }); + assert.strictEqual(null, result); + handler.cb(null, 5); + assert.deepStrictEqual({ error: null, params: 5 }, result); + + let notification = null; + session.on('Test.notification1', (n) => notification = n); + assert.strictEqual(null, notification); + + handler.notificationsChannel('notification1', { c: 8 }); + assert.deepStrictEqual({ method: 'Test.notification1', params: { c: 8 } }, + notification); + assert.deepStrictEqual({ a: 0 }, await post('Test.methodD')); + assert.deepStrictEqual({ a: 1 }, await post('Test.methodD')); + assert.deepStrictEqual({ a: 2 }, await post('Test.methodD')); + + assert(!handler.done); + session.disconnect(); + assert(handler.done); + + const session1 = new Session(); + session1.connect(); + const post1 = util.promisify(session1.post.bind(session1)); + assert.deepStrictEqual({ a: 0 }, await post1('Test.methodD')); + assert.deepStrictEqual({ a: 1 }, await post1('Test.methodD')); + assert.deepStrictEqual({ a: 2 }, await post1('Test.methodD')); +} + +common.crashOnUnhandledRejection(); + +test(); diff --git a/test/sequential/test-async-wrap-getasyncid.js b/test/sequential/test-async-wrap-getasyncid.js index bd2b3254f065..51184765d822 100644 --- a/test/sequential/test-async-wrap-getasyncid.js +++ b/test/sequential/test-async-wrap-getasyncid.js @@ -285,7 +285,14 @@ if (common.hasCrypto) { // eslint-disable-line crypto-check if (process.config.variables.v8_enable_inspector !== 0) { const binding = process.binding('inspector'); + let dispatcherHandle = null; + binding.registerDispatcherFactory((dh) => { + dispatcherHandle = dh; + return () => {}; + }); const handle = new binding.Connection(() => {}); + handle.dispatch(JSON.stringify({ id: 1, method: 'FakeDomain.method' })); testInitialized(handle, 'Connection'); + testInitialized(dispatcherHandle, 'InspectorDispatcherConnection'); handle.disconnect(); } From d5124bd3735c4c78c83511e4df3edd335f266b14 Mon Sep 17 00:00:00 2001 From: Eugene Ostroukhov Date: Fri, 13 Oct 2017 09:06:20 -0700 Subject: [PATCH 2/5] inspector: add 'Target' domain handler 'Target' domain handler implementation is extensible, enabling the user code to report targets and implement the transport. Ref: https://github.com/nodejs/node/issues/16627 --- lib/internal/inspector_custom_domains.md | 0 lib/internal/inspector_node_domains.js | 315 ++++++++++++++- test/common/index.js | 5 + test/parallel/test-inspector-target-domain.js | 381 ++++++++++++++++++ 4 files changed, 700 insertions(+), 1 deletion(-) create mode 100644 lib/internal/inspector_custom_domains.md create mode 100644 test/parallel/test-inspector-target-domain.js diff --git a/lib/internal/inspector_custom_domains.md b/lib/internal/inspector_custom_domains.md new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/lib/internal/inspector_node_domains.js b/lib/internal/inspector_node_domains.js index 36c3dc14fb49..a032a9681fe0 100644 --- a/lib/internal/inspector_node_domains.js +++ b/lib/internal/inspector_node_domains.js @@ -1,6 +1,7 @@ 'use strict'; const assert = require('assert'); +const EventEmitter = require('events'); const util = require('util'); const { registerDispatcherFactory } = process.binding('inspector'); @@ -40,6 +41,313 @@ function parseSuppressingError(message) { } } +let targetIdCounter = 1; +let sessionIdCounter = 1; + +const targetsAgent = new class extends EventEmitter { + constructor() { + super(); + this._targets = new Map(); + } + + add(info, attachCallback) { + info.targetId = `target_${targetIdCounter++}`; + this._targets.set(info.targetId, { info, attachCallback }); + this.emit('targetCreated', info); + return { + id: info.targetId, // This is for test only + destroyed: () => this._targetDestroyed(info), + updated: this._targetUpdated.bind(this, info) + }; + } + + attach(targetId, attachedCallback, messageCallback) { + const target = this._targets.get(targetId); + if (!target) + throw new Error(`Target ${targetId} was not found`); + target.attachCallback(attachedCallback, messageCallback); + } + + targets() { + return Array.from(this._targets.values()).map(({ info }) => info); + } + + getTarget(id) { + const target = this._targets.get(id); + return target && target.info; + } + + _targetDestroyed(target) { + if (this._targets.delete(target.targetId)) { + this.emit('targetDestroyed', target); + } + } + + _targetUpdated(info, type, title, url) { + let updated = false; + if (type && info.type !== type) { + updated = true; + info.type = type; + } + if (title && info.title !== title) { + updated = true; + info.title = title; + } + if (url && info.url !== url) { + updated = true; + info.url = url; + } + if (updated) + this.emit('targetInfoChanged', info); + } +}(); + +class TargetDomainHandler { + constructor(notificationCallback) { + this._notificationCallback = notificationCallback; + this._discoverTargets = false; + this._autoAttach = false; + this._waitForDebuggerOnStart = false; + this._listeners = [ + this._addListener('targetCreated', + (target) => this._onTargetCreated(target)), + this._addListener('targetDestroyed', + (target) => this._onTargetDestroyed(target)) + ]; + this._discoveryListeners = []; + + this._sessions = new Map(); + } + + setDiscoverTargets({ discover }) { + discover = !!discover; + if (discover === this._discoverTargets) + return; + this._discoverTargets = discover; + if (this._discoverTargets) { + this._discoveryListeners = [ + this._forwardEventAsNotification('targetInfoChanged', true) + ]; + } else { + for (const remover of this._discoveryListeners) { + remover(); + } + this._discoveryListeners = []; + } + } + + setAutoAttach({ autoAttach, waitForDebuggerOnStart }) { + this._autoAttach = !!autoAttach; + this._waitForDebuggerOnStart = !!waitForDebuggerOnStart; + if (this._autoAttach) + return this._attachToAllTargets.bind(this); + else + return this._detachFromAllTargets.bind(this); + } + + getTargets() { + return targetsAgent.targets() + .map((target) => this._toProtocolTarget(target)); + } + + getTargetInfo({ targetId }) { + this._checkArgument('targetId', targetId); + const target = targetsAgent.getTarget(targetId); + if (target) + return this._toProtocolTarget(target); + else + throw new Error(`Target ${targetId} was not found`); + } + + attachToTarget({ targetId }) { + this._checkArgument('targetId', targetId); + return this._attachToTarget.bind(this, targetId); + } + + detachFromTarget({ sessionId }) { + return this._detachSession.bind(this, sessionId); + } + + sendMessageToTarget({ sessionId, message }) { + this._checkArgument('message', message); + return (callback) => { + this._getSessionChecked(sessionId).messageCallback(message, callback); + }; + } + + _checkArgument(name, value) { + if (!value) + throw new Error(`Argument ${name} was not provided`); + } + + _attachToAllTargets(callback) { + let count = 1; + function decrease() { + if ((--count) === 0) { + callback(); + } + } + for (const target of targetsAgent.targets()) { + count++; + // Note that we don't care if attach fails + try { + this._attachToTarget(target.targetId, decrease); + } catch (e) { + decrease(); + } + } + decrease(); + } + + _detachFromAllTargets(callback) { + let count = 1; + function decrease() { + if ((--count) === 0) { + callback(); + } + } + for (const sessionId of this._sessions.keys()) { + count++; + try { + this._detachSession(sessionId, decrease); + } catch (e) { + decrease(); + } + } + decrease(); + } + + _attachToTarget(targetId, callback) { + const sessionId = `${targetId}:${sessionIdCounter++}`; + const attachedCb = this._onSessionAttached.bind(this, targetId, sessionId, + callback); + const messageCb = this._onMessage.bind(this, sessionId); + targetsAgent.attach(targetId, attachedCb, messageCb); + } + + _detachSession(sessionId, callback) { + const { targetId, detachCallback } = this._getSessionChecked(sessionId); + const target = this._toProtocolTarget(targetsAgent.getTarget(targetId)); + detachCallback((error) => { + this._sessionDetached(sessionId, target); + callback(error); + }); + } + + _getSessionChecked(sessionId) { + this._checkArgument('sessionId', sessionId); + const session = this._sessions.get(sessionId); + if (!session) + throw new Error(`Session ${sessionId} was not found`); + return session; + } + + _onMessage(sessionId, message) { + const session = this._sessions.get(sessionId); + if (!session) + return; + if (message === null) { + this._sessionDetached(sessionId, + targetsAgent.getTarget(session.targetId)); + } else { + this._notificationCallback( + 'receivedMessageFromTarget', { sessionId, message }); + } + + } + + _onTargetCreated(target) { + if (this._discoverTargets) { + const targetInfo = this._toProtocolTarget(target); + this._notificationCallback('targetCreated', { targetInfo }); + } + if (this._autoAttach) { + this._attachToTarget(target.targetId, () => {}); + } + } + + _onTargetDestroyed(target) { + for (const sessionId of this._sessionsForTarget(target.targetId)) { + this._sessionDetached(sessionId, target); + } + if (this._discoverTargets) { + const targetInfo = this._toProtocolTarget(target); + this._notificationCallback('targetDestroyed', { targetInfo }); + } + } + + _onSessionAttached(targetId, sessionId, callback, error, waitingForDebugger, + messageCallback, detachCallback) { + if (error) { + callback(error); + return; + } + const session = { targetId, detachCallback, messageCallback }; + this._sessions.set(sessionId, session); + const targetInfo = this._toProtocolTarget(targetsAgent.getTarget(targetId)); + if (this._sessionsForTarget(targetId).length === 1) + this._sendTargetInfoChanged(targetInfo); + this._notificationCallback('attachedToTarget', + { targetInfo, sessionId, waitingForDebugger }); + callback(null, { sessionId }); + } + + _sendTargetInfoChanged(targetInfo) { + this._notificationCallback('targetInfoChanged', { targetInfo }); + } + + _sessionDetached(sessionId, target) { + if (!this._sessions.has(sessionId)) + return; + const { targetId } = this._sessions.get(sessionId); + this._sessions.delete(sessionId); + this._notificationCallback('detachedFromTarget', { sessionId }); + if (this._sessionsForTarget(targetId).length === 0) { + this._sendTargetInfoChanged(this._toProtocolTarget(target)); + } + } + + _removeListeners() { + for (const remover of this._listeners) { + remover(); + } + } + + _forwardEventAsNotification(event, wrap) { + const listener = (data) => { + const targetInfo = this._toProtocolTarget(data); + const object = wrap ? { targetInfo } : targetInfo; + this._notificationCallback(event, object); + }; + return this._addListener(event, listener); + } + + _addListener(event, listener) { + targetsAgent.addListener(event, listener); + return () => { + targetsAgent.removeListener(event, listener); + }; + } + + _toProtocolTarget(target) { + const attached = this._sessionsForTarget(target.targetId).length > 0; + return Object.assign({ attached }, target); + } + + _sessionsForTarget(tid) { + return Array.from(this._sessions.entries()) + .filter(([, { targetId }]) => targetId === tid) + .map(([ agentSessionId ]) => agentSessionId); + } + + [SessionTerminatedSymbol]() { + for (const { detachCallback } of this._sessions.values()) + detachCallback(() => {}); + this._removeListeners(); + this._listeners.forEach((cancel) => cancel()); + } +} + class Dispatcher { constructor(session) { this._session = session; @@ -107,14 +415,19 @@ class Dispatcher { } } +function newTarget(type, title, url, attachCallback) { + return targetsAgent.add({ type, title, url }, attachCallback); +} + function registerDomainHandlerClass(domain, factory) { domainHandlerClasses.set(domain, factory); } function setup() { + registerDomainHandlerClass('Target', TargetDomainHandler); registerDispatcherFactory(Dispatcher.create); } module.exports = { - registerDomainHandlerClass, setup, SessionTerminatedSymbol + newTarget, registerDomainHandlerClass, setup, SessionTerminatedSymbol }; diff --git a/test/common/index.js b/test/common/index.js index ee2de37bb72a..768fab6d4c13 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -893,3 +893,8 @@ exports.fires = function fires(promise, error, timeoutMs) { timeout ]); }; + +exports.assertRejected = function(promise, message) { + promise.then(() => Promise.reject(message || 'Promise was not rejected'), + () => {}); +}; diff --git a/test/parallel/test-inspector-target-domain.js b/test/parallel/test-inspector-target-domain.js new file mode 100644 index 000000000000..598c1f2878e4 --- /dev/null +++ b/test/parallel/test-inspector-target-domain.js @@ -0,0 +1,381 @@ +// Flags: --expose-internals +'use strict'; +const common = require('../common'); +common.skipIfInspectorDisabled(); +const assert = require('assert'); +const { Session } = require('inspector'); +const { newTarget } = require('internal/inspector_node_domains'); +const util = require('util'); + +const targets = new Set(); + +class Target { + constructor(suffix, autoconnect) { + this._type = 'node'; + this._title = `Test target ${suffix}`; + this._url = `ws://localhost/id/${suffix}`; + this._attachCallback = null; + this._sessions = new Set(); + this.targetAttached = false; + this.messages = []; + this._autoconnect = !!autoconnect; + const { id, destroyed, updated } = + newTarget(this._type, this._title, this._url, this.attach.bind(this)); + this.id = id; + this._destroyed = destroyed; + this._updatedCb = updated; + targets.add(this); + } + + destroyed() { + targets.delete(this); + this._destroyed(); + } + + attach(attachCallback, messageCallback) { + this._attachCallback = attachCallback; + this._messageCallback = messageCallback; + if (this._autoconnect) + this.attached(false); + } + + attached(waitingForDebugger) { + this._attachCallback( + null, + waitingForDebugger, + (message, callback) => { + this.messages.push(message); + callback(); + }, + (callback) => { + assert.ok(this.targetAttached, 'Should be attached'); + this.targetAttached = false; + callback(); + }); + this.targetAttached = true; + } + + detach() { + assert(this.targetAttached); + this.targetAttached = false; + this._messageCallback(null); + } + + toInfo(attachedOverride) { + const attached = + attachedOverride === undefined ? this.targetAttached : attachedOverride; + return { + targetId: this.id, + attached, + type: this._type, + title: this._title, + url: this._url + }; + } + + updateTitle(title) { + this._title = title; + this._updatedCb(null, title); + } + + postMessage(message) { + this._messageCallback(message); + } +} + +const session = new Session(); +const post = util.promisify(session.post.bind(session)); + +function collectNotifications(session, event, array) { + function listener(notification) { + array.push(notification); + } + session.addListener(event, listener); + return function() { + session.removeListener(event, listener); + }; +} + +function clearTargets() { + for (const target of targets) { + target.destroyed(); + } +} + +function _t(notifications) { + return notifications.map((notification) => notification.params.targetInfo); +} + +async function testTargetsDiscovery() { + console.log('Testing targets discovery'); + const targetsCreated = []; + const targetsDestroyed = []; + + const listeners = [ + collectNotifications(session, 'Target.targetCreated', targetsCreated), + collectNotifications(session, 'Target.targetDestroyed', targetsDestroyed) + ]; + + assert.deepStrictEqual([], await post('Target.getTargets')); + const target1 = new Target('a'); + assert.deepStrictEqual([ target1.toInfo() ], + await post('Target.getTargets')); + assert.deepStrictEqual([], _t(targetsCreated)); + await post('Target.setDiscoverTargets', { discover: true }); + assert.deepStrictEqual([], _t(targetsCreated)); + const target2 = new Target('b'); + assert.deepStrictEqual([ target2.toInfo() ], _t(targetsCreated)); + await post('Target.setDiscoverTargets', { discover: false }); + const target3 = new Target('c'); + assert.deepStrictEqual([ target2.toInfo() ], _t(targetsCreated)); + assert.deepStrictEqual([ + target1.toInfo(), target2.toInfo(), target3.toInfo() + ], await post('Target.getTargets')); + const target = await post('Target.getTargetInfo', { targetId: target3.id }); + assert.deepStrictEqual(target3.toInfo(), target); + await common.assertRejected( + post('Target.getTargetInfo', { targetId: 'bogus' }), + 'Should have reported no such target'); + + target2.destroyed(); + assert.deepStrictEqual([], _t(targetsDestroyed)); + assert.deepStrictEqual([ + target1.toInfo(), target3.toInfo() + ], await post('Target.getTargets')); + await post('Target.setDiscoverTargets', { discover: true }); + target3.destroyed(); + assert.deepStrictEqual([ target3.toInfo() ], _t(targetsDestroyed)); + listeners.forEach((fn) => fn()); +} + +async function testAttach() { + console.log('Testing attaching to a target'); + const changed = []; + const attachedTargets = []; + const detachedTargets = []; + const destroyed = []; + + const listeners = [ + collectNotifications(session, 'Target.targetInfoChanged', changed), + collectNotifications(session, 'Target.attachedToTarget', attachedTargets), + collectNotifications(session, 'Target.detachedFromTarget', detachedTargets), + collectNotifications(session, 'Target.targetDestroyed', destroyed) + ]; + + const target = new Target('attachable'); + let wasAttached = false; + const promise = post('Target.attachToTarget', { targetId: target.id }) + .then((result) => { + wasAttached = true; + return result; + }); + assert.ok(!wasAttached); + assert.deepStrictEqual(target.toInfo(), + await post('Target.getTargetInfo', + { targetId: target.id })); + + assert.deepStrictEqual([], changed); + assert.deepStrictEqual([], attachedTargets); + + const waitingForDebugger = false; + + target.attached(waitingForDebugger); + const { sessionId } = await promise; + assert.ok(wasAttached); + assert.deepStrictEqual(target.toInfo(), + await post('Target.getTargetInfo', + { targetId: target.id })); + assert.deepStrictEqual( + { sessionId, targetInfo: target.toInfo(), waitingForDebugger }, + attachedTargets[0].params); + assert.deepStrictEqual(target.toInfo(), changed[0].params.targetInfo); + + target.destroyed(); + + assert.deepStrictEqual(target.toInfo(false), destroyed[0].params.targetInfo); + assert.deepStrictEqual(target.toInfo(false), changed[1].params.targetInfo); + assert.deepStrictEqual({ sessionId }, detachedTargets[0].params); + listeners.forEach((fn) => fn()); +} + +async function testMessages() { + console.log('Testing messages passing'); + const detached = []; + const messages = []; + + const listeners = [ + collectNotifications(session, 'Target.detachedFromTarget', detached), + collectNotifications(session, 'Target.receivedMessageFromTarget', messages) + ]; + + const target = new Target('message_recipient'); + const promise = post('Target.attachToTarget', { targetId: target.id }); + target.attached(false); + const { sessionId } = await promise; + + assert.strictEqual(0, messages.length); + const message = 'Test message'; + target.postMessage(message); + assert.strictEqual(1, messages.length); + assert.deepStrictEqual({ sessionId, message }, messages[0].params); + + assert.strictEqual(0, target.messages.length); + await post('Target.sendMessageToTarget', { sessionId, message }); + assert.deepStrictEqual([message], target.messages); + + assert.strictEqual(0, detached.length); + await post('Target.detachFromTarget', { targetId: target.id, sessionId }); + assert.deepStrictEqual({ sessionId }, detached[0].params); + assert.deepStrictEqual(1, detached.length); + + listeners.forEach((fn) => fn()); +} + +async function testChildSessionsLifetime() { + console.log('Testing target connection lifetime is linked to parent session'); + const s = new Session(); + const t1 = new Target(1, true); + const t2 = new Target(2, true); + s.connect(); + + const p = util.promisify(s.post.bind(s)); + + await p('Target.attachToTarget', { targetId: t1.id }); + await p('Target.attachToTarget', { targetId: t2.id }); + + assert.ok(t1.targetAttached); + assert.ok(t2.targetAttached); + + s.disconnect(); + + assert.ok(!t1.targetAttached); + assert.ok(!t2.targetAttached); +} + +async function testErrors() { + console.log('Testing errors thrown'); + const s = new Session(); + s.connect(); + const p = util.promisify(s.post.bind(s)); + + const target = new Target('exists', true); + + await common.assertRejected(p('Target.getTargetInfo')); + await common.assertRejected(p('Target.getTargetInfo', + { targetId: 'bad' })); + + await common.assertRejected(p('Target.attachToTarget')); + await common.assertRejected(p('Target.attachToTarget', + { targetId: 'bad' })); + + await common.assertRejected(p('Target.detachFromTarget')); + await common.assertRejected(p('Target.detachFromTarget', + { sessionId: 'bad' })); + + await common.assertRejected(p('Target.sendMessageToTarget', + { sessionId: 'sid' })); + + const { sessionId } = + await p('Target.attachToTarget', { targetId: target.id }); + await common.assertRejected(p('Target.sendMessageToTarget', + { sessionId })); + + await p('Target.sendMessageToTarget', { sessionId, message: 'amessage' }); + await p('Target.detachFromTarget', { sessionId }); + await common.assertRejected(p('Target.sendMessageToTarget', + { sessionId, message: 'amessage' })); + + s.disconnect(); +} + +async function testAutoAttach() { + console.log('Testing auto attach'); + const s = new Session(); + s.connect(); + const p = util.promisify(s.post.bind(s)); + + const first = new Target('first', true); + assert(!first.targetAttached); + await p('Target.setAutoAttach', { autoAttach: true }); + assert(first.targetAttached); + const second = new Target('second', true); + assert(second.targetAttached); + await p('Target.setAutoAttach', { autoAttach: false }); + assert(!first.targetAttached); + assert(!second.targetAttached); + + const third = new Target('third', true); + assert(!third.targetAttached); + s.disconnect(); + + const s2 = new Session(); + s2.connect(); + await s2.post('Target.setAutoAttach', { autoAttach: true }); + assert.deepStrictEqual( + [ true, true, true ], + [ first, second, third ].map((t) => t.targetAttached)); + s2.disconnect(); +} + +async function testTitleUpdate() { + console.log('Test title update'); + const updates = []; + const s = new Session(); + collectNotifications(s, 'Target.targetInfoChanged', updates), + s.connect(); + const p = util.promisify(s.post.bind(s)); + + const t = new Target('target', true); + + await p('Target.setDiscoverTargets', { discover: true }); + assert.strictEqual(0, updates.length); + const newTitle = 'new'; + t.updateTitle(newTitle); + assert.deepStrictEqual([{ + method: 'Target.targetInfoChanged', + params: { targetInfo: t.toInfo() } + }], updates); + t.updateTitle(newTitle); + assert.strictEqual(1, updates.length); + s.disconnect(); +} + +async function testDetach() { + console.log('Test target may initiate detach'); + const changed = []; + const detached = []; + + const s = new Session(); + collectNotifications(s, 'Target.targetInfoChanged', changed), + collectNotifications(s, 'Target.detachedFromTarget', detached), + s.connect(); + const p = util.promisify(s.post.bind(s)); + await p('Target.setAutoAttach', { autoAttach: true }); + const t = new Target('detachesitself', true); + + assert.strictEqual(true, changed[0].params.targetInfo.attached); + t.detach(); + assert.strictEqual(2, changed.length); + assert.strictEqual(false, changed[1].params.targetInfo.attached); + assert.strictEqual(1, detached.length); +} + +async function test() { + session.connect(); + await testTargetsDiscovery(); + await testAttach(); + await testMessages(); + session.disconnect(); + clearTargets(); + + await testChildSessionsLifetime(); + await testErrors(); + await testAutoAttach(); + await testTitleUpdate(); + clearTargets(); + await testDetach(); +} + +common.crashOnUnhandledRejection(); + +test(); From 962c61612f85a0980df44df3bb712491d61ce1e3 Mon Sep 17 00:00:00 2001 From: Eugene Ostroukhov Date: Tue, 31 Oct 2017 15:08:31 -0700 Subject: [PATCH 3/5] inspector: No built-in domain override --- doc/api/errors.md | 6 ++++++ lib/internal/errors.js | 1 + lib/internal/inspector_node_domains.js | 6 ++++++ src/inspector_agent.cc | 4 +--- 4 files changed, 14 insertions(+), 3 deletions(-) diff --git a/doc/api/errors.md b/doc/api/errors.md index d7b5de448ff8..d94ff49a3503 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -1004,6 +1004,12 @@ The `inspector` module is not available for use. While using the `inspector` module, an attempt was made to use the inspector before it was connected. + +### ERR_INSPECTOR_NO_DOMAIN_OVERRIDE + +The `ERR_INSPECTOR_NO_DOMAIN_OVERRIDE` error code indicates an attemp to +override one of the V8 built in inspector protocol domains. + ### ERR_INVALID_ARG_TYPE diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 619732df5d6f..513ec7acc61b 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -367,6 +367,7 @@ E('ERR_INSPECTOR_ALREADY_CONNECTED', 'The inspector is already connected'); E('ERR_INSPECTOR_CLOSED', 'Session was closed'); E('ERR_INSPECTOR_NOT_AVAILABLE', 'Inspector is not available'); E('ERR_INSPECTOR_NOT_CONNECTED', 'Session is not connected'); +E('ERR_INSPECTOR_NO_DOMAIN_OVERRIDE', 'Cannot override V8 protocol domain'); E('ERR_INVALID_ARG_TYPE', invalidArgType); E('ERR_INVALID_ARG_VALUE', (name, value) => `The value "${String(value)}" is invalid for argument "${name}"`); diff --git a/lib/internal/inspector_node_domains.js b/lib/internal/inspector_node_domains.js index a032a9681fe0..643518ea30b3 100644 --- a/lib/internal/inspector_node_domains.js +++ b/lib/internal/inspector_node_domains.js @@ -1,6 +1,7 @@ 'use strict'; const assert = require('assert'); +const errors = require('internal/errors'); const EventEmitter = require('events'); const util = require('util'); @@ -8,6 +9,9 @@ const { registerDispatcherFactory } = process.binding('inspector'); const domainHandlerClasses = new Map(); const SessionTerminatedSymbol = Symbol('DisposeAgent'); +// V8 does not provide a way to obtain this list programmatically +const V8_DOMAINS = [ 'Runtime', 'Debugger', 'Profiler', 'HeapProfiler', + 'Console', 'Schema' ]; function createNotificationChannel(session, domain) { return function(method, params) { @@ -420,6 +424,8 @@ function newTarget(type, title, url, attachCallback) { } function registerDomainHandlerClass(domain, factory) { + if (V8_DOMAINS.includes('domain')) + throw errors.Error('ERR_INSPECTOR_NO_DOMAIN_OVERRIDE'); domainHandlerClasses.set(domain, factory); } diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc index 709aa32152bb..2f4774010b66 100644 --- a/src/inspector_agent.cc +++ b/src/inspector_agent.cc @@ -259,10 +259,8 @@ class ChannelImpl final : public v8_inspector::V8Inspector::Channel { } void dispatchProtocolMessage(const StringView& message) { - if (v8_inspector::V8InspectorSession::canDispatchMethod(message) || - !dispatchToNodeDispatcher(message)) { + if (!dispatchToNodeDispatcher(message)) session_->dispatchProtocolMessage(message); - } } bool waitForFrontendMessage() { From 926a5c20a1c3a54e30d30ccc396e81767e6499c2 Mon Sep 17 00:00:00 2001 From: Eugene Ostroukhov Date: Tue, 7 Nov 2017 11:11:24 -0800 Subject: [PATCH 4/5] WIP - addressing code review comments, to be squashed. --- doc/guides/extending-inspector-protocol.md | 6 +++--- lib/internal/inspector_custom_domains.md | 0 lib/internal/inspector_node_domains.js | 14 +++++++++----- src/inspector_agent.cc | 4 +++- src/inspector_io.cc | 2 +- src/inspector_js_api.cc | 15 ++++++++++++--- test/common/index.js | 5 +++-- test/parallel/test-inspector-custom-domain.js | 9 +++++++-- 8 files changed, 38 insertions(+), 17 deletions(-) delete mode 100644 lib/internal/inspector_custom_domains.md diff --git a/doc/guides/extending-inspector-protocol.md b/doc/guides/extending-inspector-protocol.md index 270e9989fb3c..0ab75e74825d 100644 --- a/doc/guides/extending-inspector-protocol.md +++ b/doc/guides/extending-inspector-protocol.md @@ -7,16 +7,16 @@ details on such extension. ## Terminology * *Inspector session* - message exchange between protocol client and Node.js -* *Protocol handler* - object with a liftime of the inspector session that +* *Protocol handler* - object with a lifetime of the inspector session that interacts with the protocol client. * *Protocol domain* - 'API unit' of the inspector protocol. Defines message formats for specific and related functionality. Node.js exposes protocol domains provided by the V8. These include `Runtime`, `Debugger`, `Profiler` and `HeapProfiler`. -* *Message* - a JSON string passed between Node.js backend and a inspector +* *Message* - a JSON string passed between Node.js backend and an inspector protocol client. Message types are: request, response and notification. Protocol client only sends out requests. Messages are asynchronous and client should not -assume response will be sent immidiately after serving the request. +assume response will be sent immediately after serving the request. * *Method field* - a mandatory field in request and notification messages. This field value includes *domain* and *method* strings separated by a dot (e.g. `Debugger.pause`). diff --git a/lib/internal/inspector_custom_domains.md b/lib/internal/inspector_custom_domains.md deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/lib/internal/inspector_node_domains.js b/lib/internal/inspector_node_domains.js index 643518ea30b3..48f2c7400dcc 100644 --- a/lib/internal/inspector_node_domains.js +++ b/lib/internal/inspector_node_domains.js @@ -8,7 +8,7 @@ const util = require('util'); const { registerDispatcherFactory } = process.binding('inspector'); const domainHandlerClasses = new Map(); -const SessionTerminatedSymbol = Symbol('DisposeAgent'); +const SessionTerminatedSymbol = Symbol('SessionTerminatedSymbol'); // V8 does not provide a way to obtain this list programmatically const V8_DOMAINS = [ 'Runtime', 'Debugger', 'Profiler', 'HeapProfiler', 'Console', 'Schema' ]; @@ -23,10 +23,10 @@ function createNotificationChannel(session, domain) { } function createResponseCallback(session, id) { - let first_call = true; + let firstCall = true; return function(error, result) { - assert.ok(first_call, 'Callback should only be called once'); - first_call = false; + assert.ok(firstCall, 'Callback should only be called once'); + firstCall = false; const response = { id }; if (error) response['error'] = error; @@ -179,6 +179,10 @@ class TargetDomainHandler { }; } + setAttachToFrames() { + // Not implemented, browser specific functionality + } + _checkArgument(name, value) { if (!value) throw new Error(`Argument ${name} was not provided`); @@ -370,7 +374,7 @@ class Dispatcher { const parsed = parseSuppressingError(message); const [ domain, method ] = parsed.method ? parsed.method.split('.') : []; - if (!domain || !method) + if (!domain || !method || !method.match(/^[a-zA-Z]\w*$/)) return false; const handler = this._getOrCreateHandler(domain); diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc index 2f4774010b66..709aa32152bb 100644 --- a/src/inspector_agent.cc +++ b/src/inspector_agent.cc @@ -259,8 +259,10 @@ class ChannelImpl final : public v8_inspector::V8Inspector::Channel { } void dispatchProtocolMessage(const StringView& message) { - if (!dispatchToNodeDispatcher(message)) + if (v8_inspector::V8InspectorSession::canDispatchMethod(message) || + !dispatchToNodeDispatcher(message)) { session_->dispatchProtocolMessage(message); + } } bool waitForFrontendMessage() { diff --git a/src/inspector_io.cc b/src/inspector_io.cc index dc4d513951f9..76b624f89de2 100644 --- a/src/inspector_io.cc +++ b/src/inspector_io.cc @@ -125,7 +125,7 @@ class IoSessionDelegate : public InspectorSessionDelegate { : io_(io), session_(agent->Connect(this)) { } bool WaitForFrontendMessageWhilePaused() override; void SendMessageToFrontend(const v8_inspector::StringView& message) override; - bool IsConnected() { return !!session_; } + bool IsConnected() const { return !!session_; } void Dispatch(const StringView& message) { CHECK_NE(session_, nullptr); session_->Dispatch(message); diff --git a/src/inspector_js_api.cc b/src/inspector_js_api.cc index d4d1611f6810..9ad76a47c0f5 100644 --- a/src/inspector_js_api.cc +++ b/src/inspector_js_api.cc @@ -58,9 +58,18 @@ class JSDispatcherInterface : private AsyncWrap { Context::Scope context_scope(env_->context()); Local argument; if (message.length() > 0) { - MaybeLocal v8string = - String::NewFromTwoByte(isolate, message.characters16(), - NewStringType::kNormal, message.length()); + MaybeLocal v8string; + if (message.is8Bit()) { + v8string = String::NewFromOneByte(isolate, + message.characters8(), + NewStringType::kNormal, + message.length()); + } else { + v8string = String::NewFromTwoByte(isolate, + message.characters16(), + NewStringType::kNormal, + message.length()); + } argument = v8string.ToLocalChecked().As(); } else { argument = Null(isolate); diff --git a/test/common/index.js b/test/common/index.js index 768fab6d4c13..d02fe3359be3 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -895,6 +895,7 @@ exports.fires = function fires(promise, error, timeoutMs) { }; exports.assertRejected = function(promise, message) { - promise.then(() => Promise.reject(message || 'Promise was not rejected'), - () => {}); + return promise + .then(() => assert.fail(message || 'Promise was not rejected'), + () => { /* Expected */ }); }; diff --git a/test/parallel/test-inspector-custom-domain.js b/test/parallel/test-inspector-custom-domain.js index 6882b45f442d..591d1985bb02 100644 --- a/test/parallel/test-inspector-custom-domain.js +++ b/test/parallel/test-inspector-custom-domain.js @@ -35,6 +35,10 @@ class TestDomainHandler { return { a }; } + _privateMethod() { + return true; + } + [SessionTerminatedSymbol]() { this.done = true; } @@ -46,7 +50,7 @@ async function test() { const post = util.promisify(session.post.bind(session)); try { await post('Test.methodA'); - assert.fails('Exception must have been thrown'); + assert.fail('Exception must have been thrown'); } catch (e) { assert.strictEqual(-32601, e.code); } @@ -55,7 +59,7 @@ async function test() { assert.strictEqual(3, await post('Test.methodA', { a: 1, b: 2 })); try { await post('Test.methodB'); - assert.fails(); + assert.fail(); } catch (e) { assert.strictEqual('Booom!', e); } @@ -88,6 +92,7 @@ async function test() { assert.deepStrictEqual({ a: 0 }, await post1('Test.methodD')); assert.deepStrictEqual({ a: 1 }, await post1('Test.methodD')); assert.deepStrictEqual({ a: 2 }, await post1('Test.methodD')); + await common.assertRejected(post1('Test._privateMethod')); } common.crashOnUnhandledRejection(); From 3901aa595c139c9cfae08c6115bf6a8e5dfca5e0 Mon Sep 17 00:00:00 2001 From: Eugene Ostroukhov Date: Tue, 28 Nov 2017 11:40:56 -0800 Subject: [PATCH 5/5] (To be squashed) Reject malformed method name --- lib/internal/inspector_node_domains.js | 5 +++-- test/parallel/test-inspector-custom-domain.js | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/internal/inspector_node_domains.js b/lib/internal/inspector_node_domains.js index 48f2c7400dcc..869cfc084851 100644 --- a/lib/internal/inspector_node_domains.js +++ b/lib/internal/inspector_node_domains.js @@ -372,9 +372,10 @@ class Dispatcher { return; } const parsed = parseSuppressingError(message); - const [ domain, method ] = parsed.method ? parsed.method.split('.') : []; + const [ domain, method, tail ] = + parsed.method ? parsed.method.split('.') : []; - if (!domain || !method || !method.match(/^[a-zA-Z]\w*$/)) + if (!domain || !method || !method.match(/^[a-zA-Z]\w*$/) || tail) return false; const handler = this._getOrCreateHandler(domain); diff --git a/test/parallel/test-inspector-custom-domain.js b/test/parallel/test-inspector-custom-domain.js index 591d1985bb02..d024f5c41fdd 100644 --- a/test/parallel/test-inspector-custom-domain.js +++ b/test/parallel/test-inspector-custom-domain.js @@ -92,6 +92,7 @@ async function test() { assert.deepStrictEqual({ a: 0 }, await post1('Test.methodD')); assert.deepStrictEqual({ a: 1 }, await post1('Test.methodD')); assert.deepStrictEqual({ a: 2 }, await post1('Test.methodD')); + await common.assertRejected(post1('Test.methodD.methodE')); await common.assertRejected(post1('Test._privateMethod')); }