Skip to content

Commit da48083

Browse files
committed
http: attach error handler to socket synchronously in onSocket
Between onSocket and onSocketNT, the socket had no error handler, meaning any errors emitted during that window (e.g. from a blocklist check or custom lookup) would be unhandled even if the user had set up a request error handler. Fix this by attaching socketErrorListener synchronously in onSocket, setting socket._httpMessage so the listener can forward errors to the request. The _destroy path in onSocketNT is also guarded to prevent double-firing if socketErrorListener already emitted the error. Fixes: #48771 Refs: #61658
1 parent 04946a7 commit da48083

File tree

2 files changed

+16
-8
lines changed

2 files changed

+16
-8
lines changed

lib/_http_client.js

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -571,7 +571,7 @@ function socketErrorListener(err) {
571571
if (req) {
572572
// For Safety. Some additional errors might fire later on
573573
// and we need to make sure we don't double-fire the error event.
574-
req.socket._hadError = true;
574+
socket._hadError = true;
575575
emitErrorEvent(req, err);
576576
}
577577

@@ -906,7 +906,6 @@ function tickOnSocket(req, socket) {
906906
parser.joinDuplicateHeaders = req.joinDuplicateHeaders;
907907

908908
parser.onIncoming = parserOnIncomingClient;
909-
socket.on('error', socketErrorListener);
910909
socket.on('data', socketOnData);
911910
socket.on('end', socketOnEnd);
912911
socket.on('close', socketCloseListener);
@@ -945,8 +944,15 @@ function listenSocketTimeout(req) {
945944
}
946945

947946
ClientRequest.prototype.onSocket = function onSocket(socket, err) {
948-
// TODO(ronag): Between here and onSocketNT the socket
949-
// has no 'error' handler.
947+
// Attach the error listener synchronously so that any errors emitted on
948+
// the socket before onSocketNT runs (e.g. from a blocklist check or other
949+
// next-tick error) are forwarded to the request and can be caught by the
950+
// user's error handler. socketErrorListener requires socket._httpMessage
951+
// to be set so we set it here too.
952+
if (socket && !err) {
953+
socket._httpMessage = this;
954+
socket.on('error', socketErrorListener);
955+
}
950956
process.nextTick(onSocketNT, this, socket, err);
951957
};
952958

@@ -958,8 +964,10 @@ function onSocketNT(req, socket, err) {
958964
if (!req.aborted && !err) {
959965
err = new ConnResetException('socket hang up');
960966
}
961-
// ERR_PROXY_TUNNEL is handled by the proxying logic
962-
if (err && err.code !== 'ERR_PROXY_TUNNEL') {
967+
// ERR_PROXY_TUNNEL is handled by the proxying logic.
968+
// Skip if the error was already emitted by the early socketErrorListener.
969+
if (err && err.code !== 'ERR_PROXY_TUNNEL' &&
970+
!socket?._hadError) {
963971
emitErrorEvent(req, err);
964972
}
965973
req._closed = true;

test/parallel/test-http-request-lookup-error-catchable.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ const net = require('net');
1313
// 2. The lookup returns an IP that triggers a synchronous error (e.g., blockList)
1414
// 3. The error is emitted before http's error handler is set up (via nextTick)
1515
//
16-
// The fix defers socket.destroy() calls in internalConnect to the next tick,
17-
// giving http.request() time to set up its error handlers.
16+
// The fix attaches socketErrorListener synchronously in onSocket so that
17+
// socket errors are forwarded to the request before onSocketNT runs.
1818

1919
const blockList = new net.BlockList();
2020
blockList.addAddress(common.localhostIPv4);

0 commit comments

Comments
 (0)