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

Sqlalchemy can't catch asynch's error #319

Open
Simon-Chenzw opened this issue Jul 4, 2024 · 0 comments · May be fixed by #320
Open

Sqlalchemy can't catch asynch's error #319

Simon-Chenzw opened this issue Jul 4, 2024 · 0 comments · May be fixed by #320

Comments

@Simon-Chenzw
Copy link
Contributor

Describe the bug
I'm trying to catch the asynch exception which raised in pre_ping

According to sqlalchemy's Doc
This should be working, but it isn't.

        def handle_error(e: sa.engine.ExceptionContext):
            if isinstance(e.original_exception, asynch.errors.UnexpectedPacketFromServerError):
                raise sqlalchemy.exc.InvalidatePoolError(
                    "`asynch.errors.UnexpectedPacketFromServerError`, invalidate pool",
                )
        sqlalchemy.event.listen(self.engine.sync_engine, "handle_error", handle_error)

This event should be caught in sqlalchemy here

        try:
            return self.do_ping(dbapi_connection)
        except self.loaded_dbapi.Error as err:
            ...

PEP 249 said

Error
Exception that is the base class of all other error exceptions.
You can use this to catch all errors with one single except statement.
It must be a subclass of the Python Exception class.

But in clickhouse_sqlalchemy/drivers/asynch/connector.py#L117

    class Error(Exception):
        pass

This error does not comply with PEP 249. It's should be asynch.errors.UnexpectedPacketFromServerError

To Reproduce
Minimal piece of Python code that reproduces the problem.

import asyncio
import unittest.mock

import asynch.errors
import clickhouse_sqlalchemy.drivers.asynch.connector
import sqlalchemy as sa
import sqlalchemy.event
import sqlalchemy.exc
import sqlalchemy.ext.asyncio as saea
import logging

DSN = "clickhouse+asynch://localhost:9000"

# NOTE This is a hack to fix this issue:
# clickhouse_sqlalchemy.drivers.asynch.connector.AsyncAdapt_asynch_dbapi.Error = (
#     asynch.errors.ClickHouseException
# )  # type: ignore


def failed_when_select_one(self, operation, params=None, context=None):
    if operation == "SELECT 1":
        raise asynch.errors.UnexpectedPacketFromServerError("Unexpected packet")
    else:
        return self.await_(self._execute_async(operation, params, context))


@unittest.mock.patch.object(
    clickhouse_sqlalchemy.drivers.asynch.connector.AsyncAdapt_asynch_cursor,
    "execute",
    failed_when_select_one,
)
async def main():
    engine = saea.create_async_engine(DSN, echo=True,pool_pre_ping=True)

    @sqlalchemy.event.listens_for(engine.sync_engine, "handle_error")
    def handle_error(e: sa.engine.ExceptionContext):
        if isinstance(e.original_exception, asynch.errors.UnexpectedPacketFromServerError):
            raise sqlalchemy.exc.InvalidatePoolError(
                "`asynch.errors.UnexpectedPacketFromServerError`, invalidate pool",
            )

    for _ in range(3):
        async with engine.connect() as conn:
            await conn.execute(sa.text("SELECT 123"))


if __name__ == "__main__":
    logging.basicConfig(level=logging.INFO)
    logging.getLogger("sqlalchemy.engine").setLevel(logging.INFO)
    logging.getLogger("sqlalchemy.pool").setLevel(logging.DEBUG)

    asyncio.run(main())

Expected behavior
The exception should be caught and handled successfully.

Versions
asynch==0.2.4
clickhouse-sqlalchemy==0.3.2
SQLAlchemy==2.0.31

@Simon-Chenzw Simon-Chenzw linked a pull request Jul 4, 2024 that will close this issue
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant