From 3616ace637e11776f3e49391da748195fcda7f72 Mon Sep 17 00:00:00 2001 From: Kohei Ueno Date: Mon, 5 Aug 2024 21:30:45 +0900 Subject: [PATCH] inspector: provide detailed info to fix DevTools frontend errors PR-URL: https://github.com/nodejs/node/pull/54156 Reviewed-By: Chengzhong Wu Reviewed-By: Benjamin Gruenbaum --- lib/internal/inspector_network_tracking.js | 33 +++++++++- src/inspector/network_agent.cc | 60 +++++++++++++++++-- src/inspector/network_agent.h | 3 - src/inspector/node_protocol.pdl | 38 ++++++++++++ .../test-inspector-emit-protocol-event.js | 31 +++++++++- .../parallel/test-inspector-network-domain.js | 46 ++++++++++++++ 6 files changed, 199 insertions(+), 12 deletions(-) diff --git a/lib/internal/inspector_network_tracking.js b/lib/internal/inspector_network_tracking.js index 4865537e37b7d5..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,18 +44,27 @@ function onClientRequestStart({ request }) { request: { url, method: request.method, + headers: headerObjectToDictionary(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, + statusText: response.statusMessage ?? '', + headers: headerObjectToDictionary(response.headers), + }, }); Network.loadingFinished({ requestId: request._inspectorRequestId, diff --git a/src/inspector/network_agent.cc b/src/inspector/network_agent.cc index de17ff0ecb2041..6c1de24a2a1cab 100644 --- a/src/inspector/network_agent.cc +++ b/src/inspector/network_agent.cc @@ -5,9 +5,28 @@ 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, + const String& statusText, + std::unique_ptr headers) { + return Network::Response::create() + .setUrl(url) + .setStatus(status) + .setStatusText(statusText) + .setHeaders(std::move(headers)) + .build(); } NetworkAgent::NetworkAgent(NetworkInspector* inspector) @@ -55,8 +74,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 +93,28 @@ 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); + String statusText; + response->getString("statusText", &statusText); + + 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, statusText, std::move(headers))); } void NetworkAgent::loadingFinished( diff --git a/src/inspector/network_agent.h b/src/inspector/network_agent.h index e2ca447b6e9480..ba113baf062b70 100644 --- a/src/inspector/network_agent.h +++ b/src/inspector/network_agent.h @@ -12,9 +12,6 @@ class NetworkInspector; namespace protocol { -std::unique_ptr Request(const String& url, - const String& method); - class NetworkAgent : public Network::Backend { public: explicit NetworkAgent(NetworkInspector* inspector); diff --git a/src/inspector/node_protocol.pdl b/src/inspector/node_protocol.pdl index ab7b6a5414c846..9c442c008dcb09 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,18 @@ experimental domain Network properties string url string method + Headers headers + + # HTTP response data. + type Response extends object + properties + string url + integer status + string statusText + 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 +175,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..fe8e1bc0d15d57 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,23 @@ const EXPECTED_EVENTS = { params: { requestId: 'request-id-1', timestamp: 1000, + type: 'Other', + response: { + url: 'https://nodejs.org/en', + 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' } + } } }, { @@ -68,7 +95,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..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; @@ -52,12 +68,26 @@ 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(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'); })); 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.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-')); @@ -69,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()); }); @@ -77,12 +108,26 @@ 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(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'); })); 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.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-')); @@ -95,6 +140,7 @@ const testHttpsGet = () => new Promise((resolve, reject) => { port: httpsServer.address().port, path: '/hello-world', rejectUnauthorized: false, + headers: requestHeaders, }, common.mustCall()); });