fix: prioritise error events over timeouts

[jodevsa]

fix: prioritise error events over timeouts

Error events in the implementation of net.connect are defered to the next tick: https://github.com/nodejs/node/blob/main/lib/net.js#L1144
This will result in the TO callback always being executed before the error events callback in the callback queue.

This fix makes sure that error event callbacks are executed before the TO socket destroy logic.

fixes #1484

[mcollina]

Good spot!

[jodevsa]

@mcollina please check again

[jodevsa]

tests are not working for windows

Transformation error for D:\a\undici\undici\lib\core\connect.js ; return original code EPERM: operation not permitted, rename 'D:\a\undici\undici\node_modules\.cache\nyc\fbfece793b45952b8a9635dc1bc6687ac6fb676105e3851ee22696fdcb8c0a14.js.349094085' -> 'D:\a\undici\undici\node_modules\.cache\nyc\fbfece793b45952b8a9635dc1bc6687ac6fb676105e3851ee22696fdcb8c0a14.js' Transformation error for D:\a\undici\undici\lib\mock\mock-symbols.js ; return original code

not sure what this is about

[jodevsa]

this change also seems to fix the example from the original issue

const { Pool } = require('undici')
const assert = require('assert')
const agent = new Pool('http://foobar.bar')

for (let i = 0; i < 50000; i++) {
  agent.request({ method: 'GET', path: '/foobar' })
    .then(async ({ body }) => {
      for await (const data of body) {
        console.log('data', data.toString('utf8'))
      }
    })
    .catch((e)=>assert.equal(e.code,'ENOTFOUND'))
}

[jodevsa]

my new test is failing on windows :(

[jodevsa]

@mcollina seems like on windows we'll need 2 setImmediates I still don't know why but I noticed we have different implementation for windows in net.js. is it fine if we add another setImmediate?

[mcollina]

Detect if we are on windows and add another immediate in that case. I would also recommend moving that logic in a function because it's a bit hard to read.

[jodevsa]

@mcollina please check again

[jodevsa]

It would be a good idea to cancel the setImmedates when the response/error is fired.. let me do that first and then I'll ping you for revewing

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.93%. Comparing base (7b25efc) to head (1ffe0e6).
Report is 1266 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1551      +/-   ##
==========================================
+ Coverage   94.91%   94.93%   +0.01%     
==========================================
  Files          50       50              
  Lines        4702     4715      +13     
==========================================
+ Hits         4463     4476      +13     
  Misses        239      239              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[jodevsa]

@mcollina please check again

[mcollina]

lgtm