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

check network errors and their .__cause__ for expected error types #18849

Merged

Conversation

altendky
Copy link
Contributor

@altendky altendky commented Nov 12, 2024

Purpose:

We already have code to report 'expected' networking errors differently and more quietly than unexpected ones. For example, we expect peers to disconnect sometimes and don't need a loud traceback in the logs for that. But... it appears that there may have been a change where these exceptions are now being chained and thus not caught by the original check. This expands that check to check both the exception directly and it's .__cause__. That's where e goes when you, for example, raise NewException() from e.

2024-11-11T13:53:38.205 2.4.4 full_node full_node_server        : ERROR    Exception Stack: Traceback (most recent call last):
  File "/home/altendky/.pyenv/versions/3.10.13/lib/python3.10/asyncio/selector_events.py", line 949, in _write_ready
    n = self._sock.send(self._buffer)
BrokenPipeError: [Errno 32] Broken pipe

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/farm/chia-blockchain/chia/server/ws_connection.py", line 374, in outbound_handler
    await self._send_message(msg)
  File "/farm/chia-blockchain/chia/server/ws_connection.py", line 647, in _send_message
    await self.ws.send_bytes(encoded)
  File "/farm/chia-blockchain/.venv/lib/python3.10/site-packages/aiohttp/web_ws.py", line 394, in send_bytes
    await self._writer.send(data, binary=True, compress=compress)
  File "/farm/chia-blockchain/.venv/lib/python3.10/site-packages/aiohttp/http_websocket.py", line 714, in send
    await self._send_frame(message, WSMsgType.BINARY, compress)
  File "/farm/chia-blockchain/.venv/lib/python3.10/site-packages/aiohttp/http_websocket.py", line 678, in _send_frame
    await self.protocol._drain_helper()
  File "/farm/chia-blockchain/.venv/lib/python3.10/site-packages/aiohttp/base_protocol.py", line 95, in _drain_helper
    await asyncio.shield(waiter)
ConnectionError: Connection lost

Current Behavior:

New Behavior:

Testing Notes:

@altendky altendky added the Changed Required label for PR that categorizes merge commit message as "Changed" for changelog label Nov 12, 2024
@altendky altendky requested a review from a team as a code owner November 12, 2024 01:45
@altendky altendky requested a review from emlowe November 12, 2024 18:05
Copy link
Contributor

@emlowe emlowe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh yes

@pmaslana pmaslana merged commit ed74439 into main Nov 14, 2024
363 checks passed
@pmaslana pmaslana deleted the try_again_to_avoid_pointless_network_related_tracebacks branch November 14, 2024 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changed Required label for PR that categorizes merge commit message as "Changed" for changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants