net: defer synchronous destroy calls in internalConnect by RajeshKumar11 · Pull Request #61658 · nodejs/node · GitHub

Defer socket.destroy() calls in internalConnect and internalConnectMultiple to the next tick. This ensures that error handlers have a chance to be set up before errors are emitted, particularly important when using http.request with a custom lookup function that returns synchronously.

Problem

When using http.request() with a custom synchronous lookup function that returns an IP triggering an immediate error (e.g., via blockList), the error was emitted before the HTTP client had set up its error handler. The HTTP client sets up the error handler via process.nextTick in onSocket(), but the connection error occurred synchronously during net.createConnection().

This caused unhandled 'error' events that could not be caught:

const blockList = new net.BlockList();
blockList.addAddress('127.0.0.1');

const req = http.request({
  host: 'example.com',
  port: 80,
  lookup: (hostname, options, cb) => cb(null, '127.0.0.1', 4),
  family: 4,
  createConnection: (opts) => net.createConnection({ ...opts, blockList })
});

req.on('error', (err) => {
  // This was never called - error was unhandled!
});

Solution

Defer socket.destroy() calls to the next tick in:

internalConnect() - bind errors, blockList errors, and connect errors
internalConnectMultiple() - timeout and aggregate errors

This gives callers (especially the HTTP client) time to set up error handlers before errors are emitted.

Testing

Added test/parallel/test-http-request-lookup-error-catchable.js that verifies errors are catchable when using http.request with a synchronous custom lookup and blockList.

Fixes: #48771
Refs: #51038

Defer socket.destroy() calls in internalConnect and internalConnectMultiple to the next tick. This ensures that error handlers have a chance to be set up before errors are emitted, particularly important when using http.request with a custom lookup function that returns synchronously. Previously, if a synchronous lookup function returned an IP that triggered an immediate error (e.g., via blockList), the error would be emitted before the HTTP client had set up its error handler (which happens via process.nextTick in onSocket). This caused unhandled 'error' events. Fixes: nodejs#48771

Review requested:

@nodejs/net

Codecov Report

❌ Patch coverage is 92.30769% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 89.72%. Comparing base (f77a709) to head (f6abca6).
⚠️ Report is 26 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/net.js	92.30%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #61658      +/-   ##
==========================================
- Coverage   89.74%   89.72%   -0.03%     
==========================================
  Files         674      674              
  Lines      204360   204368       +8     
  Branches    39265    39270       +5     
==========================================
- Hits       183412   183375      -37     
- Misses      13251    13303      +52     
+ Partials     7697     7690       -7

Files with missing lines	Coverage Δ
lib/net.js	94.52% <92.30%> (+0.01%)	⬆️

... and 29 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

This looks good to me, nice fix, thanks @RajeshKumar11! 👍

CI: https://ci.nodejs.org/job/node-test-pull-request/71185/

nice fix! lgtm

Landed in d8c00ad

This is wrong and is just hiding an issue in the http client. destroy will already defer emitting error.

Defer socket.destroy() calls in internalConnect and internalConnectMultiple to the next tick. This ensures that error handlers have a chance to be set up before errors are emitted, particularly important when using http.request with a custom lookup function that returns synchronously. Previously, if a synchronous lookup function returned an IP that triggered an immediate error (e.g., via blockList), the error would be emitted before the HTTP client had set up its error handler (which happens via process.nextTick in onSocket). This caused unhandled 'error' events. Fixes: #48771 PR-URL: #61658 Refs: #51038 Reviewed-By: Tim Perry <pimterry@gmail.com> Reviewed-By: Jason Zhang <xzha4350@gmail.com>

@ronag good point, nice catch. Helpfully you've also left a TODO documenting what I think is the real underlying issue inline as well 😃

node/lib/_http_client.js

Lines 948 to 949 in 0777dcc

    
           // TODO(ronag): Between here and onSocketNT the socket 
        
           // has no 'error' handler.

I think that is exactly what's happening here:

createConnection successfully returns a socket synchronously to the HTTP client, triggering onSocket. This schedules connection setup including error handling for the next tick (onSocketNT).
The blocklist failed during that process, but it deferred emitting the error until next tick so handlers can be set up.
Because the emit-error NT callback was registered first, it fires before the onSocketNT NT callback that would listen for errors, and so there's no error handler registered => 💥

Checking the test behaviour in more detail, that makes sense: in this new test, req.on('error', ...) is actually called before the unhandled error event failure happens. That means request error handler is actually set up, but it's not just actually handling the socket error.

@RajeshKumar11 do you want to take a look and open another PR to improve this fix? The issue is real and this fix does work for the given test, but it would be better to fix in underlying problem directly instead (cleaner, and will fix other bugs too, e.g. any other kinds of next-tick errors like from non-socket streams or similar used in createConnection).

I would guess that we just want to pull the existing request error handler setup to earlier in the process, but it depends what happens in the tests when you do that. At a glance that looks safe but it needs checking and might run into issues, looks like there are some details about how the ordering of agent connections & events need to be handled maybe. Alternatively if there's a very good reason, we could add separate error handling there, just for this case specifically, and then remove it again when onSocketNT fires. Give it a go, feel free to reach out if you run into issues.

Thanks for the detailed explanation @pimterry and @ronag!

You're right that this is masking the underlying issue. I'd like to take a shot at fixing the root cause in lib/_http_client.js properly.

From what I understand, the issue is that between onSocket (line 947) and onSocketNT (line 953), the socket has no error handler, so any synchronous/next-tick errors get missed.

I'll investigate moving the error handler setup earlier and open a new PR. Will reach out if I run into issues with the agent connection ordering.

I've been working on the proper fix in lib/_http_client.js and I'm running into a timing challenge that I'd like your guidance on.

The Problem:
The error is emitted from the socket before the temporary error handler can be attached. Here's the sequence:

http.request() is called with custom createConnection
Line 428: opts.createConnection(opts, oncreate) creates socket and error is emitted (via next tick)
Line 430: oncreate(null, newSocket) calls onSocket(socket)
But by this point, the error has already been queued for emission

I tried attaching a temporary error handler immediately after createConnection returns (in the branch fix/http-client-socket-error-handler), but the test still fails.

Question:
Since the error is emitted via process.nextTick inside net.createConnection (from internalConnect), and http.request also uses process.nextTick for onSocketNT, there's a race between which nextTick callback fires first.

Should the fix be:

Attach the error handler synchronously in createConnection path before any ticks happen?
Or is there a different approach you had in mind?

I'd appreciate some guidance on the right direction before I go further down this path. Thanks!

Option 1 I think. Somehow we need to make sure we synchronously attach an error handler after createConnection called, before the next tick.

The main open question is whether we can use the existing error handler (added in tickOnSocket - we could just remove that and add it in advance in onSocket instead) or whether there's some details to how that behaves or how tickOnSocket is used or how the handler works that mean we need a separate temporary error handler that then gets removed next tick.

Hopefully the former since that would be simpler, but you'll need to make the change, test it, and carefully examine & explore the behaviour of the normal socketErrorListener to ensure that there's no unintended behaviour changes or issues with this.

Defer socket.destroy() calls in internalConnect and internalConnectMultiple to the next tick. This ensures that error handlers have a chance to be set up before errors are emitted, particularly important when using http.request with a custom lookup function that returns synchronously. Previously, if a synchronous lookup function returned an IP that triggered an immediate error (e.g., via blockList), the error would be emitted before the HTTP client had set up its error handler (which happens via process.nextTick in onSocket). This caused unhandled 'error' events. Fixes: #48771 PR-URL: #61658 Refs: #51038 Reviewed-By: Tim Perry <pimterry@gmail.com> Reviewed-By: Jason Zhang <xzha4350@gmail.com>

Opened a follow-up PR #61770 that fixes the underlying issue properly by attaching socketErrorListener synchronously in onSocket, as suggested by @pimterry.

