Skip to content

Commit

Permalink
http: add diagnostics channel http.client.request.error
Browse files Browse the repository at this point in the history
PR-URL: #54054
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Ethan Arrowood <[email protected]>
  • Loading branch information
cola119 authored and marco-ippolito committed Aug 19, 2024
1 parent 8999425 commit c711c98
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 7 deletions.
7 changes: 7 additions & 0 deletions doc/api/diagnostics_channel.md
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
23 changes: 17 additions & 6 deletions lib/_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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');
Expand All @@ -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;
Expand All @@ -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();
Expand All @@ -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;
Expand Down Expand Up @@ -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');
Expand Down
15 changes: 14 additions & 1 deletion test/parallel/test-diagnostics-channel-http.js
Original file line number Diff line number Diff line change
@@ -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');
Expand All @@ -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(({
Expand Down Expand Up @@ -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', () => {
Expand Down

0 comments on commit c711c98

Please sign in to comment.