Skip to content

Stop parsing completed HTTP proxy responses. - #1762

Open
Sakshamm-Goyal wants to merge 1 commit into
python-websockets:mainfrom
Sakshamm-Goyal:agent/fix-http-proxy-terminal-lifecycle
Open

Sakshamm-Goyal wants to merge 1 commit into
python-websockets:mainfrom
Sakshamm-Goyal:agent/fix-http-proxy-terminal-lifecycle

Conversation

@Sakshamm-Goyal

Copy link
Copy Markdown

Stop parsing completed HTTP proxy responses

When an HTTP proxy rejects CONNECT with 407 and closes the connection, connect() raises InvalidProxyStatus as expected, but the asyncio callback also raises AttributeError by resuming the completed response parser. EOF followed by connection loss can also feed EOF twice.

Treat the completed response future as terminal and share EOF handling. Seven tests cover success, rejection, malformed responses, cancellation, incomplete responses and trailing data. A local authenticated CONNECT tunnel reproduces the callback error before the fix and verifies echo/rejection without callback errors afterward.

Python 3.13 tests, full and per-module 100% coverage, Ruff and strict mypy pass. An initial synchronous reconnect timing failure passed on rerun with the repository CI timeout factor. Other Python/platform combinations weren't run locally.

Prepared and validated with an AI coding assistant. This is separate from the credential-decoding behavior in #1761.

@aaugustin

Copy link
Copy Markdown
Member

What problem is this actually causing in real life?

@aaugustin

Copy link
Copy Markdown
Member

the asyncio callback also raises AttributeError

What is "the asyncio callback" in this context?

@Sakshamm-Goyal

Copy link
Copy Markdown
Author

The concrete effect I reproduced is an extra, uncaught event-loop exception during cleanup after a proxy rejects CONNECT with 407 (for example, incorrect proxy credentials). connect() still raises the correct InvalidProxyStatus; this fix doesn't make the rejected connection succeed. I don't have a production incident or evidence of data loss or a hang to claim.

By “the asyncio callback” I meant _SelectorSocketTransport._call_connection_lost(), which calls HTTPProxyConnection.connection_lost(). The traceback is:

_SelectorSocketTransport._call_connection_lost()
  -> HTTPProxyConnection.connection_lost()
  -> HTTPProxyConnection.run_parser()
  -> response.status_code
AttributeError: 'NoneType' object has no attribute 'status_code'

The CONNECT response parser already completed when it reported 407. Cleanup resumes that exhausted generator, whose next StopIteration has no response value.

I re-ran the loopback integration on Python 3.13.11: a real local CONNECT proxy, a WebSocket echo server, and the public connect(..., proxy=...) API. Correct credentials echo successfully; incorrect credentials raise InvalidProxyStatus on both versions. With the pre-fix client at acff86b, the loop's exception handler additionally receives the AttributeError above. With this PR's head, it doesn't. So the demonstrated user-facing problem is a spurious asyncio error log / exception-handler invocation on an otherwise correctly handled proxy rejection, not a failed error return from connect().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants