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

Suppress ConnectionResetError #7404

Merged
merged 1 commit into from
May 8, 2023
Merged

Conversation

drew2a
Copy link
Contributor

@drew2a drew2a commented May 3, 2023

This PR fixes #7345 by suppressing ConnectionResetError.

ConnectionResetError can be raised in three cases:

  1. there is no aiohttp connection
  2. aiohttp transport is None
  3. aiohttp transport is closing

https://github.com/aio-libs/aiohttp/blob/29b6f302ad6736f71ef3fb029d4c5bac786a2fb7/aiohttp/http_writer.py#L69-L76

    def _write(self, chunk: bytes) -> None:
        size = len(chunk)
        self.buffer_size += size
        self.output_size += size
        transport = self.transport
        if not self._protocol.connected or transport is None or transport.is_closing():
            raise ConnectionResetError("Cannot write to closing transport")
        transport.write(chunk)

In all cases described above, we cannot do anything from the core side. All reconnect logic is on the GUI side.

The most probable root cause of the error is the situation in which the GUI goes through the shutdown procedure and closes the connection with the core. For this particular case, ConnectionResetError should be ignored on the core side anyway.

There is another way to fix the issue (@kozlovsky suggestion).

To fix it, we need:

  • Cancel all requests to Core only after the response to the shutdown request is received, not earlier;
  • Suppress ConnectionResetError in Sentry if Core is in the shutdown state.

However, this fix requires much more work, increases the complexity of the GUI-core interactions, and involves Sentry for suppressing the specific error.

Since this is not a critical issue to address, it is better to fix it in the most straightforward and simple way.

@drew2a drew2a force-pushed the fix/7345 branch 2 times, most recently from 443748b to ff067a3 Compare May 4, 2023 09:30
@drew2a drew2a marked this pull request as ready for review May 4, 2023 09:37
@drew2a drew2a requested review from a team and kozlovsky and removed request for a team May 4, 2023 09:37
@drew2a drew2a merged commit 15d5e50 into Tribler:release/7.13 May 8, 2023
@drew2a drew2a deleted the fix/7345 branch May 8, 2023 09:51
@drew2a drew2a mentioned this pull request May 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants