inspector: migrate errors from C++ to JS

Assign a code to a user-facing error.
Turn other internal-only errors to checks.

PR-URL: #19387
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

Author: targos

lib/inspector.js

@@ -30,9 +30,13 @@ class Session extends EventEmitter {
 
   connect() {
     if (this[connectionSymbol])
-      throw new ERR_INSPECTOR_ALREADY_CONNECTED();
-    this[connectionSymbol] =
-        new Connection((message) => this[onMessageSymbol](message));
+      throw new ERR_INSPECTOR_ALREADY_CONNECTED('The inspector session');
+    const connection =
+      new Connection((message) => this[onMessageSymbol](message));
+    if (connection.sessionAttached) {
+      throw new ERR_INSPECTOR_ALREADY_CONNECTED('Another inspector session');
+    }
+    this[connectionSymbol] = connection;
   }
 
   [onMessageSymbol](message) {

lib/internal/errors.js

@@ -739,8 +739,7 @@ E('ERR_HTTP_INVALID_STATUS_CODE', 'Invalid status code: %s', RangeError);
 E('ERR_HTTP_TRAILER_INVALID',
   'Trailers are invalid with this transfer encoding', Error);
 E('ERR_INDEX_OUT_OF_RANGE', 'Index out of range', RangeError);
-E('ERR_INSPECTOR_ALREADY_CONNECTED',
-  'The inspector is already connected', Error);
+E('ERR_INSPECTOR_ALREADY_CONNECTED', '%s is already connected', Error);
 E('ERR_INSPECTOR_CLOSED', 'Session was closed', Error);
 E('ERR_INSPECTOR_NOT_AVAILABLE', 'Inspector is not available', Error);
 E('ERR_INSPECTOR_NOT_CONNECTED', 'Session is not connected', Error);

src/inspector_js_api.cc

@@ -9,6 +9,7 @@ namespace node {
 namespace inspector {
 namespace {
 
+using v8::Boolean;
 using v8::Context;
 using v8::Function;
 using v8::FunctionCallbackInfo;
@@ -78,7 +79,11 @@ class JSBindingsConnection : public AsyncWrap {
 
     Agent* inspector = env->inspector_agent();
     if (inspector->delegate() != nullptr) {
-      env->ThrowTypeError("Session is already attached");
+      // This signals JS code that it has to throw an error.
+      Local<String> session_attached =
+          FIXED_ONE_BYTE_STRING(env->isolate(), "sessionAttached");
+      wrap->Set(env->context(), session_attached,
+                Boolean::New(env->isolate(), true)).ToChecked();
       return;
     }
     inspector->Connect(&delegate_);
@@ -95,10 +100,7 @@ class JSBindingsConnection : public AsyncWrap {
 
   static void New(const FunctionCallbackInfo<Value>& info) {
     Environment* env = Environment::GetCurrent(info);
-    if (!info[0]->IsFunction()) {
-      env->ThrowTypeError("Message callback is required");
-      return;
-    }
+    CHECK(info[0]->IsFunction());
     Local<Function> callback = info[0].As<Function>();
     new JSBindingsConnection(env, info.This(), callback);
   }
@@ -121,10 +123,7 @@ class JSBindingsConnection : public AsyncWrap {
     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;
-    }
+    CHECK(info[0]->IsString());
 
     session->CheckIsCurrent();
     Agent* inspector = env->inspector_agent();
@@ -143,10 +142,9 @@ void AddCommandLineAPI(const FunctionCallbackInfo<Value>& info) {
   auto env = Environment::GetCurrent(info);
   Local<Context> context = env->context();
 
-  if (info.Length() != 2 || !info[0]->IsString()) {
-    return env->ThrowTypeError("inspector.addCommandLineAPI takes "
-        "exactly 2 arguments: a string and a value.");
-  }
+  // inspector.addCommandLineAPI takes 2 arguments: a string and a value.
+  CHECK_EQ(info.Length(), 2);
+  CHECK(info[0]->IsString());
 
   Local<Object> console_api = env->inspector_console_api_object();
   console_api->Set(context, info[0], info[1]).FromJust();

test/sequential/test-inspector-module.js

@@ -51,7 +51,17 @@ common.expectsError(
   {
     code: 'ERR_INSPECTOR_ALREADY_CONNECTED',
     type: Error,
-    message: 'The inspector is already connected'
+    message: 'The inspector session is already connected'
+  }
+);
+
+const session2 = new Session();
+common.expectsError(
+  () => session2.connect(),
+  {
+    code: 'ERR_INSPECTOR_ALREADY_CONNECTED',
+    type: Error,
+    message: 'Another inspector session is already connected'
   }
 );