restore AsyncExitStack, and use it correctly #96
Conversation
| await self.aclose() | ||
| await self._task_group.__aexit__(exc_type, exc_val, exc_tb) |
There was a problem hiding this comment.
before it was possible for this aclose to raise eg CancelledError, but then it would not be raised into the taskgroup so the exception would not be processed
| await self.aclose() | ||
| await self._task_group.__aexit__(exc_type, exc_val, exc_tb) | ||
| async def __aexit__(self, exc_type, exc_val, exc_tb) -> bool | None: | ||
| return await self._exit_stack.__aexit__(exc_type, exc_val, exc_tb) |
There was a problem hiding this comment.
before the return code was not passed from TaskGroup.__aexit__ so if it suppressed/propagated cancellation it would not then be suppressed/propagated from ASGIWebSocketAsyncNetworkStream
| self._task_group = await stack.enter_async_context(anyio.create_task_group()) | ||
| self._task_group.start_soon(self._run) | ||
|
|
||
| await self.send({"type": "websocket.connect"}) |
There was a problem hiding this comment.
if send was cancelled it would not propagate into the TaskGroup async context manager!
This was fixed by using two AsyncExitStacks and pop_all(), so all code is covered by a context manager and therefore the TaskGroup.
httpx_ws/transport.py
Outdated
| if message["type"] == "websocket.close": | ||
| raise WebSocketDisconnect(message["code"], message.get("reason")) |
There was a problem hiding this comment.
aclose was run here and then later in the __aexit__, I now run it once using a callback in the AsyncExitStack
|
Well, I suspected removing the exit stack would be a problem; I should I've trusted my gut feelings 😄 Thank you for this, and for the detailed rationale behind it. May I ask you to run the linters ( |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #96 +/- ##
==========================================
- Coverage 98.72% 98.56% -0.17%
==========================================
Files 5 5
Lines 548 556 +8
==========================================
+ Hits 541 548 +7
- Misses 7 8 +1 ☔ View full report in Codecov by Sentry. |
this exit stack was recently removed, however it's absolutely critical it gets used - and correctly.