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

WSS: _pong_not_received works (mostly) correctly but outcome is erased by receive's except EofStream: #8540

Closed
1 task done
arcivanov opened this issue Jul 27, 2024 · 0 comments · Fixed by #8546
Closed
1 task done
Labels

Comments

@arcivanov
Copy link
Contributor

arcivanov commented Jul 27, 2024

Describe the bug

I'm testing a condition where a network connection is severed to see how application behaves and recovers. This is accomplished by switching the laptop into an airplane mode, disabling the WiFi interface, i.e. a hard disconnect.
I noticed that application shuts down as if receiving a normal exit signal, which isn't supposed to happen.

Tracked this down to the following:

When connection is severed with my activated heartbeat of 5.0, _pong_not_received works mostly correctly:

def _pong_not_received(self) -> None:
if not self._closed:
self._closed = True
self._close_code = WSCloseCode.ABNORMAL_CLOSURE
self._exception = asyncio.TimeoutError()
self._response.close()

However, the concurrent receive that returns some time thereafter with EofStream erases all information about the abnormal shutdown:

except EofStream:
self._close_code = WSCloseCode.OK
await self.close()
return WSMessage(WSMsgType.CLOSED, None, None)

Additionally, and corollary to the above, the way _pong_not_received closes the connection does not unblock the receive at all, because self._close is set as opposed to calling self.close(), which would then feed receive termination data immediately. That doesn't happen and when receive finally receives the EofStream, it's oblivious as to the cause.

I'm working on a patch.

To Reproduce

  1. Connect to WSS endpoint and set frequent heartbeats and start receiving.
  2. Sever network connection for long enough for heartbeat to timeout.
  3. Observe that receive returns WSCloseCode.OK waaaay past the heartbeat timeout with no indication anything went wrong.

Expected behavior

receive:

  1. does not erase the error information from the _pong_not_received when EofStream is received.
  2. terminates immediately upon _pong_not_received with proper termination cause.
  3. _pong_not_received should probably use self.close()

Logs/tracebacks

Above

Python Version

$ python --version
Not relevant, but `Python 3.12.4`

aiohttp Version

$ python -m pip show aiohttp
3.9.5

multidict Version

$ python -m pip show multidict
Not relevant

yarl Version

$ python -m pip show yarl
Not relevant

OS

Fedora 40, Linux, x86_64

Related component

Client

Additional context

No response

Code of Conduct

  • I agree to follow the aio-libs Code of Conduct
@arcivanov arcivanov added the bug label Jul 27, 2024
arcivanov added a commit to arcivanov/aiohttp that referenced this issue Jul 27, 2024
Make sure to unblock the `receive` operation by feeding the receiver
an error in a `WSMessage`
Change `TimeoutError` to `ServerTimeoutError` to accurately represent failure (this is backwards compatible since `ServerTimeoutError` has `TimeoutError` in the MRO)

fixes aio-libs#8540
arcivanov added a commit to arcivanov/aiohttp that referenced this issue Jul 28, 2024
Make sure to unblock the `receive` operation by feeding the receiver
an error in a `WSMessage`
Change `TimeoutError` to `ServerTimeoutError` to accurately represent failure (this is backwards compatible since `ServerTimeoutError` has `TimeoutError` in the MRO)

fixes aio-libs#8540
arcivanov added a commit to arcivanov/aiohttp that referenced this issue Jul 28, 2024
Make sure to unblock the `receive` operation by feeding the receiver an error in a `WSMessage`
Change `TimeoutError` to `ServerTimeoutError` to accurately represent failure (this is backwards compatible since `ServerTimeoutError` has `TimeoutError` in the MRO)

fixes aio-libs#8540
arcivanov added a commit to arcivanov/aiohttp that referenced this issue Jul 29, 2024
Make sure to unblock the `receive` operation by feeding the receiver an error in a `WSMessage`

Change `TimeoutError` to `ServerTimeoutError` to accurately represent failure (this is backwards compatible since `ServerTimeoutError` has `TimeoutError` in the MRO)

fixes aio-libs#8540
arcivanov added a commit to arcivanov/aiohttp that referenced this issue Jul 29, 2024
Make sure to unblock the `receive` operation by feeding the receiver an error in a `WSMessage`

Change `TimeoutError` to `ServerTimeoutError` to accurately represent failure (this is backwards compatible since `ServerTimeoutError` has `TimeoutError` in the MRO)

fixes aio-libs#8540
arcivanov added a commit to arcivanov/aiohttp that referenced this issue Jul 29, 2024
Make sure to unblock the `receive` operation by feeding the receiver an error in a `WSMessage`

Change `TimeoutError` to `ServerTimeoutError` to accurately represent failure (this is backwards compatible since `ServerTimeoutError` has `TimeoutError` in the MRO)

fixes aio-libs#8540
arcivanov added a commit to arcivanov/aiohttp that referenced this issue Jul 29, 2024
Make sure to unblock the `receive` operation by feeding the receiver an error in a `WSMessage`

Change `TimeoutError` to `ServerTimeoutError` to accurately represent failure (this is backwards compatible since `ServerTimeoutError` has `TimeoutError` in the MRO)

fixes aio-libs#8540
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant