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.

Sign up for GitHub

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

Jump to bottom

SSCursor: Fix connection closed check #991

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

SSCursor: Fix connection closed check #991

wants to merge 1 commit into from

Conversation

FichteFoll
Copy link

@FichteFoll FichteFoll commented Sep 9, 2024 •
edited
Loading

What do these changes do?

Since the connection closes itself on an asyncio.CancelledError, it cannot be used afterwards. SSCursor.close attempts to clear some unfinished state first before closing using Connection._finish_unbuffered_query, but that call fails if the connection has been closed already and SSCursor itself does not properly check that.

Are there changes in behavior for the user?

Cancellations now do not raise an unhandled exception anymore when using the connection and the SSCursor in a context manager.

Following is an example traceback where both the connection and the SSCursor are used in an AsyncExitStack and the task was cancelled:

        async with contextlib.AsyncExitStack() as exit_stack:
            conn: aiomysql.Connection = await exit_stack.enter_async_context(get_connection())
            # Use an unbuffered cursor so we don't load the entire table into memory
            cursor: aiomysql.Cursor = await exit_stack.enter_async_context(conn.cursor(SSCursor))
            ...
Traceback (most recent call last):
  File "code/export/__init__.py", line 177, in _sql_to_csv
    while row := await cursor.fetchone():
                 ^^^^^^^^^^^^^^^^^^^^^^^
  File "venv/lib/python3.12/site-packages/aiomysql/cursors.py", line 629, in fetchone
    row = await self._read_next()
          ^^^^^^^^^^^^^^^^^^^^^^^
  File "venv/lib/python3.12/site-packages/aiomysql/cursors.py", line 622, in _read_next
    row = await self._result._read_rowdata_packet_unbuffered()
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "venv/lib/python3.12/site-packages/aiomysql/connection.py", line 1239, in _read_rowdata_packet_unbuffered
    packet = await self.connection._read_packet()
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "venv/lib/python3.12/site-packages/aiomysql/connection.py", line 635, in _read_packet
    recv_data = await self._read_bytes(bytes_to_read)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "venv/lib/python3.12/site-packages/aiomysql/connection.py", line 657, in _read_bytes
    data = await self._reader.readexactly(num_bytes)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/asyncio/streams.py", line 752, in readexactly
    await self._wait_for_data('readexactly')
  File "/usr/lib/python3.12/asyncio/streams.py", line 545, in _wait_for_data
    await self._waiter
