Unable to read the content of WebSocketUpgradeError.response

[WSH032]

Describe the bug

Unable to read the content of WebSocketUpgradeError.response, because the stream has already been closed.

To Reproduce

Launch a server to send a denial handshake response.

from fastapi import FastAPI, WebSocket
from fastapi.responses import Response
from uvicorn import run

app = FastAPI()


@app.websocket("/")
async def _(ws: WebSocket):
    resp = Response(content="foo")
    await ws.send_denial_response(resp)


run(app)

Next, attempt the handshake using httpx-ws and read the response content after the handshake fails.

from contextlib import AsyncExitStack

import anyio
from httpx_ws import WebSocketUpgradeError, aconnect_ws


async def main():
    async with AsyncExitStack() as exit_stack:
        try:
            await exit_stack.enter_async_context(aconnect_ws("ws://127.0.0.1:8000/"))
        except WebSocketUpgradeError as e:
            # httpx.StreamClosed: Attempted to read or stream content, but the stream has been closed.
            assert await e.response.aread() == b"foo"


anyio.run(main)

Then you can see following error:

  File "e:\GitHub\fastapi-proxy\Untitled-3.py", line 12, in main
    await e.response.aread()
  File "E:\GitHub\fastapi-proxy\.venv\lib\site-packages\httpx\_models.py", line 911, in aread
    self._content = b"".join([part async for part in self.aiter_bytes()])
  File "E:\GitHub\fastapi-proxy\.venv\lib\site-packages\httpx\_models.py", line 911, in <listcomp>
    self._content = b"".join([part async for part in self.aiter_bytes()])
  File "E:\GitHub\fastapi-proxy\.venv\lib\site-packages\httpx\_models.py", line 929, in aiter_bytes
    async for raw_bytes in self.aiter_raw():
  File "E:\GitHub\fastapi-proxy\.venv\lib\site-packages\httpx\_models.py", line 978, in aiter_raw
    raise StreamClosed()
httpx.StreamClosed: Attempted to read or stream content, but the stream has been closed.

Expected behavior

The propagation of WebSocketUpgradeError leads to the exit of the client.stream context manager, then the response content is no longer readable.

httpx-ws/httpx_ws/_api.py

Lines 1048 to 1050 in 940c9ad

	with client.stream("GET", url, headers=headers, **kwargs) as response:
	if response.status_code != 101:
	raise WebSocketUpgradeError(response)

Can we make a compromise: request the user to manually close the response when WebSocketUpgradeError is raised, instead of closing it automatically?

except WebSocketUpgradeError as e:
    await e.aclose()

Configuration

fastapi == 0.110.1
uvicorn == 0.29.0
httpx-ws == 0.6.0

[frankie567]

Hmm, I see! Interesting case 😅

I would prefer to keep the normal functioning of the context manager, to keep a good developer experience.

Before raising the exception, could we actually read the content? I believe then HTTPX will cache it and make it available immediately on further reads. Something like:

    async with client.stream("GET", url, headers=headers, **kwargs) as response:
        if response.status_code != 101:
            await response.aread()
            raise WebSocketUpgradeError(response)

What do you think?

[frankie567]

Indeed, httpx does cache it, but this increases IO time, and not every user has the need to read the content.

True but, in this context, it would be quite unexpected to have a big response body, so I would say that's acceptable 🤔

^{_{*Sorry, I edited your previous message by mistake; I thought I clicked on Quote ☺️}}

[WSH032]

in this context, it would be quite unexpected to have a big response body

But if the user use an incorrect URL, leading them to believe they are establishing a WebSocket request, while in reality, they are making an HTTP file request, this situation can occur.

for example:

aconnect_ws("https://speed.hetzner.de/100MB.bin")

[WSH032]

I just discovered that httpx-ws doesn't seem to perform handshake checks, referring to the wiki:

In addition to Upgrade headers, the client sends a Sec-WebSocket-Key header containing base64-encoded random bytes, and the server replies with a hash of the key in the Sec-WebSocket-Accept header. This is intended to prevent a caching proxy from re-sending a previous WebSocket conversation,[37] and does not provide any authentication, privacy, or integrity. The hashing function appends the fixed string 258EAFA5-E914-47DA-95CA-C5AB0DC85B11 (a UUID) to the value from Sec-WebSocket-Key header (which is not decoded from base64), applies the SHA-1 hashing function, and encodes the result using base64.[38]

Currently, httpx-ws only checks for the 101 status code but does not verify the Sec-WebSocket-Key.

But this is another issue. Even if the handshake check is done correctly, if the server insists on sending a large response body, it can still bite the user.

[frankie567]

Currently, httpx-ws only checks for the 101 status code but does not verify the Sec-WebSocket-Key.

Indeed 🙃 Feel free to open an issue about this.

But this is another issue. Even if the handshake check is done correctly, if the server insists on sending a large response body, it can still bite the user.

True, true... 🤔 What we could do is to provide a lower-level API where you could have more control of the stream so you can read the response body. However, we would keep the current API like this for which, in 99% of the case, people won't even care about the response.

In a sense, it's quite similar to what HTTPX does for stream: https://www.python-httpx.org/async/#streaming-responses

[WSH032]

in 99% of the case, people won't even care about the response.

I agree with you

---

In a sense, it's quite similar to what HTTPX does for stream: https://www.python-httpx.org/async/#streaming-responses

Do you mean that?

For situations when context block usage is not practical, it is possible to enter "manual mode" by sending a Request instance using client.send(..., stream=True).

Example in the context of forwarding the response to a streaming web endpoint with Starlette:

I'm doing something similar. I'm attempting to implement a transparent WebSocket proxy server and encountered the issue being discussed during testing.

https://github.com/WSH032/fastapi-proxy-lib/blob/61192fcdccf4e7c85f65db2394b06d145f6f9b25/src/fastapi_proxy_lib/core/websocket.py#L414-L441