http: add diagnostics channel http.client.request.error

cola119 · RafaelGSS · commit 68e444d2d8d0 · 2024-08-05T11:15:46.000-03:00
PR-URL: #54054 Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
diff --git a/doc/api/diagnostics_channel.md b/doc/api/diagnostics_channel.md
@@ -1129,6 +1129,13 @@ independently.
 
 Emitted when client starts a request.
 
+`http.client.request.error`
+
+* `request` {http.ClientRequest}
+* `error` {Error}
+
+Emitted when an error occurs during a client request.
+
 `http.client.response.finish`
 
 * `request` {http.ClientRequest}
diff --git a/lib/_http_client.js b/lib/_http_client.js
@@ -90,8 +90,19 @@ const kClientRequestStatistics = Symbol('ClientRequestStatistics');
 
 const dc = require('diagnostics_channel');
 const onClientRequestStartChannel = dc.channel('http.client.request.start');
+const onClientRequestErrorChannel = dc.channel('http.client.request.error');
 const onClientResponseFinishChannel = dc.channel('http.client.response.finish');
 
+function emitErrorEvent(request, error) {
+  if (onClientRequestErrorChannel.hasSubscribers) {
+    onClientRequestErrorChannel.publish({
+      request,
+      error,
+    });
+  }
+  request.emit('error', error);
+}
+
 const { addAbortSignal, finished } = require('stream');
 
 let debug = require('internal/util/debuglog').debuglog('http', (fn) => {
@@ -343,7 +354,7 @@ function ClientRequest(input, options, cb) {
     if (typeof opts.createConnection === 'function') {
       const oncreate = once((err, socket) => {
         if (err) {
-          process.nextTick(() => this.emit('error', err));
+          process.nextTick(() => emitErrorEvent(this, err));
         } else {
           this.onSocket(socket);
         }
@@ -465,7 +476,7 @@ function socketCloseListener() {
       // receive a response. The error needs to
       // fire on the request.
       req.socket._hadError = true;
-      req.emit('error', new ConnResetException('socket hang up'));
+      emitErrorEvent(req, new ConnResetException('socket hang up'));
     }
     req._closed = true;
     req.emit('close');
@@ -492,7 +503,7 @@ function socketErrorListener(err) {
     // For Safety. Some additional errors might fire later on
     // and we need to make sure we don't double-fire the error event.
     req.socket._hadError = true;
-    req.emit('error', err);
+    emitErrorEvent(req, err);
   }
 
   const parser = socket.parser;
@@ -516,7 +527,7 @@ function socketOnEnd() {
     // If we don't have a response then we know that the socket
     // ended prematurely and we need to emit an error on the request.
     req.socket._hadError = true;
-    req.emit('error', new ConnResetException('socket hang up'));
+    emitErrorEvent(req, new ConnResetException('socket hang up'));
   }
   if (parser) {
     parser.finish();
@@ -541,7 +552,7 @@ function socketOnData(d) {
     socket.removeListener('end', socketOnEnd);
     socket.destroy();
     req.socket._hadError = true;
-    req.emit('error', ret);
+    emitErrorEvent(req, ret);
   } else if (parser.incoming && parser.incoming.upgrade) {
     // Upgrade (if status code 101) or CONNECT
     const bytesParsed = ret;
@@ -872,7 +883,7 @@ function onSocketNT(req, socket, err) {
         err = new ConnResetException('socket hang up');
       }
       if (err) {
-        req.emit('error', err);
+        emitErrorEvent(req, err);
       }
       req._closed = true;
       req.emit('close');
diff --git a/test/parallel/test-diagnostics-channel-http.js b/test/parallel/test-diagnostics-channel-http.js
@@ -1,5 +1,6 @@
 'use strict';
 const common = require('../common');
+const { addresses } = require('../common/internet');
 const assert = require('assert');
 const http = require('http');
 const net = require('net');
@@ -9,9 +10,15 @@ const isHTTPServer = (server) => server instanceof http.Server;
 const isIncomingMessage = (object) => object instanceof http.IncomingMessage;
 const isOutgoingMessage = (object) => object instanceof http.OutgoingMessage;
 const isNetSocket = (socket) => socket instanceof net.Socket;
+const isError = (error) => error instanceof Error;
 
 dc.subscribe('http.client.request.start', common.mustCall(({ request }) => {
   assert.strictEqual(isOutgoingMessage(request), true);
+}, 2));
+
+dc.subscribe('http.client.request.error', common.mustCall(({ request, error }) => {
+  assert.strictEqual(isOutgoingMessage(request), true);
+  assert.strictEqual(isError(error), true);
 }));
 
 dc.subscribe('http.client.response.finish', common.mustCall(({
@@ -50,8 +57,14 @@ const server = http.createServer(common.mustCall((req, res) => {
   res.end('done');
 }));
 
-server.listen(() => {
+server.listen(async () => {
   const { port } = server.address();
+  const invalidRequest = http.get({
+    host: addresses.INVALID_HOST,
+  });
+  await new Promise((resolve) => {
+    invalidRequest.on('error', resolve);
+  });
   http.get(`https://localhost:${port}`, (res) => {
     res.resume();
     res.on('end', () => {
