Catch ClosedResourceError. Fixes #134 #140
Conversation
|
This test is not quite deterministic - it seems to fail every 10 invocations. I'm going to see if i can narrow down the exact sequence of steps. |
|
@belm0 I don't know if you can help me fix the test, the sequencing is a bit tricky - the issue seems to occur because _send_channel is closed while the reader task is still processing an event, so this line https://github.com/HyperionGray/trio-websocket/blob/master/trio_websocket/_impl.py#L1071 throws a trio.ClosedResourceError (trio.BrokenResourceError is only thrown when the _recv_channel is closed). |
|
@belm0 OK this test consistently fails before the fix. |
tests/test_connection.py
Outdated
| async def test_server_sends_after_close(nursery): | ||
| async def handler(request): | ||
| server_ws = await request.accept() | ||
| with pytest.raises(ConnectionClosed): | ||
| while True: | ||
| await server_ws.send_message('Hello from server') | ||
|
|
||
| server = await nursery.start(serve_websocket, handler, HOST, 0, None) | ||
| stream = await trio.open_tcp_stream(HOST, server.port) | ||
| client_ws = await wrap_client_stream(nursery, stream, HOST, RESOURCE) | ||
| async with client_ws: | ||
| # pump a few messages | ||
| for x in range(2): | ||
| await client_ws.send_message('Hello from client') | ||
| await stream.aclose() | ||
|
|
||
|
|
There was a problem hiding this comment.
due to the way the nursery fixture works, the nursery will be cancelled if the test function exits
so I'm concerned that there may be non-determinism since nothing ensures that the handler test code is able to complete
Usually in this case, I'd make a done = trio.Event() at the start of the test function, set() it at the end of handler(), and await done.wait() at the end of the test function.
This may explain the non-determinism in the original test, and you may be able to get away with only one send_message() now.
There was a problem hiding this comment.
I didn't realize that about the nursery fixture. I'll update the test.
I tried many iterations before I could reproducibly cause the failure. The trickiness is that it seems to a race condition between the reader_task and the main handler code on the server side, and in particular when there is data that hasn't been processed by the reader task but the underlying stream has already been closed. For example, changing line 740 above to await server_ws.get_message() doesn't trigger the error, and only sending a single message on the client side also doesn't trigger an error.
|
thank you for spending time on a test ❤️ |
No description provided.