This MR contains the following updates: | Package | Update | Change | |---|---|---| | [node](https://nodejs.org) ([source](https://github.com/nodejs/node)) | patch | `25.6.0` → `25.6.1` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>nodejs/node (node)</summary> ### [`v25.6.1`](https://github.com/nodejs/node/releases/tag/v25.6.1): 2026-02-10, Version 25.6.1 (Current), @aduh95 [Compare Source](nodejs/node@v25.6.0...v25.6.1) ##### Notable Changes - \[[`47df4328d7`](nodejs/node@47df4328d7)] - **build,deps**: replace `cjs-module-lexer` with `merve` (Yagiz Nizipli) [#61456](nodejs/node#61456) ##### Commits - \[[`47df4328d7`](nodejs/node@47df4328d7)] - **build,deps**: replace cjs-module-lexer with merve (Yagiz Nizipli) [#61456](nodejs/node#61456) - \[[`a727054503`](nodejs/node@a727054503)] - **deps**: upgrade npm to 11.9.0 (npm team) [#61685](nodejs/node#61685) - \[[`c78c49ed6b`](nodejs/node@c78c49ed6b)] - **deps**: update amaro to 1.1.7 (Node.js GitHub Bot) [#61730](nodejs/node#61730) - \[[`4790816d9b`](nodejs/node@4790816d9b)] - **deps**: update minimatch to 10.1.2 (Node.js GitHub Bot) [#61732](nodejs/node#61732) - \[[`8c71740e8a`](nodejs/node@8c71740e8a)] - **deps**: update undici to 7.21.0 (Node.js GitHub Bot) [#61683](nodejs/node#61683) - \[[`e559ef6ab1`](nodejs/node@e559ef6ab1)] - **deps**: update googletest to [`56efe39`](nodejs/node@56efe39) (Node.js GitHub Bot) [#61605](nodejs/node#61605) - \[[`300de2bb5a`](nodejs/node@300de2bb5a)] - **deps**: update amaro to 1.1.6 (Node.js GitHub Bot) [#61603](nodejs/node#61603) - \[[`e71e9505ef`](nodejs/node@e71e9505ef)] - **dns**: fix Windows SRV ECONNREFUSED by adjusting c-ares fallback detection (notvivek12) [#61453](nodejs/node#61453) - \[[`439b816bc7`](nodejs/node@439b816bc7)] - **doc**: clarify EventEmitter error handling in threat model (Matteo Collina) [#61701](nodejs/node#61701) - \[[`c1c6641f23`](nodejs/node@c1c6641f23)] - **doc**: mention default option for test runner env (Steven) [#61659](nodejs/node#61659) - \[[`41ec451f98`](nodejs/node@41ec451f98)] - **doc**: fix --inspect security warning section (Tim Perry) [#61675](nodejs/node#61675) - \[[`bb90ef2356`](nodejs/node@bb90ef2356)] - **doc**: document `url.format(urlString)` as deprecated under DEP0169 (René) [#61644](nodejs/node#61644) - \[[`513df82e6f`](nodejs/node@513df82e6f)] - **doc**: update to Visual Studio 2026 manual install (Mike McCready) [#61655](nodejs/node#61655) - \[[`9409d30736`](nodejs/node@9409d30736)] - **doc**: deprecation add more codemod (Augustin Mauroy) [#61642](nodejs/node#61642) - \[[`75a7a67151`](nodejs/node@75a7a67151)] - **doc**: fix grammatical error in README.md (ayj8201) [#61653](nodejs/node#61653) - \[[`821e59e884`](nodejs/node@821e59e884)] - **doc**: correct tools README Boxstarter link (Mike McCready) [#61638](nodejs/node#61638) - \[[`4998f539a0`](nodejs/node@4998f539a0)] - **doc**: update `server.dropMaxConnection` link (YuSheng Chen) [#61584](nodejs/node#61584) - \[[`9383ac4ab7`](nodejs/node@9383ac4ab7)] - **http**: implement slab allocation for HTTP header parsing (Mert Can Altin) [#61375](nodejs/node#61375) - \[[`e90eb1d561`](nodejs/node@e90eb1d561)] - **meta**: persist sccache daemon until end of build workflows (René) [#61639](nodejs/node#61639) - \[[`ade36ac367`](nodejs/node@ade36ac367)] - **meta**: bump github/codeql-action from 4.31.9 to 4.32.0 (dependabot\[bot]) [#61622](nodejs/node#61622) - \[[`26638bd67f`](nodejs/node@26638bd67f)] - **meta**: bump step-security/harden-runner from 2.14.0 to 2.14.1 (dependabot\[bot]) [#61621](nodejs/node#61621) - \[[`eaa9a96cb6`](nodejs/node@eaa9a96cb6)] - **meta**: bump actions/setup-python from 6.1.0 to 6.2.0 (dependabot\[bot]) [#61627](nodejs/node#61627) - \[[`fd98187828`](nodejs/node@fd98187828)] - **meta**: bump cachix/cachix-action (dependabot\[bot]) [#61626](nodejs/node#61626) - \[[`820c1d021c`](nodejs/node@820c1d021c)] - **meta**: bump actions/setup-node from 6.1.0 to 6.2.0 (dependabot\[bot]) [#61625](nodejs/node#61625) - \[[`72a4136bd5`](nodejs/node@72a4136bd5)] - **meta**: bump actions/cache from 5.0.1 to 5.0.3 (dependabot\[bot]) [#61624](nodejs/node#61624) - \[[`e3ef6cb3bc`](nodejs/node@e3ef6cb3bc)] - **meta**: bump peter-evans/create-pull-request from 8.0.0 to 8.1.0 (dependabot\[bot]) [#61623](nodejs/node#61623) - \[[`020a836202`](nodejs/node@020a836202)] - **meta**: bump actions/stale from 10.1.0 to 10.1.1 (dependabot\[bot]) [#61620](nodejs/node#61620) - \[[`0df72f07c8`](nodejs/node@0df72f07c8)] - **meta**: bump actions/checkout from 6.0.1 to 6.0.2 (dependabot\[bot]) [#61619](nodejs/node#61619) - \[[`d147c08b83`](nodejs/node@d147c08b83)] - **module**: do not invoke resolve hooks twice for imported cjs (Joyee Cheung) [#61529](nodejs/node#61529) - \[[`a2843f8556`](nodejs/node@a2843f8556)] - **net**: defer synchronous destroy calls in internalConnect (RajeshKumar11) [#61658](nodejs/node#61658) - \[[`7fb7030781`](nodejs/node@7fb7030781)] - **repl**: fix flaky test-repl-programmatic-history (Matteo Collina) [#61614](nodejs/node#61614) - \[[`d4c9b5cf5b`](nodejs/node@d4c9b5cf5b)] - **sqlite**: avoid extra copy for large text binds (Ali Hassan) [#61580](nodejs/node#61580) - \[[`aa1b3661d9`](nodejs/node@aa1b3661d9)] - **sqlite**: use DictionaryTemplate for run() result (Mert Can Altin) [#61432](nodejs/node#61432) - \[[`9c8ad7e881`](nodejs/node@9c8ad7e881)] - **src**: elide heap allocation in structured clone implementation (Anna Henningsen) [#61703](nodejs/node#61703) - \[[`c4ecfef93d`](nodejs/node@c4ecfef93d)] - **src**: use simdutf for one-byte string UTF-8 write in stringBytes (Mert Can Altin) [#61696](nodejs/node#61696) - \[[`28905b9734`](nodejs/node@28905b9734)] - **src**: consolidate C++ ReadFileSync/WriteFileSync utilities (Joyee Cheung) [#61662](nodejs/node#61662) - \[[`e90cec2f69`](nodejs/node@e90cec2f69)] - **test**: restraint version replacement pattern in snapshots (Chengzhong Wu) [#61748](nodejs/node#61748) - \[[`adce20c0a1`](nodejs/node@adce20c0a1)] - **test**: print stack immediately avoiding GC interleaving (Chengzhong Wu) [#61699](nodejs/node#61699) - \[[`7643bc8999`](nodejs/node@7643bc8999)] - **test**: fix case-insensitive path matching on Windows (Matteo Collina) [#61682](nodejs/node#61682) - \[[`23d1ecf66f`](nodejs/node@23d1ecf66f)] - **test**: fix flaky test-performance-eventloopdelay (Matteo Collina) [#61629](nodejs/node#61629) - \[[`99012a88ed`](nodejs/node@99012a88ed)] - **test**: remove duplicate wpt tests (Filip Skokan) [#61617](nodejs/node#61617) - \[[`a8b32b8ce1`](nodejs/node@a8b32b8ce1)] - **test**: fix race condition in watch mode tests (Matteo Collina) [#61615](nodejs/node#61615) - \[[`086a5a5a25`](nodejs/node@086a5a5a25)] - **test**: update WPT for url to [`e3c46fd`](nodejs/node@e3c46fdf55) (Node.js GitHub Bot) [#61602](nodejs/node#61602) - \[[`f0574fd419`](nodejs/node@f0574fd419)] - **test**: use the skipIfNoWatch() utility function (Luigi Pinca) [#61531](nodejs/node#61531) - \[[`b064ddc221`](nodejs/node@b064ddc221)] - **test**: unify assertSnapshot common patterns (Chengzhong Wu) [#61590](nodejs/node#61590) - \[[`17122e521b`](nodejs/node@17122e521b)] - **test\_runner**: fix test enqueue when test file has syntax error (Edy Silva) [#61573](nodejs/node#61573) - \[[`bad3f02dd9`](nodejs/node@bad3f02dd9)] - **tools**: enforce removal of `lts-watch-*` labels on release proposals (Antoine du Hamel) [#61672](nodejs/node#61672) - \[[`a8f33fd6bd`](nodejs/node@a8f33fd6bd)] - **tools**: use ubuntu-slim runner in meta GitHub Actions (Tierney Cyren) [#61663](nodejs/node#61663) - \[[`c843e447ca`](nodejs/node@c843e447ca)] - **tools**: test `--shared-merve` in `test-shared` workflow (Antoine du Hamel) [#61649](nodejs/node#61649) - \[[`2fedc03f96`](nodejs/node@2fedc03f96)] - **tools**: update OpenSSL to 3.5.5 in `test-shared` (Antoine du Hamel) [#61551](nodejs/node#61551) - \[[`1c1db94670`](nodejs/node@1c1db94670)] - **tools,win**: upgrade install additional tools to Visual Studio 2026 (Mike McCready) [#61562](nodejs/node#61562) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

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: nodejs#48771 Refs: nodejs#61658

Defer socket.destroy() calls in internalConnect and internalConnectMultiple to the next tick. This ensures that error handlers have a chance to be set up before errors are emitted, particularly important when using http.request with a custom lookup function that returns synchronously. Previously, if a synchronous lookup function returned an IP that triggered an immediate error (e.g., via blockList), the error would be emitted before the HTTP client had set up its error handler (which happens via process.nextTick in onSocket). This caused unhandled 'error' events. Fixes: #48771 PR-URL: #61658 Refs: #51038 Reviewed-By: Tim Perry <pimterry@gmail.com> Reviewed-By: Jason Zhang <xzha4350@gmail.com>

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 PR-URL: #61770 Refs: #48771 Reviewed-By: Tim Perry <pimterry@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

Defer socket.destroy() calls in internalConnect and internalConnectMultiple to the next tick. This ensures that error handlers have a chance to be set up before errors are emitted, particularly important when using http.request with a custom lookup function that returns synchronously. Previously, if a synchronous lookup function returned an IP that triggered an immediate error (e.g., via blockList), the error would be emitted before the HTTP client had set up its error handler (which happens via process.nextTick in onSocket). This caused unhandled 'error' events. Fixes: #48771 PR-URL: #61658 Refs: #51038 Reviewed-By: Tim Perry <pimterry@gmail.com> Reviewed-By: Jason Zhang <xzha4350@gmail.com>

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 PR-URL: #61770 Refs: #48771 Reviewed-By: Tim Perry <pimterry@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

Defer socket.destroy() calls in internalConnect and internalConnectMultiple to the next tick. This ensures that error handlers have a chance to be set up before errors are emitted, particularly important when using http.request with a custom lookup function that returns synchronously. Previously, if a synchronous lookup function returned an IP that triggered an immediate error (e.g., via blockList), the error would be emitted before the HTTP client had set up its error handler (which happens via process.nextTick in onSocket). This caused unhandled 'error' events. Fixes: nodejs#48771 PR-URL: nodejs#61658 Refs: nodejs#51038 Reviewed-By: Tim Perry <pimterry@gmail.com> Reviewed-By: Jason Zhang <xzha4350@gmail.com>

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: nodejs#48771 Refs: nodejs#61658 PR-URL: nodejs#61770 Refs: nodejs#48771 Reviewed-By: Tim Perry <pimterry@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

Defer socket.destroy() calls in internalConnect and internalConnectMultiple to the next tick. This ensures that error handlers have a chance to be set up before errors are emitted, particularly important when using http.request with a custom lookup function that returns synchronously. Previously, if a synchronous lookup function returned an IP that triggered an immediate error (e.g., via blockList), the error would be emitted before the HTTP client had set up its error handler (which happens via process.nextTick in onSocket). This caused unhandled 'error' events. Fixes: #48771 PR-URL: #61658 Refs: #51038 Reviewed-By: Tim Perry <pimterry@gmail.com> Reviewed-By: Jason Zhang <xzha4350@gmail.com>

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 PR-URL: #61770 Refs: #48771 Reviewed-By: Tim Perry <pimterry@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

Defer socket.destroy() calls in internalConnect and internalConnectMultiple to the next tick. This ensures that error handlers have a chance to be set up before errors are emitted, particularly important when using http.request with a custom lookup function that returns synchronously. Previously, if a synchronous lookup function returned an IP that triggered an immediate error (e.g., via blockList), the error would be emitted before the HTTP client had set up its error handler (which happens via process.nextTick in onSocket). This caused unhandled 'error' events. Fixes: #48771 PR-URL: #61658 Refs: #51038 Reviewed-By: Tim Perry <pimterry@gmail.com> Reviewed-By: Jason Zhang <xzha4350@gmail.com>

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 PR-URL: #61770 Refs: #48771 Reviewed-By: Tim Perry <pimterry@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

Defer socket.destroy() calls in internalConnect and internalConnectMultiple to the next tick. This ensures that error handlers have a chance to be set up before errors are emitted, particularly important when using http.request with a custom lookup function that returns synchronously. Previously, if a synchronous lookup function returned an IP that triggered an immediate error (e.g., via blockList), the error would be emitted before the HTTP client had set up its error handler (which happens via process.nextTick in onSocket). This caused unhandled 'error' events. Fixes: #48771 PR-URL: #61658 Refs: #51038 Reviewed-By: Tim Perry <pimterry@gmail.com> Reviewed-By: Jason Zhang <xzha4350@gmail.com>

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 PR-URL: #61770 Refs: #48771 Reviewed-By: Tim Perry <pimterry@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

nodejs-github-bot added needs-ci PRs that need a full CI run. net Issues and PRs related to the net subsystem. labels Feb 3, 2026

pimterry approved these changes Feb 3, 2026

View reviewed changes

pimterry added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 3, 2026

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 3, 2026

jazelly approved these changes Feb 4, 2026

View reviewed changes

pimterry added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 6, 2026

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 6, 2026

nodejs-github-bot merged commit d8c00ad into nodejs:main Feb 6, 2026
76 of 78 checks passed

github-actions Bot mentioned this pull request Feb 8, 2026

2026-02-10, Version 25.6.1 (Current) #61739

Merged

RajeshKumar11 mentioned this pull request Feb 11, 2026

http: attach error handler to socket synchronously in onSocket #61770

Merged

github-actions Bot mentioned this pull request Feb 22, 2026

2026-02-24, Version 24.14.0 'Krypton' (LTS) #61924

Merged

net: defer synchronous destroy calls in internalConnect by RajeshKumar11 · Pull Request #61658 · nodejs/node · GitHub

Uh oh!

Uh oh!

net: defer synchronous destroy calls in internalConnect - #61658

net: defer synchronous destroy calls in internalConnect#61658nodejs-github-bot merged 1 commit intonodejs:mainnodejs/node:mainfrom RajeshKumar11:fix/net-deferred-destroy-48771RajeshKumar11/node:fix/net-deferred-destroy-48771Copy head branch name to clipboard

Conversation

RajeshKumar11 commented Feb 3, 2026

Problem

Solution

Testing

Uh oh!

nodejs-github-bot commented Feb 3, 2026

Uh oh!

codecov Bot commented Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

pimterry left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Feb 3, 2026

Uh oh!

jazelly left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nodejs-github-bot commented Feb 6, 2026

Uh oh!

ronag commented Feb 7, 2026

Uh oh!

pimterry commented Feb 8, 2026

Uh oh!

RajeshKumar11 commented Feb 10, 2026

Uh oh!

RajeshKumar11 commented Feb 10, 2026

Uh oh!

pimterry commented Feb 10, 2026

Uh oh!

RajeshKumar11 commented Feb 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

net: defer synchronous destroy calls in internalConnect#61658
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
RajeshKumar11:fix/net-deferred-destroy-48771

codecov Bot commented Feb 3, 2026 •
edited
Loading