Refactor error package (#227)

Due to an oversight, the logic cannot be used as intended. Since this is
a breaking change, we must release a new major

---------

Co-authored-by: Christoph Heer <[email protected]>

CHANGES.rst

@@ -1,6 +1,19 @@
 Changelog
 =========
 
+2.0.0
+-----
+
+Breaking Changes
+~~~~~~~~~~~~~~~~
+Reworked the ``sqlalchemy_hana.errors`` package so that it can be used inside a SQLAlchemy
+``handle_error`` event hook. Therefore
+
+- ``wrap_dbapi_error`` was removed
+- ``wrap_hdbcli_error`` was removed
+- ``HANAError`` now extends ``sqlalchemy.exc.DBAPIError``
+
+
 1.4.0
 -----
 

pyproject.toml

@@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"
 
 [project]
 name = "sqlalchemy-hana"
-version = "1.4.0"
+version = "2.0.0"
 description = "SQLAlchemy dialect for SAP HANA"
 keywords = ["sqlalchemy", "sap", "hana"]
 requires-python = "~=3.8"
@@ -43,6 +43,7 @@ dev = [
     "pylint==3.1.0",
     "mypy==1.9.0",
     "types-hdbcli==2.19.0.20240310",
+    "typing-extensions==4.10.0",
 ]
 test = [
     "pytest==8.1.1",
@@ -124,6 +125,7 @@ disable = [
     "too-many-function-args",
     "too-many-lines",
     "too-many-branches",
+    "too-many-return-statements",
 ]
 
 [tool.pylint.basic]

sqlalchemy_hana/errors.py

@@ -1,19 +1,36 @@
 """HANA error handling for humans.
 
 This module contains improved error handling for hdbcli errors.
-Basically it takes a py:exc:`hdbcli.dbapi.Error` or :py:exc:`sqlalchemy.exc.DBAPIError` instance
-and raises a more specific exception if possible.
+Basically it takes a :py:exc:`sqlalchemy.exc.DBAPIError` instance and returns a more specific
+exception if possible.
 """
 
 from __future__ import annotations
 
+from typing import TYPE_CHECKING, cast
+
 from hdbcli.dbapi import Error as HdbcliError
 from sqlalchemy.exc import DBAPIError
 
+if TYPE_CHECKING:
+    from typing_extensions import Self
+
 
-class HANAError(Exception):
+class HANAError(DBAPIError):
     """Base class for all sqlalchemy-hana errors."""
 
+    @classmethod
+    def from_dbapi_error(cls, error: DBAPIError) -> Self:
+        """Create a new exception instance based on the given dbapi error."""
+        return cls(
+            statement=error.statement,
+            params=error.params,
+            orig=cast(BaseException, error.orig),
+            hide_parameters=error.hide_parameters,
+            code=error.code,
+            ismulti=error.ismulti,
+        )
+
 
 class SequenceCacheTimeoutError(HANAError):
     """Exception raised when the sequence cache times out."""
@@ -63,55 +80,25 @@ class InvalidObjectNameError(HANAError):
     """Error when an invalid object name is referenced."""
 
 
-def wrap_dbapi_error(error: DBAPIError) -> None:
-    """Takes a :py:exc:`sqlalchemy.exc.DBAPIError` and raises a more specific exception if possible.
+def convert_dbapi_error(dbapi_error: DBAPIError) -> DBAPIError:
+    """Takes a :py:exc:`sqlalchemy.exc.DBAPIError` and returns a more specific error if possible.
 
     For that the :py:data:`sqlalchemy.exc.DBAPIError.orig` attribute is checked for a
     :py:exc:`hdbcli.dbapi.Error`.
-    If found, :py:func:`wrap_hdbcli_error` is called with it.
-    Else ``None`` is returned.
-
-    Args:
-        error: The error to be wrapped
-
-    Returns:
-        None
-    """
-    if isinstance(error.orig, HdbcliError):
-        wrap_hdbcli_error(error.orig)
+    If it does not contain a hdbcli error, the original exception is returned.
 
-
-def convert_dbapi_error(error: DBAPIError) -> DBAPIError | HANAError:
-    """Takes a :py:exc:`sqlalchemy.exc.DBAPIError` and converts it to a more specific exception.
-
-    Similar to :py:func:`~wrap_dbapi_error`, but instead of throwing the error, it returns it as
-    an object.
-    """
-    try:
-        wrap_dbapi_error(error)
-    except HANAError as thrown:
-        return thrown
-    return error
-
-
-def wrap_hdbcli_error(error: HdbcliError) -> None:
-    """Wraps the given :py:exc:`hdbcli.dbapi.Error` and raises specific exception if possible.
-
-    For this, the error code and error text are checked.
-    If a specific exception is raised, the original exception is set as the new exception's cause.
+    Else the error code and error text are further checked.
 
     In addition, an edge case is handled where SQLAlchemy creates a savepoint and the same
     transaction later fails leading to an automatic rollback by HANA.
     However, SQLAlchemy still tries to roll back the savepoint, which fails because the savepoint
     is no longer valid.
-    In this case, the cause of the exception is used for further processing.
-
-    Args:
-        error: The error to be wrapped
-
-    Returns:
-        None
+    In this case, the cause of the exception is used for further processing
     """
+    error = dbapi_error.orig
+    if not isinstance(error, HdbcliError):
+        return dbapi_error
+
     # extract hidden inner exceptions
     # TxSavepoint not found should normally only happen if a transaction was rolled back by HANA,
     # but SQLAlchemy also tries to perform a savepoint rollback, which fails due to the transaction
@@ -123,44 +110,45 @@ def wrap_hdbcli_error(error: HdbcliError) -> None:
         and "TxSavepoint not found" in error.errortext
     ):
         error = error.__context__
+        dbapi_error.orig = error
 
     if error.errorcode in [-10807, -10709]:  # sqldbc error codes for connection errors
-        raise ClientConnectionError from error
+        return ClientConnectionError.from_dbapi_error(dbapi_error)
     if error.errorcode == 613:
-        raise StatementTimeoutError from error
+        return StatementTimeoutError.from_dbapi_error(dbapi_error)
     if (
         error.errorcode == 139
         and "current operation cancelled by request and transaction rolled back"
         in error.errortext
     ):
-        raise TransactionCancelledError from error
+        return TransactionCancelledError.from_dbapi_error(dbapi_error)
     if "Lock timeout occurs while waiting sequence cache lock" in str(error.errortext):
-        raise SequenceCacheTimeoutError from error
+        return SequenceCacheTimeoutError.from_dbapi_error(dbapi_error)
     if error.errorcode == 131:
-        raise LockWaitTimeoutError from error
+        return LockWaitTimeoutError.from_dbapi_error(dbapi_error)
     if error.errorcode == 146:
-        raise LockAcquisitionError from error
+        return LockAcquisitionError.from_dbapi_error(dbapi_error)
     if error.errorcode == 133:
-        raise DeadlockError from error
+        return DeadlockError.from_dbapi_error(dbapi_error)
     if (
         "OutOfMemory exception" in error.errortext
         or "cannot allocate enough memory" in error.errortext
         or "Allocation failed" in error.errortext
         or error.errorcode == 4
     ):
-        raise DatabaseOutOfMemoryError from error
+        return DatabaseOutOfMemoryError.from_dbapi_error(dbapi_error)
     if (
         error.errorcode == 129
         and "max number of SqlExecutor threads are exceeded" in error.errortext
     ):
-        raise DatabaseOverloadedError from error
+        return DatabaseOverloadedError.from_dbapi_error(dbapi_error)
     if (
         # ERR_SQL_CONNECT_NOT_ALLOWED: user not allowed to connect from client
         error.errorcode == 663
         # GBA503: geo blocking service responded with a 503
         and "Error GBA503: Service is unavailable" in error.errortext
     ):
-        raise DatabaseConnectNotPossibleError from error
+        return DatabaseConnectNotPossibleError.from_dbapi_error(dbapi_error)
     if (
         # 129 -> ERR_TX_ROLLBACK: transaction rolled back by an internal error
         error.errorcode in [129, 145]
@@ -169,14 +157,13 @@ def wrap_hdbcli_error(error: HdbcliError) -> None:
         or "DTX commit(first phase commit) failed" in error.errortext
         or "An error occurred while reading from the channel" in error.errortext
     ):
-        raise StatementExecutionError from error
+        return StatementExecutionError.from_dbapi_error(dbapi_error)
     if error.errorcode == 397:
-        raise InvalidObjectNameError from error
+        return InvalidObjectNameError.from_dbapi_error(dbapi_error)
+    return dbapi_error
 
 
 __all__ = (
-    "wrap_dbapi_error",
-    "wrap_hdbcli_error",
     "HANAError",
     "SequenceCacheTimeoutError",
     "LockWaitTimeoutError",

test/test_errors.py

@@ -2,8 +2,6 @@
 
 from __future__ import annotations
 
-from unittest import mock
-
 import pytest
 from hdbcli.dbapi import Error as HdbcliError
 from sqlalchemy.exc import DBAPIError
@@ -23,37 +21,16 @@
     StatementTimeoutError,
     TransactionCancelledError,
     convert_dbapi_error,
-    wrap_dbapi_error,
-    wrap_hdbcli_error,
 )
 
 
-class TestWrapDbapiError(TestBase):
-    def test_calls_wrap_hdbcli_error(self) -> None:
-        hdbcli_error = HdbcliError()
-        error = DBAPIError("", None, orig=hdbcli_error)
-
-        with mock.patch("sqlalchemy_hana.errors.wrap_hdbcli_error") as mocked:
-            wrap_dbapi_error(error)
-
-        mocked.assert_called_once_with(hdbcli_error)
-
-    def test_calls_not_wrap_hdbcli_error(self) -> None:
-        error = DBAPIError("", None, orig=ValueError())
-
-        with mock.patch("sqlalchemy_hana.errors.wrap_hdbcli_error") as mocked:
-            wrap_dbapi_error(error)
-
-        mocked.assert_not_called()
-
-
-class TestWrapHdbcliError(TestBase):
-    def test_wrap_hdbcli_error_txsavepoint_not_found(self) -> None:
+class TestConvertDBAPIError(TestBase):
+    def test_convert_dbapi_error_txsavepoint_not_found(self) -> None:
         error = HdbcliError(128, "TxSavepoint not found")
         error.__context__ = HdbcliError(133, "some deadlock")
+        dbapi_error = DBAPIError(None, None, error)
 
-        with pytest.raises(DeadlockError):
-            wrap_hdbcli_error(error)
+        assert isinstance(convert_dbapi_error(dbapi_error), DeadlockError)
 
     @pytest.mark.parametrize(
         "errorcode,errortext,expected_exception",
@@ -101,16 +78,15 @@ def test_wrap_hdbcli_error_txsavepoint_not_found(self) -> None:
             (397, "", InvalidObjectNameError),
         ],
     )
-    def test_wrap_hdbcli_error(
+    def test_convert_dbapi_error(
         self,
         errorcode: int,
         errortext: str,
         expected_exception: type[Exception],
     ) -> None:
         error = HdbcliError(errorcode, errortext)
-
-        with pytest.raises(expected_exception):
-            wrap_hdbcli_error(error)
+        dbapi_error = DBAPIError(None, None, error)
+        assert isinstance(convert_dbapi_error(dbapi_error), expected_exception)
 
     @pytest.mark.parametrize(
         "errorcode,errortext",
@@ -120,29 +96,7 @@ def test_wrap_hdbcli_error(
             (123, "some error"),
         ],
     )
-    def test_wrap_hdbcli_error_no_wrap(self, errorcode: int, errortext: str) -> None:
+    def test_convert_dbapi_error_no_wrap(self, errorcode: int, errortext: str) -> None:
         error = HdbcliError(errorcode, errortext)
-        wrap_hdbcli_error(error)
-
-
-class TestConvertDbapiError(TestBase):
-    def test_calls_wrap_dbapi_error_throw(self) -> None:
-        error = DBAPIError("", None, orig=HdbcliError())
-
-        with mock.patch(
-            "sqlalchemy_hana.errors.wrap_dbapi_error", side_effect=StatementTimeoutError
-        ) as mocked:
-            new_error = convert_dbapi_error(error)
-            assert isinstance(new_error, StatementTimeoutError)
-
-        mocked.assert_called_once_with(error)
-
-    def test_calls_not_wrap_hdbcli_error_no_throw(self) -> None:
-        error = DBAPIError("", None, orig=ValueError())
-
-        with mock.patch(
-            "sqlalchemy_hana.errors.wrap_dbapi_error", return_value=None
-        ) as mocked:
-            assert convert_dbapi_error(error) == error
-
-        mocked.assert_called_once_with(error)
+        dbapi_error = DBAPIError(None, None, error)
+        assert convert_dbapi_error(dbapi_error) is dbapi_error