fix(proxy): fail the request when the CONNECT tunnel drops instead of looping (#5441)

When a proxy tore down the socket while the CONNECT tunnel was still being
established, the inner client rejected with UND_ERR_SOCKET. The proxy-agent
catch only special-cased ERR_TLS_CERT_ALTNAME_INVALID and passed every other
error through raw. client.js#onError treats UND_ERR_SOCKET as a recoverable
error on an established connection, so with no running request it left the
request queued and connect() was re-driven forever, hammering the proxy while
the request never settled.

Wrap a tunnel-establishment socket failure in a new ProxyConnectionError
(UND_ERR_PRX_CONN) so onError fails the request instead of retrying, mirroring
the existing SecureProxyConnectionError handling for the TLS case.

Fixes #3897

Author: cesarvspr

docs/docs/api/Errors.md

@@ -26,6 +26,7 @@ import { errors } from 'undici'
 | `InformationalError`                 | `UND_ERR_INFO`                        | expected error with reason                                                |
 | `ResponseExceededMaxSizeError`       | `UND_ERR_RES_EXCEEDED_MAX_SIZE`       | response body exceed the max size allowed                                 |
 | `SecureProxyConnectionError`         | `UND_ERR_PRX_TLS`                     | tls connection to a proxy failed                                          |
+| `ProxyConnectionError`               | `UND_ERR_PRX_CONN`                    | establishing the proxy `CONNECT` tunnel failed                            |
 | `MessageSizeExceededError`           | `UND_ERR_WS_MESSAGE_SIZE_EXCEEDED`    | WebSocket decompressed message exceeded the maximum allowed size          |
 | `AbortError`                         | `UND_ERR_ABORT`                       | the operation was aborted (base class of `RequestAbortedError`).          |
 | `RequestRetryError`                  | `UND_ERR_REQ_RETRY`                   | request failed and could not be retried; carries `statusCode`, `headers` and `data`. |

lib/core/errors.js

@@ -403,6 +403,25 @@ class SecureProxyConnectionError extends UndiciError {
   }
 }
 
