Skip to content

Adjust Retryable Errors #742

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

Merged
merged 5 commits into from
Jul 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions docs/source/api.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -1304,7 +1304,7 @@ Connectivity Errors


.. autoclass:: neo4j.exceptions.DriverError
:members: is_retriable
:members: is_retryable

.. autoclass:: neo4j.exceptions.TransactionError
:show-inheritance:
Expand Down
2 changes: 1 addition & 1 deletion neo4j/_async/work/session.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion neo4j/_async/work/transaction.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion neo4j/_sync/work/session.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion neo4j/_sync/work/transaction.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
81 changes: 62 additions & 19 deletions neo4j/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
"""
Expand All @@ -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)

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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,
Expand All @@ -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


Expand Down Expand Up @@ -349,7 +392,7 @@ class ServiceUnavailable(DriverError):
""" Raised when no database service is available.
"""

def is_retriable(self):
def is_retryable(self):
return True


Expand Down Expand Up @@ -377,7 +420,7 @@ class IncompleteCommit(ServiceUnavailable):
successfully or not.
"""

def is_retriable(self):
def is_retryable(self):
return False


Expand Down
65 changes: 46 additions & 19 deletions tests/unit/common/test_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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