From d823d8a6b2b720f43bc4bb70bf230bf6302ea25a Mon Sep 17 00:00:00 2001 From: cola119 Date: Thu, 1 Aug 2024 15:32:35 +0900 Subject: [PATCH 1/4] inspector: provide detailed info to fix DevTools frontend errors --- lib/internal/inspector_network_tracking.js | 10 +++- src/inspector/network_agent.cc | 53 ++++++++++++++++--- src/inspector/network_agent.h | 8 ++- src/inspector/node_protocol.pdl | 37 +++++++++++++ .../test-inspector-emit-protocol-event.js | 20 ++++++- .../parallel/test-inspector-network-domain.js | 10 ++++ 6 files changed, 127 insertions(+), 11 deletions(-) diff --git a/lib/internal/inspector_network_tracking.js b/lib/internal/inspector_network_tracking.js index 4865537e37b7d5..428c5033154147 100644 --- a/lib/internal/inspector_network_tracking.js +++ b/lib/internal/inspector_network_tracking.js @@ -22,18 +22,26 @@ function onClientRequestStart({ request }) { request: { url, method: request.method, + headers: request.getHeaders(), }, }); } -function onClientResponseFinish({ request }) { +function onClientResponseFinish({ request, response }) { if (typeof request._inspectorRequestId !== 'string') { return; } + const url = `${request.protocol}//${request.host}${request.path}`; const timestamp = DateNow() / 1000; Network.responseReceived({ requestId: request._inspectorRequestId, timestamp, + type: 'Other', + response: { + url, + status: response.statusCode, + headers: response.headers, + }, }); Network.loadingFinished({ requestId: request._inspectorRequestId, diff --git a/src/inspector/network_agent.cc b/src/inspector/network_agent.cc index de17ff0ecb2041..a75a7f8d6edd5d 100644 --- a/src/inspector/network_agent.cc +++ b/src/inspector/network_agent.cc @@ -5,9 +5,24 @@ namespace node { namespace inspector { namespace protocol { -std::unique_ptr Request(const String& url, - const String& method) { - return Network::Request::create().setUrl(url).setMethod(method).build(); +std::unique_ptr createRequest( + const String& url, + const String& method, + std::unique_ptr headers) { + return Network::Request::create() + .setUrl(url) + .setMethod(method) + .setHeaders(std::move(headers)) + .build(); +} + +std::unique_ptr createResponse( + const String& url, int status, std::unique_ptr headers) { + return Network::Response::create() + .setUrl(url) + .setStatus(status) + .setHeaders(std::move(headers)) + .build(); } NetworkAgent::NetworkAgent(NetworkInspector* inspector) @@ -55,8 +70,17 @@ void NetworkAgent::requestWillBeSent( String method; request->getString("method", &method); - frontend_->requestWillBeSent( - request_id, Request(url, method), timestamp, wall_time); + ErrorSupport errors; + auto headers = + Network::Headers::fromValue(request->getObject("headers"), &errors); + if (errors.hasErrors()) { + headers = std::make_unique(DictionaryValue::create()); + } + + frontend_->requestWillBeSent(request_id, + createRequest(url, method, std::move(headers)), + timestamp, + wall_time); } void NetworkAgent::responseReceived( @@ -65,8 +89,25 @@ void NetworkAgent::responseReceived( params->getString("requestId", &request_id); double timestamp; params->getDouble("timestamp", ×tamp); + String type; + params->getString("type", &type); + auto response = params->getObject("response"); + String url; + response->getString("url", &url); + int status; + response->getInteger("status", &status); + + ErrorSupport errors; + auto headers = + Network::Headers::fromValue(response->getObject("headers"), &errors); + if (errors.hasErrors()) { + headers = std::make_unique(DictionaryValue::create()); + } - frontend_->responseReceived(request_id, timestamp); + frontend_->responseReceived(request_id, + timestamp, + type, + createResponse(url, status, std::move(headers))); } void NetworkAgent::loadingFinished( diff --git a/src/inspector/network_agent.h b/src/inspector/network_agent.h index e2ca447b6e9480..bd0f82bffd3c5e 100644 --- a/src/inspector/network_agent.h +++ b/src/inspector/network_agent.h @@ -12,8 +12,12 @@ class NetworkInspector; namespace protocol { -std::unique_ptr Request(const String& url, - const String& method); +std::unique_ptr createRequest( + const String& url, + const String& method, + std::unique_ptr headers); +std::unique_ptr createResponse( + const String& url, int status, std::unique_ptr headers); class NetworkAgent : public Network::Backend { public: diff --git a/src/inspector/node_protocol.pdl b/src/inspector/node_protocol.pdl index ab7b6a5414c846..a0859e215ac19b 100644 --- a/src/inspector/node_protocol.pdl +++ b/src/inspector/node_protocol.pdl @@ -101,6 +101,28 @@ experimental domain NodeWorker # Partial support for Network domain of ChromeDevTools Protocol. # https://chromedevtools.github.io/devtools-protocol/tot/Network experimental domain Network + # Resource type as it was perceived by the rendering engine. + type ResourceType extends string + enum + Document + Stylesheet + Image + Media + Font + Script + TextTrack + XHR + Fetch + Prefetch + EventSource + WebSocket + Manifest + SignedExchange + Ping + CSPViolationReport + Preflight + Other + # Unique request identifier. type RequestId extends string @@ -115,6 +137,17 @@ experimental domain Network properties string url string method + Headers headers + + # HTTP response data. + type Response extends object + properties + string url + integer status + Headers headers + + # Request / response headers as keys / values of JSON object. + type Headers extends object # Disables network tracking, prevents network events from being sent to the client. command disable @@ -141,6 +174,10 @@ experimental domain Network RequestId requestId # Timestamp. MonotonicTime timestamp + # Resource type. + ResourceType type + # Response data. + Response response event loadingFinished parameters diff --git a/test/parallel/test-inspector-emit-protocol-event.js b/test/parallel/test-inspector-emit-protocol-event.js index 1a4e622c78881b..ec494ea8e9ef99 100644 --- a/test/parallel/test-inspector-emit-protocol-event.js +++ b/test/parallel/test-inspector-emit-protocol-event.js @@ -15,7 +15,17 @@ const EXPECTED_EVENTS = { requestId: 'request-id-1', request: { url: 'https://nodejs.org/en', - method: 'GET' + method: 'GET', + }, + timestamp: 1000, + wallTime: 1000, + }, + expected: { + requestId: 'request-id-1', + request: { + url: 'https://nodejs.org/en', + method: 'GET', + headers: {} // Headers should be an empty object if not provided. }, timestamp: 1000, wallTime: 1000, @@ -26,6 +36,12 @@ const EXPECTED_EVENTS = { params: { requestId: 'request-id-1', timestamp: 1000, + type: 'Other', + response: { + url: 'https://nodejs.org/en', + status: 200, + headers: { host: 'nodejs.org' } + } } }, { @@ -68,7 +84,7 @@ const runAsyncTest = async () => { for (const [domain, events] of Object.entries(EXPECTED_EVENTS)) { for (const event of events) { session.on(`${domain}.${event.name}`, common.mustCall(({ params }) => { - assert.deepStrictEqual(params, event.params); + assert.deepStrictEqual(params, event.expected ?? event.params); })); inspector[domain][event.name](event.params); } diff --git a/test/parallel/test-inspector-network-domain.js b/test/parallel/test-inspector-network-domain.js index 1dc0d4f65a216e..18b7c247178595 100644 --- a/test/parallel/test-inspector-network-domain.js +++ b/test/parallel/test-inspector-network-domain.js @@ -52,12 +52,17 @@ const testHttpGet = () => new Promise((resolve, reject) => { assert.ok(params.requestId.startsWith('node-network-event-')); assert.strictEqual(params.request.url, 'http://127.0.0.1/hello-world'); assert.strictEqual(params.request.method, 'GET'); + assert.strictEqual(typeof params.request.headers, 'object'); assert.strictEqual(typeof params.timestamp, 'number'); assert.strictEqual(typeof params.wallTime, 'number'); })); session.on('Network.responseReceived', common.mustCall(({ params }) => { assert.ok(params.requestId.startsWith('node-network-event-')); assert.strictEqual(typeof params.timestamp, 'number'); + assert.strictEqual(params.type, 'Other'); + assert.strictEqual(params.response.status, 200); + assert.strictEqual(params.response.url, 'http://127.0.0.1/hello-world'); + assert.strictEqual(typeof params.response.headers, 'object'); })); session.on('Network.loadingFinished', common.mustCall(({ params }) => { assert.ok(params.requestId.startsWith('node-network-event-')); @@ -77,12 +82,17 @@ const testHttpsGet = () => new Promise((resolve, reject) => { assert.ok(params.requestId.startsWith('node-network-event-')); assert.strictEqual(params.request.url, 'https://127.0.0.1/hello-world'); assert.strictEqual(params.request.method, 'GET'); + assert.strictEqual(typeof params.request.headers, 'object'); assert.strictEqual(typeof params.timestamp, 'number'); assert.strictEqual(typeof params.wallTime, 'number'); })); session.on('Network.responseReceived', common.mustCall(({ params }) => { assert.ok(params.requestId.startsWith('node-network-event-')); assert.strictEqual(typeof params.timestamp, 'number'); + assert.strictEqual(params.type, 'Other'); + assert.strictEqual(params.response.status, 200); + assert.strictEqual(params.response.url, 'https://127.0.0.1/hello-world'); + assert.strictEqual(typeof params.response.headers, 'object'); })); session.on('Network.loadingFinished', common.mustCall(({ params }) => { assert.ok(params.requestId.startsWith('node-network-event-')); From 4d446e4cdb231ba85d348bf6d2a644db24a3ac44 Mon Sep 17 00:00:00 2001 From: cola119 Date: Thu, 1 Aug 2024 19:05:19 +0900 Subject: [PATCH 2/4] inspector: support `Network.Response.statusText` property --- lib/internal/inspector_network_tracking.js | 1 + src/inspector/network_agent.cc | 17 ++++++++++++----- src/inspector/node_protocol.pdl | 1 + .../test-inspector-emit-protocol-event.js | 11 +++++++++++ test/parallel/test-inspector-network-domain.js | 2 ++ 5 files changed, 27 insertions(+), 5 deletions(-) diff --git a/lib/internal/inspector_network_tracking.js b/lib/internal/inspector_network_tracking.js index 428c5033154147..6f967431b7bb29 100644 --- a/lib/internal/inspector_network_tracking.js +++ b/lib/internal/inspector_network_tracking.js @@ -40,6 +40,7 @@ function onClientResponseFinish({ request, response }) { response: { url, status: response.statusCode, + statusText: response.statusMessage ?? '', headers: response.headers, }, }); diff --git a/src/inspector/network_agent.cc b/src/inspector/network_agent.cc index a75a7f8d6edd5d..6c1de24a2a1cab 100644 --- a/src/inspector/network_agent.cc +++ b/src/inspector/network_agent.cc @@ -17,10 +17,14 @@ std::unique_ptr createRequest( } std::unique_ptr createResponse( - const String& url, int status, std::unique_ptr headers) { + const String& url, + int status, + const String& statusText, + std::unique_ptr headers) { return Network::Response::create() .setUrl(url) .setStatus(status) + .setStatusText(statusText) .setHeaders(std::move(headers)) .build(); } @@ -96,6 +100,8 @@ void NetworkAgent::responseReceived( response->getString("url", &url); int status; response->getInteger("status", &status); + String statusText; + response->getString("statusText", &statusText); ErrorSupport errors; auto headers = @@ -104,10 +110,11 @@ void NetworkAgent::responseReceived( headers = std::make_unique(DictionaryValue::create()); } - frontend_->responseReceived(request_id, - timestamp, - type, - createResponse(url, status, std::move(headers))); + frontend_->responseReceived( + request_id, + timestamp, + type, + createResponse(url, status, statusText, std::move(headers))); } void NetworkAgent::loadingFinished( diff --git a/src/inspector/node_protocol.pdl b/src/inspector/node_protocol.pdl index a0859e215ac19b..9c442c008dcb09 100644 --- a/src/inspector/node_protocol.pdl +++ b/src/inspector/node_protocol.pdl @@ -144,6 +144,7 @@ experimental domain Network properties string url integer status + string statusText Headers headers # Request / response headers as keys / values of JSON object. diff --git a/test/parallel/test-inspector-emit-protocol-event.js b/test/parallel/test-inspector-emit-protocol-event.js index ec494ea8e9ef99..fe8e1bc0d15d57 100644 --- a/test/parallel/test-inspector-emit-protocol-event.js +++ b/test/parallel/test-inspector-emit-protocol-event.js @@ -42,6 +42,17 @@ const EXPECTED_EVENTS = { status: 200, headers: { host: 'nodejs.org' } } + }, + expected: { + requestId: 'request-id-1', + timestamp: 1000, + type: 'Other', + response: { + url: 'https://nodejs.org/en', + status: 200, + statusText: '', // Status text should be an empty string if not provided. + headers: { host: 'nodejs.org' } + } } }, { diff --git a/test/parallel/test-inspector-network-domain.js b/test/parallel/test-inspector-network-domain.js index 18b7c247178595..2d94d415aadfd1 100644 --- a/test/parallel/test-inspector-network-domain.js +++ b/test/parallel/test-inspector-network-domain.js @@ -61,6 +61,7 @@ const testHttpGet = () => new Promise((resolve, reject) => { assert.strictEqual(typeof params.timestamp, 'number'); assert.strictEqual(params.type, 'Other'); assert.strictEqual(params.response.status, 200); + assert.strictEqual(params.response.statusText, 'OK'); assert.strictEqual(params.response.url, 'http://127.0.0.1/hello-world'); assert.strictEqual(typeof params.response.headers, 'object'); })); @@ -91,6 +92,7 @@ const testHttpsGet = () => new Promise((resolve, reject) => { assert.strictEqual(typeof params.timestamp, 'number'); assert.strictEqual(params.type, 'Other'); assert.strictEqual(params.response.status, 200); + assert.strictEqual(params.response.statusText, 'OK'); assert.strictEqual(params.response.url, 'https://127.0.0.1/hello-world'); assert.strictEqual(typeof params.response.headers, 'object'); })); From a489613af2f8162672b14b6b27fe116bd236d547 Mon Sep 17 00:00:00 2001 From: cola119 Date: Fri, 2 Aug 2024 16:13:47 +0900 Subject: [PATCH 3/4] inspector: delete unused declarations in header file for cleanup --- src/inspector/network_agent.h | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/inspector/network_agent.h b/src/inspector/network_agent.h index bd0f82bffd3c5e..ba113baf062b70 100644 --- a/src/inspector/network_agent.h +++ b/src/inspector/network_agent.h @@ -12,13 +12,6 @@ class NetworkInspector; namespace protocol { -std::unique_ptr createRequest( - const String& url, - const String& method, - std::unique_ptr headers); -std::unique_ptr createResponse( - const String& url, int status, std::unique_ptr headers); - class NetworkAgent : public Network::Backend { public: explicit NetworkAgent(NetworkInspector* inspector); From e02aaa76caecbe1cf1278c43601dd8716a3558e9 Mon Sep 17 00:00:00 2001 From: cola119 Date: Fri, 2 Aug 2024 16:18:54 +0900 Subject: [PATCH 4/4] inspector: convert a header object to a simple string dictionary --- lib/internal/inspector_network_tracking.js | 26 ++++++++++++-- .../parallel/test-inspector-network-domain.js | 34 +++++++++++++++++++ 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/lib/internal/inspector_network_tracking.js b/lib/internal/inspector_network_tracking.js index 6f967431b7bb29..ab6dd789a3612e 100644 --- a/lib/internal/inspector_network_tracking.js +++ b/lib/internal/inspector_network_tracking.js @@ -1,7 +1,10 @@ 'use strict'; const { + ArrayIsArray, DateNow, + ObjectEntries, + String, } = primordials; let dc; @@ -10,6 +13,25 @@ let Network; let requestId = 0; const getNextRequestId = () => `node-network-event-${++requestId}`; +// Convert a Headers object (Map) to a plain object (Map) +const headerObjectToDictionary = (headers = {}) => { + const dict = {}; + for (const { 0: key, 1: value } of ObjectEntries(headers)) { + if (typeof value === 'string') { + dict[key] = value; + } else if (ArrayIsArray(value)) { + if (key.toLowerCase() === 'cookie') dict[key] = value.join('; '); + // ChromeDevTools frontend treats 'set-cookie' as a special case + // https://github.com/ChromeDevTools/devtools-frontend/blob/4275917f84266ef40613db3c1784a25f902ea74e/front_end/core/sdk/NetworkRequest.ts#L1368 + else if (key.toLowerCase() === 'set-cookie') dict[key] = value.join('\n'); + else dict[key] = value.join(', '); + } else { + dict[key] = String(value); + } + } + return dict; +}; + function onClientRequestStart({ request }) { const url = `${request.protocol}//${request.host}${request.path}`; const wallTime = DateNow(); @@ -22,7 +44,7 @@ function onClientRequestStart({ request }) { request: { url, method: request.method, - headers: request.getHeaders(), + headers: headerObjectToDictionary(request.getHeaders()), }, }); } @@ -41,7 +63,7 @@ function onClientResponseFinish({ request, response }) { url, status: response.statusCode, statusText: response.statusMessage ?? '', - headers: response.headers, + headers: headerObjectToDictionary(response.headers), }, }); Network.loadingFinished({ diff --git a/test/parallel/test-inspector-network-domain.js b/test/parallel/test-inspector-network-domain.js index 2d94d415aadfd1..982de9e6314407 100644 --- a/test/parallel/test-inspector-network-domain.js +++ b/test/parallel/test-inspector-network-domain.js @@ -13,10 +13,25 @@ const inspector = require('node:inspector/promises'); const session = new inspector.Session(); session.connect(); +const requestHeaders = { + 'accept-language': 'en-US', + 'Cookie': ['k1=v1', 'k2=v2'], + 'age': 1000, + 'x-header1': ['value1', 'value2'] +}; + +const setResponseHeaders = (res) => { + res.setHeader('server', 'node'); + res.setHeader('etag', 12345); + res.setHeader('Set-Cookie', ['key1=value1', 'key2=value2']); + res.setHeader('x-header2', ['value1', 'value2']); +}; + const httpServer = http.createServer((req, res) => { const path = req.url; switch (path) { case '/hello-world': + setResponseHeaders(res); res.writeHead(200); res.end('hello world\n'); break; @@ -32,6 +47,7 @@ const httpsServer = https.createServer({ const path = req.url; switch (path) { case '/hello-world': + setResponseHeaders(res); res.writeHead(200); res.end('hello world\n'); break; @@ -53,6 +69,10 @@ const testHttpGet = () => new Promise((resolve, reject) => { assert.strictEqual(params.request.url, 'http://127.0.0.1/hello-world'); assert.strictEqual(params.request.method, 'GET'); assert.strictEqual(typeof params.request.headers, 'object'); + assert.strictEqual(params.request.headers['accept-language'], 'en-US'); + assert.strictEqual(params.request.headers.cookie, 'k1=v1; k2=v2'); + assert.strictEqual(params.request.headers.age, '1000'); + assert.strictEqual(params.request.headers['x-header1'], 'value1, value2'); assert.strictEqual(typeof params.timestamp, 'number'); assert.strictEqual(typeof params.wallTime, 'number'); })); @@ -64,6 +84,10 @@ const testHttpGet = () => new Promise((resolve, reject) => { assert.strictEqual(params.response.statusText, 'OK'); assert.strictEqual(params.response.url, 'http://127.0.0.1/hello-world'); assert.strictEqual(typeof params.response.headers, 'object'); + assert.strictEqual(params.response.headers.server, 'node'); + assert.strictEqual(params.response.headers.etag, '12345'); + assert.strictEqual(params.response.headers['set-cookie'], 'key1=value1\nkey2=value2'); + assert.strictEqual(params.response.headers['x-header2'], 'value1, value2'); })); session.on('Network.loadingFinished', common.mustCall(({ params }) => { assert.ok(params.requestId.startsWith('node-network-event-')); @@ -75,6 +99,7 @@ const testHttpGet = () => new Promise((resolve, reject) => { host: '127.0.0.1', port: httpServer.address().port, path: '/hello-world', + headers: requestHeaders }, common.mustCall()); }); @@ -84,6 +109,10 @@ const testHttpsGet = () => new Promise((resolve, reject) => { assert.strictEqual(params.request.url, 'https://127.0.0.1/hello-world'); assert.strictEqual(params.request.method, 'GET'); assert.strictEqual(typeof params.request.headers, 'object'); + assert.strictEqual(params.request.headers['accept-language'], 'en-US'); + assert.strictEqual(params.request.headers.cookie, 'k1=v1; k2=v2'); + assert.strictEqual(params.request.headers.age, '1000'); + assert.strictEqual(params.request.headers['x-header1'], 'value1, value2'); assert.strictEqual(typeof params.timestamp, 'number'); assert.strictEqual(typeof params.wallTime, 'number'); })); @@ -95,6 +124,10 @@ const testHttpsGet = () => new Promise((resolve, reject) => { assert.strictEqual(params.response.statusText, 'OK'); assert.strictEqual(params.response.url, 'https://127.0.0.1/hello-world'); assert.strictEqual(typeof params.response.headers, 'object'); + assert.strictEqual(params.response.headers.server, 'node'); + assert.strictEqual(params.response.headers.etag, '12345'); + assert.strictEqual(params.response.headers['set-cookie'], 'key1=value1\nkey2=value2'); + assert.strictEqual(params.response.headers['x-header2'], 'value1, value2'); })); session.on('Network.loadingFinished', common.mustCall(({ params }) => { assert.ok(params.requestId.startsWith('node-network-event-')); @@ -107,6 +140,7 @@ const testHttpsGet = () => new Promise((resolve, reject) => { port: httpsServer.address().port, path: '/hello-world', rejectUnauthorized: false, + headers: requestHeaders, }, common.mustCall()); });