asyncio.exceptions.CancelledError

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "code/export/__init__.py", line 69, in _wrap_with_log_msg
    result = await coro
             ^^^^^^^^^^
  File "code/export/__init__.py", line 85, in export_provider
    await self._export_table(
  File "code/export/__init__.py", line 154, in _export_table
    await self._sql_to_csv(table, csv_path, columns, joins)
  File "code/export/__init__.py", line 167, in _sql_to_csv
    async with contextlib.AsyncExitStack() as exit_stack:
  File "/usr/lib/python3.12/contextlib.py", line 754, in __aexit__
    raise exc_details[1]
  File "/usr/lib/python3.12/contextlib.py", line 737, in __aexit__
    cb_suppress = await cb(*exc_details)
                  ^^^^^^^^^^^^^^^^^^^^^^
  File "venv/lib/python3.12/site-packages/aiomysql/utils.py", line 78, in __aexit__
    await self._obj.close()
  File "venv/lib/python3.12/site-packages/aiomysql/cursors.py", line 605, in close
    await self._result._finish_unbuffered_query()
  File "venv/lib/python3.12/site-packages/aiomysql/connection.py", line 1258, in _finish_unbuffered_query
    packet = await self.connection._read_packet()
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "venv/lib/python3.12/site-packages/aiomysql/connection.py", line 609, in _read_packet
    packet_header = await self._read_bytes(4)
                    ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "venv/lib/python3.12/site-packages/aiomysql/connection.py", line 657, in _read_bytes
    data = await self._reader.readexactly(num_bytes)
                 ^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'readexactly'

Related issue number

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes (not necessary since this is a bugfix)

@FichteFoll
SSCursor: Fix connection closed check
a085bc5 
Since the connection closes itself on an `asyncio.CancelledError`, it
cannot be used afterwards. `SSCursor.close` attempts to clear some
unfinished state first before closing using
`Connection._finish_unbuffered_query`, but that call fails if the
connection has been closed already and SSCursor itself does not
properly check that.
@FichteFoll
Copy link
Author

FichteFoll commented Sep 9, 2024 •
edited
Loading

Tests are passing but submitting coverage data failed.
[2024-09-09T15:08:31.614Z] ['error'] There was an error running the uploader: Error uploading to [https://codecov.io:](https://codecov.io/) Error: There was an error fetching the storage URL during POST: 429 - {'detail': ErrorDetail(string='Rate limit reached. Please upload with the Codecov repository upload token to resolve issue. Expected time to availability: 2649s.', code='throttled')}

@FichteFoll
Copy link
Author

FichteFoll commented Sep 24, 2024 •
edited
Loading

Here's another situation where the connection already closes itself on an error (incomplete read/connection loss) and then the SSCursor's __aexit__ causes the AttributeError to be thrown again:

Traceback (most recent call last):
  File "venv/lib/python3.12/site-packages/aiomysql/connection.py", line 657, in _read_bytes
    data = await self._reader.readexactly(num_bytes)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/asyncio/streams.py", line 750, in readexactly
    raise exceptions.IncompleteReadError(incomplete, n)
asyncio.exceptions.IncompleteReadError: 36 bytes read on a total of 46 expected bytes

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

Traceback (most recent call last):
  File "code/export/exporter.py", line 443, in _sql_to_csv
    async for row in cursor:
  File "venv/lib/python3.12/site-packages/aiomysql/cursors.py", line 506, in __anext__
    ret = await self.fetchone()
          ^^^^^^^^^^^^^^^^^^^^^
  File "venv/lib/python3.12/site-packages/aiomysql/cursors.py", line 629, in fetchone
    row = await self._read_next()
          ^^^^^^^^^^^^^^^^^^^^^^^
  File "venv/lib/python3.12/site-packages/aiomysql/cursors.py", line 622, in _read_next
    row = await self._result._read_rowdata_packet_unbuffered()
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "venv/lib/python3.12/site-packages/aiomysql/connection.py", line 1239, in _read_rowdata_packet_unbuffered
    packet = await self.connection._read_packet()
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "venv/lib/python3.12/site-packages/aiomysql/connection.py", line 635, in _read_packet
    recv_data = await self._read_bytes(bytes_to_read)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "venv/lib/python3.12/site-packages/aiomysql/connection.py", line 661, in _read_bytes
    raise OperationalError(CR.CR_SERVER_LOST, msg) from e
pymysql.err.OperationalError: (2013, 'Lost connection to MySQL server during query')

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  …
  File "code/export/exporter.py", line 433, in _sql_to_csv
    async with contextlib.AsyncExitStack() as exit_stack:
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/contextlib.py", line 754, in __aexit__
    raise exc_details[1]
  File "/usr/lib/python3.12/contextlib.py", line 737, in __aexit__
    cb_suppress = await cb(*exc_details)
                  ^^^^^^^^^^^^^^^^^^^^^^
  File "venv/lib/python3.12/site-packages/aiomysql/utils.py", line 78, in __aexit__
    await self._obj.close()
  File "venv/lib/python3.12/site-packages/aiomysql/cursors.py", line 605, in close
    await self._result._finish_unbuffered_query()
  File "venv/lib/python3.12/site-packages/aiomysql/connection.py", line 1258, in _finish_unbuffered_query
    packet = await self.connection._read_packet()
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "venv/lib/python3.12/site-packages/aiomysql/connection.py", line 609, in _read_packet
    packet_header = await self._read_bytes(4)
                    ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "venv/lib/python3.12/site-packages/aiomysql/connection.py", line 657, in _read_bytes
    data = await self._reader.readexactly(num_bytes)
                 ^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'readexactly'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@FichteFoll