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

Properly handle websocket transport errors and recovery #1897

Merged
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changes/1897.bugfix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Properly handle websocket transport errors and recover
- Additionally, errors will now include additional information
9 changes: 9 additions & 0 deletions hikari/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
"InternalServerError",
"ShardCloseCode",
"GatewayConnectionError",
"GatewayTransportError",
"GatewayServerClosedConnectionError",
"GatewayError",
"MissingIntentWarning",
Expand Down Expand Up @@ -176,6 +177,14 @@ def is_standard(self) -> bool:
return (self.value // 1000) == 1


@attrs.define(auto_exc=True, repr=False, slots=False)
class GatewayTransportError(GatewayError):
"""An exception thrown if an issue occurs at the transport layer."""

def __str__(self) -> str:
return f"Gateway transport error: {self.reason!r}"


@attrs.define(auto_exc=True, repr=False, slots=False)
class GatewayConnectionError(GatewayError):
"""An exception thrown if a connection issue occurs."""
Expand Down
10 changes: 7 additions & 3 deletions hikari/impl/shard.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,10 +210,10 @@ async def send_json(self, data: data_binding.JSONObject) -> None:

def _handle_other_message(self, message: aiohttp.WSMessage, /) -> typing.NoReturn:
if message.type == aiohttp.WSMsgType.TEXT:
raise errors.GatewayError("Unexpected message type received TEXT, expected BINARY")
raise errors.GatewayTransportError("Unexpected message type received TEXT, expected BINARY")

if message.type == aiohttp.WSMsgType.BINARY:
raise errors.GatewayError("Unexpected message type received BINARY, expected TEXT")
raise errors.GatewayTransportError("Unexpected message type received BINARY, expected TEXT")

if message.type == aiohttp.WSMsgType.CLOSE:
close_code = int(message.data)
Expand All @@ -229,7 +229,8 @@ def _handle_other_message(self, message: aiohttp.WSMessage, /) -> typing.NoRetur
raise errors.GatewayConnectionError("Socket has closed")

# Assume exception for now.
raise errors.GatewayError("Unexpected websocket exception from gateway") from self._ws.exception()
reason = f"{message.data!r} [extra={message.extra!r}, type={message.type}]"
raise errors.GatewayTransportError(reason) from self._ws.exception()

async def _receive_and_check_text(self) -> str:
message = await self._ws.receive()
Expand Down Expand Up @@ -920,6 +921,9 @@ async def _keep_alive(self) -> None:
except errors.GatewayConnectionError as ex:
self._logger.warning("failed to communicate with server, reason was: %r. Will retry shortly", ex.reason)

except errors.GatewayTransportError as ex:
self._logger.error("encountered transport error. Will try to reconnect shorty", exc_info=ex)

except errors.GatewayServerClosedConnectionError as ex:
if not ex.can_reconnect:
self._logger.info(
Expand Down
2 changes: 1 addition & 1 deletion tests/hikari/impl/test_shard.py
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ def test__handle_other_message_when_message_type_is_unknown(self, transport_impl
exception = Exception("some error")
transport_impl._ws.exception = mock.Mock(return_value=exception)

with pytest.raises(errors.GatewayError, match="Unexpected websocket exception from gateway") as exc_info:
with pytest.raises(errors.GatewayTransportError) as exc_info:
transport_impl._handle_other_message(stub_response)

assert exc_info.value.__cause__ is exception
Expand Down
Loading