Add .is_retriable() to Neo4jError and DriverError (#682)

robsdedude · web-flow · commit 3389c4ad0f51 · 2022-03-14T15:27:20.000+01:00
This should help users to implement custom retry policies together with explicit
transactions.
This PR also improves documentation around errors and changes the driver's
session code to also use the new flag for determining when to retry a tx.
This will make sure that it's a reliable feature + simplify the driver's code.
diff --git a/docs/source/api.rst b/docs/source/api.rst
@@ -417,7 +417,7 @@ Will result in:
 Sessions & Transactions
 ***********************
 All database activity is co-ordinated through two mechanisms:
-**sessions** (:class:`neo4j.AsyncSession`) and **transactions**
+**sessions** (:class:`neo4j.Session`) and **transactions**
 (:class:`neo4j.Transaction`, :class:`neo4j.ManagedTransaction`).
 
 A **session** is a logical container for any number of causally-related transactional units of work.
@@ -1263,18 +1263,7 @@ Neo4j Execution Errors
 
 
 .. autoclass:: neo4j.exceptions.Neo4jError
-
-    .. autoproperty:: message
-
-    .. autoproperty:: code
-
-    There are many Neo4j status codes, see `status code <https://neo4j.com/docs/status-codes/current/>`_.
-
-    .. autoproperty:: classification
-
-    .. autoproperty:: category
-
-    .. autoproperty:: title
+    :members: message, code, is_retriable
 
 
 .. autoclass:: neo4j.exceptions.ClientError
@@ -1332,7 +1321,7 @@ Connectivity Errors
 
 
 .. autoclass:: neo4j.exceptions.DriverError
-
+    :members: is_retriable
 
 .. autoclass:: neo4j.exceptions.TransactionError
     :show-inheritance:
diff --git a/neo4j/__init__.py b/neo4j/__init__.py
@@ -39,6 +39,7 @@
     "DEFAULT_DATABASE",
     "Driver",
     "ExperimentalWarning",
+    "get_user_agent",
     "GraphDatabase",
     "IPv4Address",
     "IPv6Address",
diff --git a/neo4j/_async/work/session.py b/neo4j/_async/work/session.py
@@ -30,12 +30,11 @@
 from ...data import DataHydrator
 from ...exceptions import (
     ClientError,
-    IncompleteCommit,
+    DriverError,
     Neo4jError,
     ServiceUnavailable,
     SessionExpired,
     TransactionError,
-    TransientError,
 )
 from ...meta import (
     deprecated,
@@ -328,8 +327,9 @@ async def _transaction_error_handler(self, _):
             self._transaction = None
             await self._disconnect()
 
-    async def _open_transaction(self, *, tx_cls, access_mode, metadata=None,
-                          timeout=None):
+    async def _open_transaction(
+        self, *, tx_cls, access_mode, metadata=None, timeout=None
+    ):
         await self._connect(access_mode=access_mode)
         self._transaction = tx_cls(
             self._connection, self._config.fetch_size,
@@ -393,7 +393,11 @@ async def _run_transaction(
         metadata = getattr(transaction_function, "metadata", None)
         timeout = getattr(transaction_function, "timeout", None)
 
-        retry_delay = retry_delay_generator(self._config.initial_retry_delay, self._config.retry_delay_multiplier, self._config.retry_delay_jitter_factor)
+        retry_delay = retry_delay_generator(
+            self._config.initial_retry_delay,
+            self._config.retry_delay_multiplier,
+            self._config.retry_delay_jitter_factor
+        )
 
         errors = []
 
@@ -414,24 +418,22 @@ async def _run_transaction(
                     raise
                 else:
                     await tx._commit()
-            except IncompleteCommit:
-                raise
-            except (ServiceUnavailable, SessionExpired) as error:
-                errors.append(error)
+            except (DriverError, Neo4jError) as error:
                 await self._disconnect()
-            except TransientError as transient_error:
-                if not transient_error.is_retriable():
+                if not error.is_retriable():
                     raise
-                errors.append(transient_error)
+                errors.append(error)
             else:
                 return result
             if t0 == -1:
-                t0 = perf_counter()  # The timer should be started after the first attempt
+                # The timer should be started after the first attempt
+                t0 = perf_counter()
             t1 = perf_counter()
             if t1 - t0 > self._config.max_transaction_retry_time:
                 break
             delay = next(retry_delay)
-            log.warning("Transaction failed and will be retried in {}s ({})".format(delay, "; ".join(errors[-1].args)))
+            log.warning("Transaction failed and will be retried in {}s ({})"
+                        "".format(delay, "; ".join(errors[-1].args)))
             await async_sleep(delay)
 
         if errors:
diff --git a/neo4j/_sync/work/session.py b/neo4j/_sync/work/session.py
@@ -30,12 +30,11 @@
 from ...data import DataHydrator
 from ...exceptions import (
     ClientError,
-    IncompleteCommit,
+    DriverError,
     Neo4jError,
     ServiceUnavailable,
     SessionExpired,
     TransactionError,
-    TransientError,
 )
 from ...meta import (
     deprecated,
@@ -328,8 +327,9 @@ def _transaction_error_handler(self, _):
             self._transaction = None
             self._disconnect()
 
-    def _open_transaction(self, *, tx_cls, access_mode, metadata=None,
-                          timeout=None):
+    def _open_transaction(
+        self, *, tx_cls, access_mode, metadata=None, timeout=None
+    ):
         self._connect(access_mode=access_mode)
         self._transaction = tx_cls(
             self._connection, self._config.fetch_size,
@@ -393,7 +393,11 @@ def _run_transaction(
         metadata = getattr(transaction_function, "metadata", None)
         timeout = getattr(transaction_function, "timeout", None)
 
-        retry_delay = retry_delay_generator(self._config.initial_retry_delay, self._config.retry_delay_multiplier, self._config.retry_delay_jitter_factor)
+        retry_delay = retry_delay_generator(
+            self._config.initial_retry_delay,
+            self._config.retry_delay_multiplier,
+            self._config.retry_delay_jitter_factor
+        )
 
         errors = []
 
@@ -414,24 +418,22 @@ def _run_transaction(
                     raise
                 else:
                     tx._commit()
-            except IncompleteCommit:
-                raise
-            except (ServiceUnavailable, SessionExpired) as error:
-                errors.append(error)
+            except (DriverError, Neo4jError) as error:
                 self._disconnect()
-            except TransientError as transient_error:
-                if not transient_error.is_retriable():
+                if not error.is_retriable():
                     raise
-                errors.append(transient_error)
+                errors.append(error)
             else:
                 return result
             if t0 == -1:
-                t0 = perf_counter()  # The timer should be started after the first attempt
+                # The timer should be started after the first attempt
+                t0 = perf_counter()
             t1 = perf_counter()
             if t1 - t0 > self._config.max_transaction_retry_time:
                 break
             delay = next(retry_delay)
-            log.warning("Transaction failed and will be retried in {}s ({})".format(delay, "; ".join(errors[-1].args)))
+            log.warning("Transaction failed and will be retried in {}s ({})"
+                        "".format(delay, "; ".join(errors[-1].args)))
             sleep(delay)
 
         if errors:
diff --git a/neo4j/exceptions.py b/neo4j/exceptions.py
@@ -75,11 +75,16 @@ class Neo4jError(Exception):
     """ Raised when the Cypher engine returns an error to the client.
     """
 
+    #: (str or None) The error message returned by the server.
     message = None
+    #: (str or None) The error code returned by the server.
+    #: There are many Neo4j status codes, see
+    #: `status codes <https://neo4j.com/docs/status-codes/current/>`_.
     code = None
     classification = None
     category = None
     title = None
+    #: (dict) Any additional information returned by the server.
     metadata = None
 
     @classmethod
@@ -126,6 +131,19 @@ def _extract_error_class(cls, classification, code):
         else:
             return cls
 
+    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
+        retry policy in conjunction with :ref:`explicit-transactions-ref`.
+
+        :return: :const:`True` if the error is retryable,
+            :const:`False` otherwise.
+        :rtype: bool
+        """
+        return False
+
     def invalidates_all_connections(self):
         return self.code == "Neo.ClientError.Security.AuthorizationExpired"
 
@@ -163,15 +181,13 @@ class TransientError(Neo4jError):
     """
 
     def is_retriable(self):
-        """These are really client errors but classification on the server is not entirely correct and they are classified as transient.
-
-        :return: True if it is a retriable TransientError, otherwise False.
-        :rtype: bool
-        """
-        return not (self.code in (
+        # 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",
-        ))
+        )
 
 
 class DatabaseUnavailable(TransientError):
@@ -220,6 +236,7 @@ class TokenExpired(AuthError):
     A new driver instance with a fresh authentication token needs to be created.
     """
 
+
 client_errors = {
 
     # ConstraintError
@@ -266,6 +283,18 @@ class TokenExpired(AuthError):
 class DriverError(Exception):
     """ Raised when the Driver raises an error.
     """
+    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
+        retry policy in conjunction with :ref:`explicit-transactions-ref`.
+
+        :return: :const:`True` if the error is retryable,
+            :const:`False` otherwise.
+        :rtype: bool
+        """
+        return False
 
 
 class SessionExpired(DriverError):
@@ -276,6 +305,9 @@ class SessionExpired(DriverError):
     def __init__(self, session, *args, **kwargs):
         super(SessionExpired, self).__init__(session, *args, **kwargs)
 
+    def is_retriable(self):
+        return True
+
 
 class TransactionError(DriverError):
     """ Raised when an error occurs while using a transaction.
@@ -315,6 +347,9 @@ class ServiceUnavailable(DriverError):
     """ Raised when no database service is available.
     """
 
+    def is_retriable(self):
+        return True
+
 
 class RoutingServiceUnavailable(ServiceUnavailable):
     """ Raised when no routing service is available.
@@ -340,6 +375,9 @@ class IncompleteCommit(ServiceUnavailable):
     successfully or not.
     """
 
+    def is_retriable(self):
+        return False
+
 
 class ConfigurationError(DriverError):
     """ Raised when there is an error concerning a configuration.
