From 6f8d11831e828128e7f14167e95ed92e2cc4f1e0 Mon Sep 17 00:00:00 2001 From: Rouven Bauer Date: Mon, 14 Mar 2022 13:48:58 +0100 Subject: [PATCH] Add `.is_retriable()` to Neo4jError and DriverError 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. --- docs/source/api.rst | 17 +++--------- neo4j/__init__.py | 1 + neo4j/_async/work/session.py | 30 +++++++++++---------- neo4j/_sync/work/session.py | 30 +++++++++++---------- neo4j/exceptions.py | 52 +++++++++++++++++++++++++++++++----- 5 files changed, 81 insertions(+), 49 deletions(-) diff --git a/docs/source/api.rst b/docs/source/api.rst index 018c76d3b..020a7e887 100644 --- 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 `_. - - .. 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 index f683775fc..6d5171a91 100644 --- 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 index 9cb54fed6..b77fea09a 100644 --- 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 index fd2f99cc4..9ae88ca36 100644 --- 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 index bafe97371..81e64d2ee 100644 --- 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 `_. 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.