+const kProxyConnectionError = Symbol.for('undici.error.UND_ERR_PRX_CONN')
+class ProxyConnectionError extends UndiciError {
+  constructor (cause, message, options = {}) {
+    super(message, { cause, ...options })
+    this.name = 'ProxyConnectionError'
+    this.message = message || 'Proxy Connection failed'
+    this.code = 'UND_ERR_PRX_CONN'
+    this.cause = cause
+  }
+
+  static [Symbol.hasInstance] (instance) {
+    return instance && instance[kProxyConnectionError] === true
+  }
+
+  get [kProxyConnectionError] () {
+    return true
+  }
+}
+
 const kMaxOriginsReachedError = Symbol.for('undici.error.UND_ERR_MAX_ORIGINS_REACHED')
 class MaxOriginsReachedError extends UndiciError {
   constructor (message) {
@@ -471,6 +490,7 @@ module.exports = {
   RequestRetryError,
   ResponseError,
   SecureProxyConnectionError,
+  ProxyConnectionError,
   MaxOriginsReachedError,
   Socks5ProxyError,
   MessageSizeExceededError

lib/dispatcher/proxy-agent.js

@@ -4,7 +4,7 @@ const { kProxy, kClose, kDestroy, kDispatch } = require('../core/symbols')
 const Agent = require('./agent')
 const Pool = require('./pool')
 const DispatcherBase = require('./dispatcher-base')
-const { InvalidArgumentError, RequestAbortedError, SecureProxyConnectionError } = require('../core/errors')
+const { InvalidArgumentError, RequestAbortedError, SecureProxyConnectionError, ProxyConnectionError } = require('../core/errors')
 const buildConnector = require('../core/connect')
 const Client = require('./client')
 const { channels } = require('../core/diagnostics')
@@ -231,6 +231,14 @@ class ProxyAgent extends DispatcherBase {
           if (err.code === 'ERR_TLS_CERT_ALTNAME_INVALID') {
             // Throw a custom error to avoid loop in client.js#connect
             callback(new SecureProxyConnectionError(err))
+          } else if (err.code === 'UND_ERR_SOCKET') {
+            // A socket failure while establishing the tunnel means the CONNECT
+            // never completed, so there is nothing to recover - the proxy just
+            // tore down the connection. client.js#onError treats UND_ERR_SOCKET
+            // as a recoverable error on an established connection and leaves the
+            // request queued, which makes connect() retry forever. Surface it as
+            // a non-recoverable proxy error so the request fails instead. (#3897)
+            callback(new ProxyConnectionError(err))
           } else {
             callback(err)
           }

test/issue-3897.js

@@ -0,0 +1,53 @@
+'use strict'
+
+const { tspl } = require('@matteo.collina/tspl')
+const { test, after } = require('node:test')
+const { createServer } = require('node:http')
+const { once } = require('node:events')
+const { request } = require('..')
+const { ProxyConnectionError } = require('../lib/core/errors')
+const ProxyAgent = require('../lib/dispatcher/proxy-agent')
+
+// Regression test for https://github.com/nodejs/undici/issues/3897
+//
+// When the proxy tears down the socket while the CONNECT tunnel is being
+// established, the inner client rejects with UND_ERR_SOCKET. client.js#onError
+// treats UND_ERR_SOCKET as a recoverable error on an established connection and
+// leaves the request queued, so connect() is retried forever - the proxy gets
+// hammered with CONNECT attempts and the request never settles. The fix surfaces
+// a tunnel-establishment socket failure as a non-recoverable ProxyConnectionError
+// so the request fails after a single attempt.
+test('a proxy that drops the CONNECT tunnel fails the request instead of looping', { timeout: 5000 }, async (t) => {
+  t = tspl(t, { plan: 3 })
+
+  let connectAttempts = 0
+  const proxy = createServer()
+  proxy.on('connect', (req, socket) => {
+    connectAttempts++
+    // Tear the tunnel down before it is established, like a proxy that does not
+    // implement CONNECT or rejects the upstream.
+    socket.destroy()
+  })
+
+  proxy.listen(0)
+  await once(proxy, 'listening')
+
+  const proxyUrl = `http://127.0.0.1:${proxy.address().port}`
+  const proxyAgent = new ProxyAgent(proxyUrl)
+
+  after(async () => {
+    await proxyAgent.close()
+    proxy.close()
+  })
+
+  await t.rejects(
+    request('http://localhost/', { dispatcher: proxyAgent }),
+    (err) => {
+      t.ok(err instanceof ProxyConnectionError)
+      t.strictEqual(connectAttempts, 1)
+      return true
+    }
+  )
+
+  await t.completed
+})

test/types/errors.test-d.ts

@@ -110,6 +110,11 @@ expectAssignable<errors.SecureProxyConnectionError>(new errors.SecureProxyConnec
 expectAssignable<'SecureProxyConnectionError'>(new errors.SecureProxyConnectionError().name)
 expectAssignable<'UND_ERR_PRX_TLS'>(new errors.SecureProxyConnectionError().code)
 
+expectAssignable<errors.UndiciError>(new errors.ProxyConnectionError())
+expectAssignable<errors.ProxyConnectionError>(new errors.ProxyConnectionError())
+expectAssignable<'ProxyConnectionError'>(new errors.ProxyConnectionError().name)
+expectAssignable<'UND_ERR_PRX_CONN'>(new errors.ProxyConnectionError().code)
+
 expectAssignable<errors.UndiciError>(new errors.MaxOriginsReachedError())
 expectAssignable<errors.MaxOriginsReachedError>(new errors.MaxOriginsReachedError())
 expectAssignable<'MaxOriginsReachedError'>(new errors.MaxOriginsReachedError().name)

types/errors.d.ts

@@ -154,6 +154,16 @@ declare namespace Errors {
     code: 'UND_ERR_PRX_TLS'
   }
 
+  export class ProxyConnectionError extends UndiciError {
+    constructor (
+      cause?: Error,
+      message?: string,
+      options?: Record<any, any>
+    )
+    name: 'ProxyConnectionError'
+    code: 'UND_ERR_PRX_CONN'
+  }
+
   export class MaxOriginsReachedError extends UndiciError {
     name: 'MaxOriginsReachedError'
     code: 'UND_ERR_MAX_ORIGINS_REACHED'