From d0bf9dd5769d44f3dbd46ae4c5a6af57c8454e99 Mon Sep 17 00:00:00 2001 From: Chengzhong Wu Date: Mon, 27 Jan 2025 00:05:56 +0000 Subject: [PATCH 1/2] src: add a hard dependency v8_inspector_headers A GYP hard dependency instructs GYP to not remove the dependency link between two static library targets in its generated output. This allows V8 dependents to include V8 inspector protocol headers. --- src/inspector/node_inspector.gypi | 2 ++ tools/v8_gypfiles/inspector.gypi | 42 ++---------------------- tools/v8_gypfiles/v8.gyp | 53 +++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 40 deletions(-) diff --git a/src/inspector/node_inspector.gypi b/src/inspector/node_inspector.gypi index 3ae714b51407a4..60767733113a74 100644 --- a/src/inspector/node_inspector.gypi +++ b/src/inspector/node_inspector.gypi @@ -2,6 +2,7 @@ 'variables': { 'protocol_tool_path': '../../deps/inspector_protocol', 'jinja_dir': '../../tools/inspector_protocol', + 'v8_gypfiles_dir': '../../tools/v8_gypfiles', 'node_inspector_sources': [ 'src/inspector_agent.cc', 'src/inspector_io.cc', @@ -74,6 +75,7 @@ ], 'dependencies': [ '<(protocol_tool_path)/inspector_protocol.gyp:crdtp', + '<(v8_gypfiles_dir)/v8.gyp:v8_inspector_headers', ], 'actions': [ { diff --git a/tools/v8_gypfiles/inspector.gypi b/tools/v8_gypfiles/inspector.gypi index aed336e76ee6b0..517823767d8ff5 100644 --- a/tools/v8_gypfiles/inspector.gypi +++ b/tools/v8_gypfiles/inspector.gypi @@ -129,6 +129,8 @@ '<(inspector_protocol_path)/crdtp/span.h', '<(inspector_protocol_path)/crdtp/status.cc', '<(inspector_protocol_path)/crdtp/status.h', + + '<@(inspector_generated_sources)', ], 'v8_inspector_js_protocol': '<(V8_ROOT)/include/js_protocol.pdl', }, @@ -136,44 +138,4 @@ '<(inspector_generated_output_root)', '<(inspector_protocol_path)', ], - 'actions': [ - { - 'action_name': 'protocol_compatibility', - 'inputs': [ - '<(v8_inspector_js_protocol)', - ], - 'outputs': [ - '<@(inspector_generated_output_root)/src/js_protocol.stamp', - ], - 'action': [ - '<(python)', - '<(inspector_protocol_path)/check_protocol_compatibility.py', - '--stamp', '<@(_outputs)', - '<@(_inputs)', - ], - 'message': 'Checking inspector protocol compatibility', - }, - { - 'action_name': 'protocol_generated_sources', - 'inputs': [ - '<(v8_inspector_js_protocol)', - '<(inspector_path)/inspector_protocol_config.json', - '<@(inspector_protocol_files)', - ], - 'outputs': [ - '<@(inspector_generated_sources)', - ], - 'process_outputs_as_sources': 1, - 'action': [ - '<(python)', - '<(inspector_protocol_path)/code_generator.py', - '--jinja_dir', '<(V8_ROOT)/third_party', - '--output_base', '<(inspector_generated_output_root)/src/inspector', - '--config', '<(inspector_path)/inspector_protocol_config.json', - '--config_value', 'protocol.path=<(v8_inspector_js_protocol)', - '--inspector_protocol_dir', '<(inspector_protocol_path)', - ], - 'message': 'Generating inspector protocol sources from protocol json', - }, - ], } diff --git a/tools/v8_gypfiles/v8.gyp b/tools/v8_gypfiles/v8.gyp index 9ccab9214a650c..f9399b216a81c5 100644 --- a/tools/v8_gypfiles/v8.gyp +++ b/tools/v8_gypfiles/v8.gyp @@ -1017,6 +1017,58 @@ }], ], }, # v8_compiler_for_mksnapshot + { + 'target_name': 'v8_inspector_headers', + 'type': 'none', + 'toolsets': ['host', 'target'], + 'hard_dependency': 1, + 'includes': ['inspector.gypi'], + 'direct_dependent_settings': { + 'include_dirs': [ + '<(inspector_generated_output_root)/include', + ], + }, + 'actions': [ + { + 'action_name': 'protocol_compatibility', + 'inputs': [ + '<(v8_inspector_js_protocol)', + ], + 'outputs': [ + '<@(inspector_generated_output_root)/src/js_protocol.stamp', + ], + 'action': [ + '<(python)', + '<(inspector_protocol_path)/check_protocol_compatibility.py', + '--stamp', '<@(_outputs)', + '<@(_inputs)', + ], + 'message': 'Checking inspector protocol compatibility', + }, + { + 'action_name': 'protocol_generated_sources', + 'inputs': [ + '<(v8_inspector_js_protocol)', + '<(inspector_path)/inspector_protocol_config.json', + '<@(inspector_protocol_files)', + ], + 'outputs': [ + '<@(inspector_generated_sources)', + ], + 'process_outputs_as_sources': 1, + 'action': [ + '<(python)', + '<(inspector_protocol_path)/code_generator.py', + '--jinja_dir', '<(V8_ROOT)/third_party', + '--output_base', '<(inspector_generated_output_root)/src/inspector', + '--config', '<(inspector_path)/inspector_protocol_config.json', + '--config_value', 'protocol.path=<(v8_inspector_js_protocol)', + '--inspector_protocol_dir', '<(inspector_protocol_path)', + ], + 'message': 'Generating inspector protocol sources from protocol json', + }, + ], + }, # v8_inspector_headers { 'target_name': 'v8_base_without_compiler', 'type': 'static_library', @@ -1026,6 +1078,7 @@ 'v8_bigint', 'v8_headers', 'v8_heap_base', + 'v8_inspector_headers', 'v8_libbase', 'v8_shared_internal_headers', 'v8_version', From 314077cc0709256bf15a0af0c6e65209598961e2 Mon Sep 17 00:00:00 2001 From: Chengzhong Wu Date: Wed, 5 Feb 2025 18:03:57 +0000 Subject: [PATCH 2/2] inspector: add Network.Initiator in inspector protocol Add initiator stack trace in inspector network events, reflecting the location where the script created the request. The `http.client.request.created` event is closer to where user code creates the http request, and correctly reflects which script initiated the request. --- lib/internal/inspector/network_http.js | 8 ++--- lib/internal/inspector/network_undici.js | 8 ++--- src/inspector/network_agent.cc | 13 +++++++-- src/inspector/network_agent.h | 4 ++- src/inspector/network_inspector.cc | 5 ++-- src/inspector/network_inspector.h | 3 +- src/inspector/node_inspector.gypi | 1 + src/inspector/node_protocol.pdl | 29 +++++++++++++++++++ src/inspector/node_protocol_config.json | 11 +++++++ src/inspector_agent.cc | 3 +- test/parallel/test-inspector-network-fetch.js | 11 +++++++ test/parallel/test-inspector-network-http.js | 11 +++++++ 12 files changed, 92 insertions(+), 15 deletions(-) diff --git a/lib/internal/inspector/network_http.js b/lib/internal/inspector/network_http.js index 16669f308f3a8e..00b671cc4f8e7a 100644 --- a/lib/internal/inspector/network_http.js +++ b/lib/internal/inspector/network_http.js @@ -44,11 +44,11 @@ const convertHeaderObject = (headers = {}) => { }; /** - * When a client request starts, emit Network.requestWillBeSent event. + * When a client request is created, emit Network.requestWillBeSent event. * https://chromedevtools.github.io/devtools-protocol/1-3/Network/#event-requestWillBeSent * @param {{ request: import('http').ClientRequest }} event */ -function onClientRequestStart({ request }) { +function onClientRequestCreated({ request }) { request[kInspectorRequestId] = getNextRequestId(); const { 0: host, 1: headers } = convertHeaderObject(request.getHeaders()); @@ -115,13 +115,13 @@ function onClientResponseFinish({ request, response }) { } function enable() { - dc.subscribe('http.client.request.start', onClientRequestStart); + dc.subscribe('http.client.request.created', onClientRequestCreated); dc.subscribe('http.client.request.error', onClientRequestError); dc.subscribe('http.client.response.finish', onClientResponseFinish); } function disable() { - dc.unsubscribe('http.client.request.start', onClientRequestStart); + dc.unsubscribe('http.client.request.created', onClientRequestCreated); dc.unsubscribe('http.client.request.error', onClientRequestError); dc.unsubscribe('http.client.response.finish', onClientResponseFinish); } diff --git a/lib/internal/inspector/network_undici.js b/lib/internal/inspector/network_undici.js index 7afc5970117127..636e2b21b45b4a 100644 --- a/lib/internal/inspector/network_undici.js +++ b/lib/internal/inspector/network_undici.js @@ -129,10 +129,10 @@ function enable() { } function disable() { - dc.subscribe('undici:request:create', onClientRequestStart); - dc.subscribe('undici:request:error', onClientRequestError); - dc.subscribe('undici:request:headers', onClientResponseHeaders); - dc.subscribe('undici:request:trailers', onClientResponseFinish); + dc.unsubscribe('undici:request:create', onClientRequestStart); + dc.unsubscribe('undici:request:error', onClientRequestError); + dc.unsubscribe('undici:request:headers', onClientResponseHeaders); + dc.unsubscribe('undici:request:trailers', onClientResponseFinish); } module.exports = { diff --git a/src/inspector/network_agent.cc b/src/inspector/network_agent.cc index 68a1bd4198fde4..157d0c52cf98e2 100644 --- a/src/inspector/network_agent.cc +++ b/src/inspector/network_agent.cc @@ -29,8 +29,9 @@ std::unique_ptr createResponse( .build(); } -NetworkAgent::NetworkAgent(NetworkInspector* inspector) - : inspector_(inspector) { +NetworkAgent::NetworkAgent(NetworkInspector* inspector, + v8_inspector::V8Inspector* v8_inspector) + : inspector_(inspector), v8_inspector_(v8_inspector) { event_notifier_map_["requestWillBeSent"] = &NetworkAgent::requestWillBeSent; event_notifier_map_["responseReceived"] = &NetworkAgent::responseReceived; event_notifier_map_["loadingFailed"] = &NetworkAgent::loadingFailed; @@ -75,6 +76,13 @@ void NetworkAgent::requestWillBeSent( String method; request->getString("method", &method); + std::unique_ptr initiator = + Network::Initiator::create() + .setType(Network::Initiator::TypeEnum::Script) + .setStack( + v8_inspector_->captureStackTrace(true)->buildInspectorObject(0)) + .build(); + ErrorSupport errors; errors.Push(); errors.SetName("headers"); @@ -86,6 +94,7 @@ void NetworkAgent::requestWillBeSent( frontend_->requestWillBeSent(request_id, createRequest(url, method, std::move(headers)), + std::move(initiator), timestamp, wall_time); } diff --git a/src/inspector/network_agent.h b/src/inspector/network_agent.h index 8d0e71c49b440d..67994e19475d65 100644 --- a/src/inspector/network_agent.h +++ b/src/inspector/network_agent.h @@ -14,7 +14,8 @@ namespace protocol { class NetworkAgent : public Network::Backend { public: - explicit NetworkAgent(NetworkInspector* inspector); + explicit NetworkAgent(NetworkInspector* inspector, + v8_inspector::V8Inspector* v8_inspector); void Wire(UberDispatcher* dispatcher); @@ -35,6 +36,7 @@ class NetworkAgent : public Network::Backend { private: NetworkInspector* inspector_; + v8_inspector::V8Inspector* v8_inspector_; std::shared_ptr frontend_; using EventNotifier = void (NetworkAgent::*)(std::unique_ptr); diff --git a/src/inspector/network_inspector.cc b/src/inspector/network_inspector.cc index a03a66d461e527..2a3488b8ffd854 100644 --- a/src/inspector/network_inspector.cc +++ b/src/inspector/network_inspector.cc @@ -3,9 +3,10 @@ namespace node { namespace inspector { -NetworkInspector::NetworkInspector(Environment* env) +NetworkInspector::NetworkInspector(Environment* env, + v8_inspector::V8Inspector* v8_inspector) : enabled_(false), env_(env) { - network_agent_ = std::make_unique(this); + network_agent_ = std::make_unique(this, v8_inspector); } NetworkInspector::~NetworkInspector() { network_agent_.reset(); diff --git a/src/inspector/network_inspector.h b/src/inspector/network_inspector.h index 1a30997bad98f1..a77260e3815a5c 100644 --- a/src/inspector/network_inspector.h +++ b/src/inspector/network_inspector.h @@ -11,7 +11,8 @@ namespace inspector { class NetworkInspector { public: - explicit NetworkInspector(Environment* env); + explicit NetworkInspector(Environment* env, + v8_inspector::V8Inspector* v8_inspector); ~NetworkInspector(); void Wire(protocol::UberDispatcher* dispatcher); diff --git a/src/inspector/node_inspector.gypi b/src/inspector/node_inspector.gypi index 60767733113a74..db0709dfcf17ed 100644 --- a/src/inspector/node_inspector.gypi +++ b/src/inspector/node_inspector.gypi @@ -97,6 +97,7 @@ 'action_name': 'node_protocol_generated_sources', 'inputs': [ 'node_protocol_config.json', + 'node_protocol.pdl', '<(SHARED_INTERMEDIATE_DIR)/src/node_protocol.json', '<@(node_protocol_files)', '<(protocol_tool_path)/code_generator.py', diff --git a/src/inspector/node_protocol.pdl b/src/inspector/node_protocol.pdl index d5b50dc81b40f1..2fe6634ad7e278 100644 --- a/src/inspector/node_protocol.pdl +++ b/src/inspector/node_protocol.pdl @@ -101,6 +101,8 @@ experimental domain NodeWorker # Partial support for Network domain of ChromeDevTools Protocol. # https://chromedevtools.github.io/devtools-protocol/tot/Network experimental domain Network + depends on Runtime + # Resource type as it was perceived by the rendering engine. type ResourceType extends string enum @@ -132,6 +134,31 @@ experimental domain Network # Monotonically increasing time in seconds since an arbitrary point in the past. type MonotonicTime extends number + # Information about the request initiator. + type Initiator extends object + properties + # Type of this initiator. + enum type + parser + script + preload + SignedExchange + preflight + other + # Initiator JavaScript stack trace, set for Script only. + # Requires the Debugger domain to be enabled. + optional Runtime.StackTrace stack + # Initiator URL, set for Parser type or for Script type (when script is importing module) or for SignedExchange type. + optional string url + # Initiator line number, set for Parser type or for Script type (when script is importing + # module) (0-based). + optional number lineNumber + # Initiator column number, set for Parser type or for Script type (when script is importing + # module) (0-based). + optional number columnNumber + # Set if another request triggered this request (e.g. preflight). + optional RequestId requestId + # HTTP request data. type Request extends object properties @@ -163,6 +190,8 @@ experimental domain Network RequestId requestId # Request data. Request request + # Request initiator. + Initiator initiator # Timestamp. MonotonicTime timestamp # Timestamp. diff --git a/src/inspector/node_protocol_config.json b/src/inspector/node_protocol_config.json index 959a1f4eb1b67a..6c3cbdd4cb95a7 100644 --- a/src/inspector/node_protocol_config.json +++ b/src/inspector/node_protocol_config.json @@ -5,6 +5,17 @@ "output": "node/inspector/protocol", "namespace": ["node", "inspector", "protocol"] }, + "imported": { + "path": "../../deps/v8/include/js_protocol.pdl", + "header": "", + "namespace": ["v8_inspector", "protocol"], + "options": [ + { + "domain": "Runtime", + "imported": ["StackTrace"] + } + ] + }, "exported": { "package": "include/inspector", "output": "../../include/inspector", diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc index be767c1825d64f..f48be5ea158a53 100644 --- a/src/inspector_agent.cc +++ b/src/inspector_agent.cc @@ -239,7 +239,8 @@ class ChannelImpl final : public v8_inspector::V8Inspector::Channel, } runtime_agent_ = std::make_unique(); runtime_agent_->Wire(node_dispatcher_.get()); - network_inspector_ = std::make_unique(env); + network_inspector_ = + std::make_unique(env, inspector.get()); network_inspector_->Wire(node_dispatcher_.get()); } diff --git a/test/parallel/test-inspector-network-fetch.js b/test/parallel/test-inspector-network-fetch.js index 26f6d52ff40694..88585ab72bac75 100644 --- a/test/parallel/test-inspector-network-fetch.js +++ b/test/parallel/test-inspector-network-fetch.js @@ -65,6 +65,13 @@ const terminate = () => { inspector.close(); }; +function findFrameInInitiator(scriptName, initiator) { + const frame = initiator.stack.callFrames.find((it) => { + return it.url === scriptName; + }); + return frame; +} + const testHttpGet = () => new Promise((resolve, reject) => { session.on('Network.requestWillBeSent', common.mustCall(({ params }) => { assert.ok(params.requestId.startsWith('node-network-event-')); @@ -77,6 +84,10 @@ const testHttpGet = () => new Promise((resolve, reject) => { assert.strictEqual(params.request.headers['x-header1'], 'value1, value2'); assert.strictEqual(typeof params.timestamp, 'number'); assert.strictEqual(typeof params.wallTime, 'number'); + + assert.strictEqual(typeof params.initiator, 'object'); + assert.strictEqual(params.initiator.type, 'script'); + assert.ok(findFrameInInitiator(__filename, params.initiator)); })); session.on('Network.responseReceived', common.mustCall(({ params }) => { assert.ok(params.requestId.startsWith('node-network-event-')); diff --git a/test/parallel/test-inspector-network-http.js b/test/parallel/test-inspector-network-http.js index e1e987cdd71e28..a02329891e1208 100644 --- a/test/parallel/test-inspector-network-http.js +++ b/test/parallel/test-inspector-network-http.js @@ -64,6 +64,13 @@ const terminate = () => { inspector.close(); }; +function findFrameInInitiator(scriptName, initiator) { + const frame = initiator.stack.callFrames.find((it) => { + return it.url === scriptName; + }); + return frame; +} + function verifyRequestWillBeSent({ method, params }, expect) { assert.strictEqual(method, 'Network.requestWillBeSent'); @@ -78,6 +85,10 @@ function verifyRequestWillBeSent({ method, params }, expect) { assert.strictEqual(typeof params.timestamp, 'number'); assert.strictEqual(typeof params.wallTime, 'number'); + assert.strictEqual(typeof params.initiator, 'object'); + assert.strictEqual(params.initiator.type, 'script'); + assert.ok(findFrameInInitiator(__filename, params.initiator)); + return params; }