src: inspector context name = program title + pid

bnoordhuis · gibfahn · commit fe1ceb984290 · 2017-12-19T20:41:16.000Z
Report (for example) "node[1337]" as the human-readable name rather than the more generic and less helpful "Node.js Main Context." While not perfect yet, it should be an improvement to people that debug multiple processes from DevTools, VS Code, etc. PR-URL: #17087 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com>
diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc
@@ -302,7 +302,8 @@ class NodeInspectorClient : public V8InspectorClient {
       : env_(env), platform_(platform), terminated_(false),
         running_nested_loop_(false) {
     client_ = V8Inspector::create(env->isolate(), this);
-    contextCreated(env->context(), "Node.js Main Context");
+    // TODO(bnoordhuis) Make name configurable from src/node.cc.
+    contextCreated(env->context(), GetHumanReadableProcessName());
   }
 
   void runMessageLoopOnPause(int context_group_id) override {
diff --git a/src/inspector_io.cc b/src/inspector_io.cc
@@ -26,17 +26,6 @@ using v8_inspector::StringView;
 template<typename Transport>
 using TransportAndIo = std::pair<Transport*, InspectorIo*>;
 
-std::string GetProcessTitle() {
-  char title[2048];
-  int err = uv_get_process_title(title, sizeof(title));
-  if (err == 0) {
-    return title;
-  } else {
-    // Title is too long, or could not be retrieved.
-    return "Node.js";
-  }
-}
-
 std::string ScriptPath(uv_loop_t* loop, const std::string& script_name) {
   std::string script_path;
 
@@ -484,7 +473,7 @@ std::vector<std::string> InspectorIoDelegate::GetTargetIds() {
 }
 
 std::string InspectorIoDelegate::GetTargetTitle(const std::string& id) {
-  return script_name_.empty() ? GetProcessTitle() : script_name_;
+  return script_name_.empty() ? GetHumanReadableProcessName() : script_name_;
 }
 
 std::string InspectorIoDelegate::GetTargetUrl(const std::string& id) {
diff --git a/src/node.cc b/src/node.cc
@@ -2034,14 +2034,11 @@ NO_RETURN void Assert(const char* const (*args)[4]) {
   auto message = (*args)[2];
   auto function = (*args)[3];
 
-  char exepath[256];
-  size_t exepath_size = sizeof(exepath);
-  if (uv_exepath(exepath, &exepath_size))
-    snprintf(exepath, sizeof(exepath), "node");
-
-  fprintf(stderr, "%s[%u]: %s:%s:%s%s Assertion `%s' failed.\n",
-          exepath, GetProcessId(), filename, linenum,
-          function, *function ? ":" : "", message);
+  char name[1024];
+  GetHumanReadableProcessName(&name);
+
+  fprintf(stderr, "%s: %s:%s:%s%s Assertion `%s' failed.\n",
+          name, filename, linenum, function, *function ? ":" : "", message);
   fflush(stderr);
 
   Abort();
diff --git a/src/node_internals.h b/src/node_internals.h
@@ -146,6 +146,9 @@ void RegisterSignalHandler(int signal,
 uint32_t GetProcessId();
 bool SafeGetenv(const char* key, std::string* text);
 
+std::string GetHumanReadableProcessName();
+void GetHumanReadableProcessName(char (*name)[1024]);
+
 template <typename T, size_t N>
 constexpr size_t arraysize(const T(&)[N]) { return N; }
 
diff --git a/src/util.cc b/src/util.cc
@@ -113,6 +113,18 @@ void LowMemoryNotification() {
   }
 }
 
+std::string GetHumanReadableProcessName() {
+  char name[1024];
+  GetHumanReadableProcessName(&name);
+  return name;
+}
+
+void GetHumanReadableProcessName(char (*name)[1024]) {
+  char title[1024] = "Node.js";
+  uv_get_process_title(title, sizeof(title));
+  snprintf(*name, sizeof(*name), "%s[%u]", title, GetProcessId());
+}
+
 uint32_t GetProcessId() {
 #ifdef _WIN32
   return GetCurrentProcessId();
diff --git a/test/sequential/test-inspector-contexts.js b/test/sequential/test-inspector-contexts.js
@@ -23,9 +23,18 @@ async function testContextCreatedAndDestroyed() {
 
   session.post('Runtime.enable');
   let contextCreated = await mainContextPromise;
-  strictEqual('Node.js Main Context',
-              contextCreated.params.context.name,
-              JSON.stringify(contextCreated));
+  {
+    const { name } = contextCreated.params.context;
+    if (common.isSunOS || common.isWindows) {
+      // uv_get_process_title() is unimplemented on Solaris-likes, it returns
+      // an empy string.  On the Windows CI buildbots it returns "Administrator:
+      // Windows PowerShell[42]" because of a GetConsoleTitle() quirk. Not much
+      // we can do about either, just verify that it contains the PID.
+      strictEqual(name.includes(`[${process.pid}]`), true);
+    } else {
+      strictEqual(`${process.argv0}[${process.pid}]`, name);
+    }
+  }
 
   const secondContextCreatedPromise =
       notificationPromise('Runtime.executionContextCreated');

Original file line number	Diff line number	Diff line change
`@@ -302,7 +302,8 @@ class NodeInspectorClient : public V8InspectorClient {`
`302`	`302`	`: env_(env), platform_(platform), terminated_(false),`
`303`	`303`	`running_nested_loop_(false) {`
`304`	`304`	`client_ = V8Inspector::create(env->isolate(), this);`
`305`		`- contextCreated(env->context(), "Node.js Main Context");`
	`305`	`+ // TODO(bnoordhuis) Make name configurable from src/node.cc.`
	`306`	`+ contextCreated(env->context(), GetHumanReadableProcessName());`
`306`	`307`	`}`
`307`	`308`
`308`	`309`	`void runMessageLoopOnPause(int context_group_id) override {`