Skip to content

Commit fe1ceb9

Browse files
bnoordhuisgibfahn
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 cbe1e5c commit fe1ceb9

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
@@ -2034,14 +2034,11 @@ NO_RETURN void Assert(const char* const (*args)[4]) {
20342034
auto message = (*args)[2];
20352035
auto function = (*args)[3];
20362036

2037-
char exepath[256];
2038-
size_t exepath_size = sizeof(exepath);
2039-
if (uv_exepath(exepath, &exepath_size))
2040-
snprintf(exepath, sizeof(exepath), "node");
2041-
2042-
fprintf(stderr, "%s[%u]: %s:%s:%s%s Assertion `%s' failed.\n",
2043-
exepath, GetProcessId(), filename, linenum,
2044-
function, *function ? ":" : "", message);
2037+
char name[1024];
2038+
GetHumanReadableProcessName(&name);
2039+
2040+
fprintf(stderr, "%s: %s:%s:%s%s Assertion `%s' failed.\n",
2041+
name, filename, linenum, function, *function ? ":" : "", message);
20452042
fflush(stderr);
20462043

20472044
Abort();

src/node_internals.h

+3
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,9 @@ void RegisterSignalHandler(int signal,
146146
uint32_t GetProcessId();
147147
bool SafeGetenv(const char* key, std::string* text);
148148

149+
std::string GetHumanReadableProcessName();
150+
void GetHumanReadableProcessName(char (*name)[1024]);
151+
149152
template <typename T, size_t N>
150153
constexpr size_t arraysize(const T(&)[N]) { return N; }
151154

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)