inspector: provide detailed info to fix DevTools frontend errors

PR-URL: #54156 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
nodejs · Oct 2, 2024 · d1007fb · d1007fb
1 parent 337cd41
commit d1007fb
Show file tree

Hide file tree

Showing 6 changed files with 199 additions and 12 deletions.
diff --git 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<string, number | string | string[]>) to a plain object (Map<string, string>)
+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
@@ -5,9 +5,28 @@ namespace node {
 namespace inspector {
 namespace protocol {
 
-std::unique_ptr<Network::Request> Request(const String& url,
-                                          const String& method) {
-  return Network::Request::create().setUrl(url).setMethod(method).build();
+std::unique_ptr<Network::Request> createRequest(
+    const String& url,
+    const String& method,
+    std::unique_ptr<Network::Headers> headers) {
+  return Network::Request::create()
+      .setUrl(url)
+      .setMethod(method)
+      .setHeaders(std::move(headers))
+      .build();
+}
+
+std::unique_ptr<Network::Response> createResponse(
+    const String& url,
+    int status,
+    const String& statusText,
+    std::unique_ptr<Network::Headers> 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<Network::Headers>(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", &timestamp);
+  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<Network::Headers>(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
@@ -12,9 +12,6 @@ class NetworkInspector;
 
 namespace protocol {
 
-std::unique_ptr<Network::Request> 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
@@ -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
@@ -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
@@ -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());
 });