Skip to content

Conversation

@RajeshKumar11
Copy link

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
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/net

@nodejs-github-bot 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
@codecov
Copy link

codecov bot commented Feb 3, 2026

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 7 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.

Copy link
Member

@pimterry pimterry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@pimterry pimterry added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 3, 2026
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 3, 2026
@nodejs-github-bot
Copy link
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ci PRs that need a full CI run. net Issues and PRs related to the net subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Conditional unhandled 'error' event when http.request with .lookup

3 participants