Skip to content

Commit f6cf32d

Browse files
committed
Change send_message() behavior when close handshake is in progress
send_message() is changed to raise ConnectionClosed when a close handshake is in progress. Previously it would silently ignore the call, which was an oversight, given that ConnectionClosed is defined to cover connections "closed or in the process of closing". Significantly, this fixes send_message() leaking a wsproto exception (LocalProtocolError) with wsproto >= 1.22.0. Fixes #175.
1 parent 9bcb7aa commit f6cf32d

File tree

2 files changed

+9
-13
lines changed

2 files changed

+9
-13
lines changed

tests/test_connection.py

+2-9
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@
4040
import wsproto
4141
from trio.testing import memory_stream_pair
4242
from wsproto.events import CloseConnection
43-
from wsproto.utilities import LocalProtocolError
4443

4544
try:
4645
from trio.lowlevel import current_task # pylint: disable=ungrouped-imports
@@ -928,14 +927,10 @@ async def handler(request: WebSocketRequest):
928927
await trio.sleep(.1)
929928

930929

931-
@pytest.mark.xfail(
932-
reason='send_message() API oversight for closing-in-process case',
933-
raises=None if WS_PROTO_VERSION < (1, 2, 0) else LocalProtocolError,
934-
strict=True)
935930
async def test_remote_close_local_message_race(nursery, autojump_clock):
936931
"""as remote initiates close, local attempts message (issue #175)
937932
938-
This exposes multiple problems in the trio-websocket API and implementation:
933+
This exposed multiple problems in the trio-websocket API and implementation:
939934
* send_message() silently fails if a close is in progress. This was
940935
likely an oversight in the API, since send_message() raises `ConnectionClosed`
941936
only in the already-closed case, yet `ConnectionClosed` is defined to cover
@@ -957,9 +952,7 @@ async def handler(request: WebSocketRequest):
957952
await client.send_message('foo')
958953
await client._for_testing_peer_closed_connection.wait()
959954
with pytest.raises(ConnectionClosed):
960-
await client.send_message('bar') # wsproto < 1.2.0: silently ignored
961-
# wsproto >= 1.2.0: raises LocalProtocolError
962-
# desired: raises ConnectionClosed
955+
await client.send_message('bar')
963956

964957

965958
@fail_after(DEFAULT_TEST_MAX_DURATION)

trio_websocket/_impl.py

+7-4
Original file line numberDiff line numberDiff line change
@@ -932,7 +932,7 @@ async def send_message(self, message):
932932
933933
:param message: The message to send.
934934
:type message: str or bytes
935-
:raises ConnectionClosed: if connection is already closed.
935+
:raises ConnectionClosed: if connection is closed, or being closed
936936
'''
937937
if self._close_reason:
938938
raise ConnectionClosed(self._close_reason)
@@ -1096,10 +1096,13 @@ async def _handle_close_connection_event(self, event):
10961096
10971097
:param wsproto.events.CloseConnection event:
10981098
'''
1099-
if self._for_testing_peer_closed_connection:
1100-
self._for_testing_peer_closed_connection.set()
1101-
await trio.sleep(0)
11021099
if self._wsproto.state == ConnectionState.REMOTE_CLOSING:
1100+
# Set _close_reason in advance, so that send_message() will raise
1101+
# ConnectionClosed during the close handshake.
1102+
self._close_reason = CloseReason(event.code, event.reason or None)
1103+
if self._for_testing_peer_closed_connection:
1104+
self._for_testing_peer_closed_connection.set()
1105+
await trio.sleep(0)
11031106
await self._send(event.response())
11041107
await self._close_web_socket(event.code, event.reason or None)
11051108
self._close_handshake.set()

0 commit comments

Comments
 (0)