Skip to content

Commit 362b8c7

Browse files
bnoordhuisMylesBorins
authored andcommitted
src: inspector context name = program title + pid
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 <[email protected]> Reviewed-By: Eugene Ostroukhov <[email protected]> Reviewed-By: Timothy Gu <[email protected]>
1 parent 7ecec67 commit 362b8c7

File tree

6 files changed

+35
-24
lines changed

6 files changed

+35
-24
lines changed

src/inspector_agent.cc

+2-1
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,8 @@ class NodeInspectorClient : public V8InspectorClient {
302302
: env_(env), platform_(platform), terminated_(false),
303303
running_nested_loop_(false) {
304304
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());
306307
}
307308

308309
void runMessageLoopOnPause(int context_group_id) override {

src/inspector_io.cc

+1-12
Original file line numberDiff line numberDiff line change
@@ -26,17 +26,6 @@ using v8_inspector::StringView;
2626
template<typename Transport>
2727
using TransportAndIo = std::pair<Transport*, InspectorIo*>;
2828

29-
std::string GetProcessTitle() {
30-
char title[2048];
31-
int err = uv_get_process_title(title, sizeof(title));
32-
if (err == 0) {
33-
return title;
34-
} else {
35-
// Title is too long, or could not be retrieved.
36-
return "Node.js";
37-
}
38-
}
39-
4029
std::string ScriptPath(uv_loop_t* loop, const std::string& script_name) {
4130
std::string script_path;
4231

@@ -484,7 +473,7 @@ std::vector<std::string> InspectorIoDelegate::GetTargetIds() {
484473
}
485474

486475
std::string InspectorIoDelegate::GetTargetTitle(const std::string& id) {
487-
return script_name_.empty() ? GetProcessTitle() : script_name_;
476+
return script_name_.empty() ? GetHumanReadableProcessName() : script_name_;
488477
}
489478

490479
std::string InspectorIoDelegate::GetTargetUrl(const std::string& id) {

src/node.cc

+5-8
Original file line numberDiff line numberDiff line change
@@ -1989,14 +1989,11 @@ NO_RETURN void Assert(const char* const (*args)[4]) {
19891989
auto message = (*args)[2];
19901990
auto function = (*args)[3];
19911991

1992-
char exepath[256];
1993-
size_t exepath_size = sizeof(exepath);
1994-
if (uv_exepath(exepath, &exepath_size))
1995-
snprintf(exepath, sizeof(exepath), "node");
1996-
1997-
fprintf(stderr, "%s[%u]: %s:%s:%s%s Assertion `%s' failed.\n",
1998-
exepath, GetProcessId(), filename, linenum,
1999-
function, *function ? ":" : "", message);
1992+
char name[1024];
1993+
GetHumanReadableProcessName(&name);
1994+
1995+
fprintf(stderr, "%s: %s:%s:%s%s Assertion `%s' failed.\n",
1996+
name, filename, linenum, function, *function ? ":" : "", message);
20001997
fflush(stderr);
20011998

20021999
Abort();

src/node_internals.h

+3
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,9 @@ void RegisterSignalHandler(int signal,
169169
uint32_t GetProcessId();
170170
bool SafeGetenv(const char* key, std::string* text);
171171

172+
std::string GetHumanReadableProcessName();
173+
void GetHumanReadableProcessName(char (*name)[1024]);
174+
172175
template <typename T, size_t N>
173176
constexpr size_t arraysize(const T(&)[N]) { return N; }
174177

src/util.cc

+12
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,18 @@ void LowMemoryNotification() {
113113
}
114114
}
115115

116+
std::string GetHumanReadableProcessName() {
117+
char name[1024];
118+
GetHumanReadableProcessName(&name);
119+
return name;
120+
}
121+
122+
void GetHumanReadableProcessName(char (*name)[1024]) {
123+
char title[1024] = "Node.js";
124+
uv_get_process_title(title, sizeof(title));
125+
snprintf(*name, sizeof(*name), "%s[%u]", title, GetProcessId());
126+
}
127+
116128
uint32_t GetProcessId() {
117129
#ifdef _WIN32
118130
return GetCurrentProcessId();

test/sequential/test-inspector-contexts.js

+12-3
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,18 @@ async function testContextCreatedAndDestroyed() {
2323

2424
session.post('Runtime.enable');
2525
let contextCreated = await mainContextPromise;
26-
strictEqual('Node.js Main Context',
27-
contextCreated.params.context.name,
28-
JSON.stringify(contextCreated));
26+
{
27+
const { name } = contextCreated.params.context;
28+
if (common.isSunOS || common.isWindows) {
29+
// uv_get_process_title() is unimplemented on Solaris-likes, it returns
30+
// an empy string. On the Windows CI buildbots it returns "Administrator:
31+
// Windows PowerShell[42]" because of a GetConsoleTitle() quirk. Not much
32+
// we can do about either, just verify that it contains the PID.
33+
strictEqual(name.includes(`[${process.pid}]`), true);
34+
} else {
35+
strictEqual(`${process.argv0}[${process.pid}]`, name);
36+
}
37+
}
2938

3039
const secondContextCreatedPromise =
3140
notificationPromise('Runtime.executionContextCreated');

0 commit comments

Comments
 (0)