Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore keepalives before connection_ack in websockets transport #110

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 18 additions & 8 deletions gql/transport/websockets.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,23 @@ async def _receive(self) -> str:

return answer

async def _wait_ack(self) -> None:
"""Wait for the connection_ack message. Keep alive messages are ignored
"""

while True:
init_answer = await self._receive()

answer_type, answer_id, execution_result = self._parse_answer(init_answer)

if answer_type == "connection_ack":
return

if answer_type != "ka":
raise TransportProtocolError(
"Websocket server did not return a connection ack"
)
KingDarBoja marked this conversation as resolved.
Show resolved Hide resolved

async def _send_init_message_and_wait_ack(self) -> None:
"""Send init message to the provided websocket and wait for the connection ACK.

Expand All @@ -188,14 +205,7 @@ async def _send_init_message_and_wait_ack(self) -> None:
await self._send(init_message)

# Wait for the connection_ack message or raise a TimeoutError
init_answer = await asyncio.wait_for(self._receive(), self.ack_timeout)

answer_type, answer_id, execution_result = self._parse_answer(init_answer)

if answer_type != "connection_ack":
raise TransportProtocolError(
"Websocket server did not return a connection ack"
)
await asyncio.wait_for(self._wait_ack(), self.ack_timeout)

async def _send_stop_message(self, query_id: int) -> None:
"""Send stop message to the provided websocket connection and query_id.
Expand Down
2 changes: 1 addition & 1 deletion tests/test_websocket_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ async def test_websocket_transport_protocol_errors(event_loop, client_and_server

async def server_without_ack(ws, path):
# Sending something else than an ack
await WebSocketServer.send_keepalive(ws)
await WebSocketServer.send_complete(ws, 1)
await ws.wait_closed()


Expand Down
40 changes: 40 additions & 0 deletions tests/test_websocket_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -472,3 +472,43 @@ async def test_websocket_add_extra_parameters_to_connect(event_loop, server):

async with Client(transport=sample_transport) as session:
await session.execute(query)


async def server_sending_keep_alive_before_connection_ack(ws, path):
await WebSocketServer.send_keepalive(ws)
await WebSocketServer.send_keepalive(ws)
await WebSocketServer.send_keepalive(ws)
await WebSocketServer.send_keepalive(ws)
await WebSocketServer.send_connection_ack(ws)
result = await ws.recv()
print(f"Server received: {result}")
await ws.send(query1_server_answer.format(query_id=1))
await WebSocketServer.send_complete(ws, 1)
await ws.wait_closed()


@pytest.mark.asyncio
@pytest.mark.parametrize(
"server", [server_sending_keep_alive_before_connection_ack], indirect=True
)
@pytest.mark.parametrize("query_str", [query1_str])
async def test_websocket_non_regression_bug_108(
event_loop, client_and_server, query_str
):

# This test will check that we now ignore keepalive message
# arriving before the connection_ack
# See bug #108

session, server = client_and_server

query = gql(query_str)

result = await session.execute(query)

print("Client received:", result)

continents = result["continents"]
africa = continents[0]

assert africa["code"] == "AF"