aio-libs / aio-libs/aiohttp

WSS receive_json/string/binary API error handling

Offen
#8,581 5 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen
enhancement
Vorherrschende Sprache
Python
Sterne
16.5k
Forks
2.4k
Ø Merge
17 Std. 22 Min.
Gemergte PRs (30 T.)
212

Beschreibung

### Describe the solution you'd like

The current API provides for a very bizarre/absent error handling mechanism for web socket receive operations.

Specifically these are the current implementations for receiving `string`, `bytes` and `json`:

https://github.com/aio-libs/aiohttp/blob/9ed3841f5d920360da1937eb440ff9cb60e7c2c6/aiohttp/client_ws.py#L298-L320

As you can see any message that is not of the type `TEXT` or `BINARY` will result in a `TypeError` raised. However, if we inspect the underlying `receive`, we'll see that whole bunch of error and state conditions will produce messages that are neither `TEXT` nor `BINARY`, including `WS_CLOSED_MESSAGE`, `WSMsgType.CLOSED` and `WSMsgType.ERROR` (this one is internal API implementation and should never be exposed to a user anyway) [emphasis added by `****`]:

```python
async def receive(self, timeout: Optional[float] = None) -> WSMessage:
while True:
if self._waiting:
raise RuntimeError("Concurrent call to receive() is not allowed")

if self._closed:
**** return WS_CLOSED_MESSAGE
elif self._closing:
await self.close()
**** return WS_CLOSED_MESSAGE

try:
self._waiting = True
try:
async with async_timeout.timeout(timeout or self._receive_timeout):
msg = await self._reader.read()
self._reset_heartbeat()
finally:
self._waiting = False
if self._close_wait:
set_result(self._close_wait, None)
except (asyncio.CancelledError, asyncio.TimeoutError):
self._close_code = WSCloseCode.ABNORMAL_CLOSURE
raise
except EofStream:
self._close_code = WSCloseCode.OK
await self.close()
**** return WSMessage(WSMsgType.CLOSED, None, None)
except ClientError:
self._closed = True
self._close_code = WSCloseCode.ABNORMAL_CLOSURE
**** return WS_CLOSED_MESSAGE
except WebSocketError as exc:
self._close_code = exc.code
await self.close(code=exc.code)
**** return WSMessage(WSMsgType.ERROR, exc, None)
except Exception as exc:
self._exception = exc
self._closing = True
self._close_code = WSCloseCode.ABNORMAL_CLOSURE
await self.close()
**** return WSMessage(WSMsgType.ERROR, exc, None)

if msg.type is WSMsgType.CLOSE:
self._closing = True
self._close_code = msg.data
if not self._closed and self._autoclose:
await self.close()
elif msg.type is WSMsgType.CLOSING:
self._closing = True
elif msg.type is WSMsgType.PING and self._autoping:
await self.pong(msg.data)
continue
elif msg.type is WSMsgType.PONG and self._autoping:
continue

return msg
```

So, what happens when you `receive_str` on a closed web socket? - `TypeError`
Server-initiated socket closing? `TypeError`
TCP connection reset and disconnected? `TypeError`

The `TypeError` is a builtin, its only payload is a string and information about the state or exception is entirely lost except as encoded within the error string. This is clearly not a desired API behavior.

In my application internally I had to implement my own wrapper of the `receive` as follows to ensure that:

1) If WSS is closed normally a `None` is returned to indicate `EOF`.
2) If we receive an unsolicited server closure, a `ServerDisconnectedError` is raised.
3) If an exception occurs during a `receive`, the exception is unwrapped from the `WSMessage(WSMsgType.ERROR, exc, None)` and re-raised.

The code is as follows and I believe the API should change along the similar lines:

```python
async def ws_receive(self, ws_conn: ClientWebSocketResponse):
async def receive_str(timeout: float | None = None) -> str | None:
msg = await ws_conn.receive(timeout)
if msg.type == WSMsgType.CLOSE:
raise ServerDisconnectedError()
if msg.type in (WSMsgType.CLOSING, WSMsgType.CLOSED):
return None
if msg.type == WSMsgType.ERROR:
raise msg.data
if msg.type != WSMsgType.TEXT:
raise TypeError(f"Received message {msg.type}:{msg.data!r} is not str")
return cast(str, msg.data)

async def receive_json(
loads: JSONDecoder = DEFAULT_JSON_DECODER,
timeout: float | None = None, ) -> Any:
data = await receive_str(timeout=timeout)
if data is None:
return None
return loads(data)

return await receive_json()
```

### Code of Conduct

- [X] I agree to follow the aio-libs Code of Conduct

Beitragsleitfaden

Beitragsleitfaden öffnen

Bewertung

Dieses Issue wurde noch nicht bewertet.

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.