Skip to content

net: defer self.destroy calls to nextTick

Rodrigo Muino Tomonari requested to merge github/fork/anilhelvaci/fix-48771-net-exception-with-tests into main

Fixes: https://github.com/nodejs/node/issues/48771

What is the problem being solved?

#48771 Reported request object returned from http.request method cannot catch error events triggered when there’s an immediate failure trying to connect to an address returned from dns lookup.

Solution

#51038 implemented changes suggested in https://github.com/nodejs/node/issues/48771#issuecomment-1661858958. However the #51038 couldn’t be merged due to lack of tests(https://github.com/nodejs/node/pull/51038#pullrequestreview-1761719500). In this PR, I apply the same fix but with some tests.

Testing Considerations

All process.nextTick(() => self.destroy()) are hit except one. Below is the self.destroy() call that is not hit in these tests provided: https://github.com/nodejs/node/blob/9404d3aaaf0a8264ee08ba169ec24685bf8d487d/lib/net.js#L1113

I am not tagging this PR as "DRAFT" since the piece of code that isn't tested is for a connection timeout case.

@mcollina Please let me know if these tests are sufficient or not.

Merge request reports