Skip to content

net: defer self.destroy calls to nextTick #54857

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
anilhelvaci wants to merge 7 commits into
base: main
Choose a base branch
from
Open

net: defer self.destroy calls to nextTick #54857

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f80efc1
net: defer self.destroy calls to nextTick
anilhelvaci Sep 6, 2024
728684c
debug: insert console.logs for detecting where the test times out. Wi…
anilhelvaci Jul 30, 2025
d7d9ce2
chore: try handling the error event onSocket
anilhelvaci Sep 23, 2025
1bf26ed
http: use private symbol for immediateError property
anilhelvaci Sep 25, 2025
293356d
chore: apply private symbol to onImmediateError and address linting e…
anilhelvaci Sep 27, 2025
5691c39
fix: rebase fixes
anilhelvaci Sep 8, 2026
19d5f3f
test: get rid of debug logging
anilhelvaci Sep 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
chore: try handling the error event onSocket 
Fixes: 48771

fix: revert net.js

fix: rollback net changes
  • Loading branch information
@anilhelvaci
anilhelvaci committed Sep 8, 2026
commit d7d9ce28581ee9cee6ec83128e87ba2c3eebdd71
3 changes: 2 additions & 1 deletion lib/_http_client.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1160,7 +1160,7 @@ ClientRequest.prototype.onSocket = function onSocket(socket, err) {
};

function onSocketNT(req, socket, err) {
if (req.destroyed || err) {
if (req.destroyed || err || req.immediateErr) {
req.destroyed = true;

function _destroy(req, err) {
Expand All @@ -1178,6 +1178,7 @@ function onSocketNT(req, socket, err) {
}

if (socket) {
socket.removeListener('error', socket.onImmediateError);
if (!err && req.agent && !socket.destroyed) {
socket.emit('free');
socket.removeListener('error', socketErrorListener);
Expand Down
8 changes: 4 additions & 4 deletions lib/net.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1366,7 +1366,7 @@ function internalConnect(
err = checkBindError(err, localPort, self._handle);
if (err) {
const ex = new ExceptionWithHostPort(err, 'bind', localAddress, localPort);
process.nextTick(() => self.destroy(ex));
self.destroy(ex);
return;
}
}
Expand Down Expand Up @@ -1408,7 +1408,7 @@ function internalConnect(
}

const ex = new ExceptionWithHostPort(err, 'connect', address, port, details);
process.nextTick(() => self.destroy(ex));
self.destroy(ex);
} else if ((addressType === 6 || addressType === 4) && hasObserver('net')) {
startPerf(self, kPerfHooksNetConnectContext, { type: 'net', name: 'connect', detail: { host: address, port } });
}
Expand All @@ -1427,11 +1427,11 @@ function internalConnectMultiple(context, canceled) {
// All connections have been tried without success, destroy with error
if (canceled || context.current === context.addresses.length) {
if (context.errors.length === 0) {
process.nextTick(() => self.destroy(new ERR_SOCKET_CONNECTION_TIMEOUT()));
self.destroy(new ERR_SOCKET_CONNECTION_TIMEOUT());
return;
}

process.nextTick(() => self.destroy(new NodeAggregateError(context.errors)));
self.destroy(new NodeAggregateError(context.errors));
return;
}

Expand Down
21 changes: 11 additions & 10 deletions test/parallel/test-http-client-immediate-error.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ const config = {

function agentFactory(handle, count) {
const agent = new http.Agent();
agent.createConnection = common.mustCall((cfg) => {
agent.createConnection = common.mustCall((...cfg) => {
const normalized = net._normalizeArgs(cfg);
const sock = new net.Socket();

// Fake the handle so we can enforce returning an immediate error
Expand All @@ -39,29 +40,29 @@ function agentFactory(handle, count) {
// Simulate just enough socket handle initialization
sock[async_id_symbol] = newAsyncId();

sock.connect(cfg);
sock.connect(normalized);
return sock;
}, count);

return agent;
};

const handleWithoutBind = {
const handleWithoutBind = callCount => ({
connect: common.mustCall((req, addr, port) => {
return UV_ENETUNREACH;
}, 3), // Because two tests will use this
}, callCount),
readStart() { },
close() { },
};
});

const handleWithBind = {
const handleWithBind = callCount => ({
readStart() { },
close() { },
bind: common.mustCall(() => UV_EADDRINUSE, 2), // Because two tests will use this handle
};
bind: common.mustCall(() => UV_EADDRINUSE, callCount),
});

const agentWithoutBind = agentFactory(handleWithoutBind, 3);
const agentWithBind = agentFactory(handleWithBind, 2);
const agentWithoutBind = agentFactory(handleWithoutBind(3), 3); // Because three tests will use this
const agentWithBind = agentFactory(handleWithBind(2), 2); // Because two tests will use this handle

console.log('test initiated: test-1', Date.now());
http.get({
Expand Down