Adjust Retryable Errors (#742)

 * Deprecate `is_retriable()` in favor of `is_retryable()`
 * Rewrite errors for backwards compatibility
   * Neo.TransientError.Transaction.Terminated
     -> Neo.ClientError.Transaction.Terminated
   * Neo.TransientError.Transaction.LockClientStopped
     -> Neo.ClientError.Transaction.LockClientStopped

Author: robsdedude

CHANGELOG.md

@@ -96,6 +96,8 @@
   - ANSI colour codes for log output are now opt-in
   - Prepend log format with log-level (if colours are disabled)
   - Prepend log format with thread name and id
+- Deprecated `neo4j.exceptions.Neo4jError.is_retriable()`.  
+  Use `neo4j.exceptions.Neo4jError.is_retryable()` instead.
 - Importing submodules from `neo4j.time` (`neo4j.time.xyz`) has been deprecated.
   Everything needed should be imported from `neo4j.time` directly.
 - `neo4j.spatial.hydrate_point` and `neo4j.spatial.dehydrate_point` have been

docs/source/api.rst

@@ -1246,7 +1246,7 @@ Neo4j Execution Errors
 
 
 .. autoclass:: neo4j.exceptions.Neo4jError
-    :members: message, code, is_retriable
+    :members: message, code, is_retriable, is_retryable
 
 
 .. autoclass:: neo4j.exceptions.ClientError
@@ -1304,7 +1304,7 @@ Connectivity Errors
 
 
 .. autoclass:: neo4j.exceptions.DriverError
-    :members: is_retriable
+    :members: is_retryable
 
 .. autoclass:: neo4j.exceptions.TransactionError
     :show-inheritance:

neo4j/_async/work/session.py

@@ -417,7 +417,7 @@ async def _run_transaction(
                     await tx._commit()
             except (DriverError, Neo4jError) as error:
                 await self._disconnect()
-                if not error.is_retriable():
+                if not error.is_retryable():
                     raise
                 errors.append(error)
             else:

neo4j/_async/work/transaction.py

@@ -245,7 +245,7 @@ class AsyncManagedTransaction(_AsyncTransactionBase):
     Note that transaction functions have to be idempotent (i.e., the result
     of running the function once has to be the same as running it any number
     of times). This is, because the driver will retry the transaction function
-    if the error is classified as retriable.
+    if the error is classified as retryable.
 
     .. versionadded:: 5.0
 

neo4j/_sync/work/session.py

@@ -417,7 +417,7 @@ def _run_transaction(
                     tx._commit()
             except (DriverError, Neo4jError) as error:
                 self._disconnect()
-                if not error.is_retriable():
+                if not error.is_retryable():
                     raise
                 errors.append(error)
             else:

neo4j/_sync/work/transaction.py

@@ -245,7 +245,7 @@ class ManagedTransaction(_TransactionBase):
     Note that transaction functions have to be idempotent (i.e., the result
     of running the function once has to be the same as running it any number
     of times). This is, because the driver will retry the transaction function
-    if the error is classified as retriable.
+    if the error is classified as retryable.
 
     .. versionadded:: 5.0
 

neo4j/exceptions.py

@@ -63,14 +63,38 @@
   + BoltFailure
   + BoltProtocolError
   + Bolt*
-
 """
 
+
+from .meta import deprecated
+
+
 CLASSIFICATION_CLIENT = "ClientError"
 CLASSIFICATION_TRANSIENT = "TransientError"
 CLASSIFICATION_DATABASE = "DatabaseError"
 
 
+ERROR_REWRITE_MAP = {
+    # This error can be retried ed. The driver just needs to re-authenticate
+    # with the same credentials.
+    "Neo.ClientError.Security.AuthorizationExpired": (
+        CLASSIFICATION_TRANSIENT, None
+    ),
+    # In 5.0, this error has been re-classified as ClientError.
+    # For backwards compatibility with Neo4j 4.4 and earlier, we re-map it in
+    # the driver, too.
+    "Neo.TransientError.Transaction.Terminated": (
+        CLASSIFICATION_CLIENT, "Neo.ClientError.Transaction.Terminated"
+    ),
+    # In 5.0, this error has been re-classified as ClientError.
+    # For backwards compatibility with Neo4j 4.4 and earlier, we re-map it in
+    # the driver, too.
+    "Neo.TransientError.Transaction.LockClientStopped": (
+        CLASSIFICATION_CLIENT, "Neo.ClientError.Transaction.LockClientStopped"
+    ),
+}
+
+
 class Neo4jError(Exception):
     """ Raised when the Cypher engine returns an error to the client.
     """
@@ -93,12 +117,17 @@ def hydrate(cls, message=None, code=None, **metadata):
         code = code or "Neo.DatabaseError.General.UnknownError"
         try:
             _, classification, category, title = code.split(".")
-            if code == "Neo.ClientError.Security.AuthorizationExpired":
-                classification = CLASSIFICATION_TRANSIENT
         except ValueError:
             classification = CLASSIFICATION_DATABASE
             category = "General"
             title = "UnknownError"
+        else:
+            classification_override, code_override = \
+                ERROR_REWRITE_MAP.get(code, (None, None))
+            if classification_override is not None:
+                classification = classification_override
+            if code_override is not None:
+                code = code_override
 
         error_class = cls._extract_error_class(classification, code)
 
@@ -131,11 +160,31 @@ def _extract_error_class(cls, classification, code):
         else:
             return cls
 
+    # TODO 6.0: Remove this alias
+    @deprecated(
+        "Neo4jError.is_retriable is deprecated and will be removed in a "
+        "future version. Please use Neo4jError.is_retryable instead."
+    )
     def is_retriable(self):
         """Whether the error is retryable.
 
-        Indicated whether a transaction that yielded this error makes sense to
-        retry. This methods  makes mostly sense when implementing a custom
+        See :meth:`.is_retryable`.
+
+        :return: :const:`True` if the error is retryable,
+            :const:`False` otherwise.
+        :rtype: bool
+
+        .. deprecated:: 5.0
+            This method will be removed in a future version.
+            Please use :meth:`.is_retryable` instead.
+        """
+        return self.is_retryable()
+
+    def is_retryable(self):
+        """Whether the error is retryable.
+
+        Indicates whether a transaction that yielded this error makes sense to
+        retry. This method makes mostly sense when implementing a custom
         retry policy in conjunction with :ref:`explicit-transactions-ref`.
 
         :return: :const:`True` if the error is retryable,
@@ -182,14 +231,8 @@ class TransientError(Neo4jError):
     """ The database cannot service the request right now, retrying later might yield a successful outcome.
     """
 
-    def is_retriable(self):
-        # Transient errors are always retriable.
-        # However, there are some errors that are misclassified by the server.
-        # They should really be ClientErrors.
-        return self.code not in (
-            "Neo.TransientError.Transaction.Terminated",
-            "Neo.TransientError.Transaction.LockClientStopped",
-        )
+    def is_retryable(self):
+        return True
 
 
 class DatabaseUnavailable(TransientError):
@@ -285,11 +328,11 @@ class TokenExpired(AuthError):
 class DriverError(Exception):
     """ Raised when the Driver raises an error.
     """
-    def is_retriable(self):
+    def is_retryable(self):
         """Whether the error is retryable.
 
-        Indicated whether a transaction that yielded this error makes sense to
-        retry. This methods  makes mostly sense when implementing a custom
+        Indicates whether a transaction that yielded this error makes sense to
+        retry. This method makes mostly sense when implementing a custom
         retry policy in conjunction with :ref:`explicit-transactions-ref`.
 
         :return: :const:`True` if the error is retryable,
@@ -307,7 +350,7 @@ class SessionExpired(DriverError):
     def __init__(self, session, *args, **kwargs):
         super(SessionExpired, self).__init__(session, *args, **kwargs)
 
-    def is_retriable(self):
+    def is_retryable(self):
         return True
 
 
@@ -349,7 +392,7 @@ class ServiceUnavailable(DriverError):
     """ Raised when no database service is available.
     """
 
-    def is_retriable(self):
+    def is_retryable(self):
         return True
 
 
@@ -377,7 +420,7 @@ class IncompleteCommit(ServiceUnavailable):
     successfully or not.
     """
 
-    def is_retriable(self):
+    def is_retryable(self):
         return False
 
 

tests/unit/common/test_exceptions.py

@@ -211,22 +211,49 @@ def test_neo4jerror_hydrate_with_message_and_code_client():
     assert error.code == "Neo.{}.General.TestError".format(CLASSIFICATION_CLIENT)
 
 
-def test_transient_error_is_retriable_case_1():
-    error = Neo4jError.hydrate(message="Test error message", code="Neo.TransientError.Transaction.Terminated")
-
-    assert isinstance(error, TransientError)
-    assert error.is_retriable() is False
-
-
-def test_transient_error_is_retriable_case_2():
-    error = Neo4jError.hydrate(message="Test error message", code="Neo.TransientError.Transaction.LockClientStopped")
-
-    assert isinstance(error, TransientError)
-    assert error.is_retriable() is False
-
-
-def test_transient_error_is_retriable_case_3():
-    error = Neo4jError.hydrate(message="Test error message", code="Neo.TransientError.General.TestError")
-
-    assert isinstance(error, TransientError)
-    assert error.is_retriable() is True
+@pytest.mark.parametrize(
+    ("code", "expected_cls", "expected_code"),
+    (
+        (
+            "Neo.TransientError.Transaction.Terminated",
+            ClientError,
+            "Neo.ClientError.Transaction.Terminated"
+        ),
+        (
+            "Neo.ClientError.Transaction.Terminated",
+            ClientError,
+            "Neo.ClientError.Transaction.Terminated"
+        ),
+        (
+            "Neo.TransientError.Transaction.LockClientStopped",
+            ClientError,
+            "Neo.ClientError.Transaction.LockClientStopped"
+        ),
+        (
+            "Neo.ClientError.Transaction.LockClientStopped",
+            ClientError,
+            "Neo.ClientError.Transaction.LockClientStopped"
+        ),
+        (
+            "Neo.ClientError.Security.AuthorizationExpired",
+            TransientError,
+            "Neo.ClientError.Security.AuthorizationExpired"
+        ),
+        (
+            "Neo.TransientError.General.TestError",
+            TransientError,
+            "Neo.TransientError.General.TestError"
+        )
+    )
+)
+def test_error_rewrite(code, expected_cls, expected_code):
+    message = "Test error message"
+    error = Neo4jError.hydrate(message=message, code=code)
+
+    expected_retryable = expected_cls is TransientError
+    assert error.__class__ is expected_cls
+    assert error.code == expected_code
+    assert error.message == message
+    assert error.is_retryable() is expected_retryable
+    with pytest.warns(DeprecationWarning, match=".*is_retryable.*"):
+        assert error.is_retriable() is expected_retryable