Slow http CONNECT requests will block other requests when using ProxyAgent

[andrewfecenko]

Bug Description

When using ProxyAgent, a slow CONNECT request to the proxy will block the Client's queue (this[kClient] in proxy-agent.js) and can cause delays in issues any requests with that ProxyAgent.

Reproducible By

We found that the easiest way to reproduce this with a test was to add a delay for the first connect request to the proxy via proxy.authenticate:

test('delayed proxy-agent request blocks queue', async (t) => {
  t.plan(2)
  const server = await buildServer()
  const proxy = await buildProxy()

  const serverUrl = `http://localhost:${server.address().port}`
  const proxyUrl = `http://localhost:${proxy.address().port}`
  const proxyAgent = new ProxyAgent({auth: Buffer.from('user:pass').toString('base64'), uri: proxyUrl});
  let firstConnect = true;
  await new Promise(async (resolve) => {
    let connectCount = 0
    console.log('running test');
    proxy.authenticate = function (req, res) {
      if (++connectCount === 2) {
        console.log('allowing connect');
        firstConnect = false;
        res(null, true)
      } else {
        console.log('delaying connect');
        setTimeout(() => {
          console.log('delayed connect');
          t.equal(firstConnect, true);
          res(null, true);
        }, 1000);
      }
    }

    server.on('request', (req, res) => {
      res.end()
    })

    request(`${serverUrl}`, { dispatcher: proxyAgent })
      .then((resp) => {
        resp.body.on('error', () => {}).destroy()
      })
      .catch(() => {})
    request(`${serverUrl}`, { dispatcher: proxyAgent })
      .then((resp) => {
        resp.body.on('error', () => {}).destroy()
        resolve();
      })
      .catch(() => {})
  })
  console.log('done');
  t.pass()
  server.close()
  proxy.close()
  proxyAgent.close()
})

Expected Behavior

A single request issued with a ProxyAgent should not block other requests also using that ProxyAgent.

Environment

Ran the above test using node v19.4.0 on macOS Ventura 13.2.1

Additional context

We discovered this issue after investigating delays in requests that use undici's request function with a ProxyAgent passed in as the dispatcher. We were instantiating a single ProxyAgent and using it in all requests, but we have since made a change to instantiate a new ProxyAgent for every single request call. This appears to have fixed the issue where a single request blocks other successive requests.

I believe one potential solution is to allow the consumers of ProxyAgent to choose whether they'd like this[kClient] to be instantiated with an instance of Client or an instance of Pool. If I change proxy-agent to use a Pool instead the above test no longer passes (the first connect request does not block the successive ones).

[andrewfecenko]

As seen above, I've created a pull request with what I think are the necessary changes. Would love any and all thoughts