From 8e64c02b19dd49084ebdb82177c63f78db4c44e2 Mon Sep 17 00:00:00 2001 From: Kohei Ueno Date: Sun, 28 Jul 2024 18:57:38 +0900 Subject: [PATCH] http: add diagnostics channel `http.client.request.error` PR-URL: https://github.com/nodejs/node/pull/54054 Reviewed-By: Paolo Insogna Reviewed-By: Matteo Collina Reviewed-By: Marco Ippolito Reviewed-By: Ethan Arrowood --- doc/api/diagnostics_channel.md | 7 ++++++ lib/_http_client.js | 23 ++++++++++++++----- .../parallel/test-diagnostics-channel-http.js | 15 +++++++++++- 3 files changed, 38 insertions(+), 7 deletions(-) diff --git a/doc/api/diagnostics_channel.md b/doc/api/diagnostics_channel.md index e5858b00cc069e..39be4115bb4315 100644 --- a/doc/api/diagnostics_channel.md +++ b/doc/api/diagnostics_channel.md @@ -1119,6 +1119,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 index f9fd3700c3d19d..dd41aa7fab0fdb 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -95,8 +95,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) => { @@ -348,7 +359,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); } @@ -470,7 +481,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'); @@ -497,7 +508,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; @@ -521,7 +532,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(); @@ -546,7 +557,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; @@ -877,7 +888,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 index c2e84444e2866e..e134b9ac05a85d 100644 --- 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(`http://localhost:${port}`, (res) => { res.resume(); res.on('end', () => {