Close the tunnel connection when TLS setup fails after CONNECT

IT
4d ago
+248-4

Fixes #1175.

The bug

In TunnelHTTPConnection.handle_request() (_sync/_async http_proxy.py), a proxy CONNECT that succeeds leaves the underlying stream registered on self._connection. If the subsequent stream.start_tls() call then raises (e.g. a certificate/handshake failure), the exception was left to propagate without closing self._connection first — unlike the CONNECT-status-check branch a few lines above it, which already does exactly this on a non-2xx CONNECT response:

if connect_response.status < 200 or connect_response.status > 299:
    ...
    self._connection.close()
    raise ProxyError(msg)

self._connection already completed one successful request (the CONNECT) and self._connected stays False, so it keeps accurately reporting itself as ACTIVE, Request Count: 1 to the pool — nothing ever told it the tunnel died. The pool's own exception handling in handle_request() only removes the failed request from its queue, never the connection from self._connections, so it has no other way to notice. Each failed handshake permanently consumes a connection-pool slot; once failures reach max_connections, unrelated later requests raise PoolTimeout instead of the real ConnectError.

The fix

Wrap start_tls() the same way the status-check block already does:

with Trace("start_tls", logger, request, kwargs) as trace:
    try:
        stream = stream.start_tls(**kwargs)
    except BaseException:
        self._connection.close()  # aclose() in the async variant
        raise
    trace.return_value = stream

Applied to both _sync/http_proxy.py and _async/http_proxy.py (they're kept in sync via scripts/unasync.py, which I re-ran after editing the async source).

Testing

Added test_proxy_tunneling_with_tls_failure to both tests/httpcore2/_sync/test_http_proxy.py and tests/httpcore2/_async/test_http_proxy.py, following the existing MockStream/MockBackend pattern used by test_proxy_tunneling_with_403: a FailingTLSStream whose start_tls always raises ConnectError, run three times against a pool with max_connections=2, asserting not proxy.connections after every failure.

  • Reverting only the fix reproduces the exact bug: the test fails with a stale ACTIVE TunnelHTTPConnection left in proxy.connections.
  • With the fix applied, all three failures leave the pool empty, as expected.
  • Also independently reproduced the original discussion's own repro script against this branch (before/after), matching its exact ConnectError → ConnectError → PoolTimeout sequence when reverted, and ConnectError ×3 with an empty pool after each, when fixed.
  • ruff check / ruff format --check and mypy clean on all four touched files.
  • Full tests/httpcore2 suite: 235 passed, only 3 pre-existing keepalive_expiry=0.0 timing flakes remain, which I confirmed also fail identically on unmodified main (unrelated to this change).
  • scripts/unasync.py --check passes (sync/async stay in lockstep).