8000 net: deferred self.destroy calls in internalConnect(Multiple) to next… by timothyjrogers · Pull Request #51038 · nodejs/node · GitHub
[go: up one dir, main page]

Skip to content

net: deferred self.destroy calls in internalConnect(Multiple) to next…#51038

Closed
timothyjrogers wants to merge 1 commit intonodejs:mainfrom
timothyjrogers:48771-unhandled-net-exception
Closed

net: deferred self.destroy calls in internalConnect(Multiple) to next…#51038
timothyjrogers wants to merge 1 commit intonodejs:mainfrom
timothyjrogers:48771-unhandled-net-exception

Conversation

@timothyjrogers
Copy link

… tick

self.destroy calls in the internalConnect adn internalConnectMultiple functions have a narrow case where they can throw before an error handler has been established. This change defers them to the next tick to allow time for the error handler to be set.

Fixes: #48771

… tick

self.destroy calls in the internalConnect adn internalConnectMultiple
functions have a narrow case where they can throw before an error
handler has been established. This change defers them to the next
tick to allow time for the error handler to be set.

Fixes: #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 Dec 4, 2023
Copy link
Member
@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

Could you add a test for this?

@ShogunPanda
Copy link
Contributor

Once you add a test as @mcollina requested, it will LGTM.

@timothyjrogers timothyjrogers closed this by deleting the head repository Dec 28, 2024
nodejs-github-bot pushed a commit that referenced this pull request Feb 6, 2026
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>
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

4 participants

0