changed the timer so it is initiated after the first try (#401)

* changed the timer so it is initiated after the first try

* added is_retriable method to TransientError

Author: martin-neotech

neo4j/exceptions.py

@@ -139,6 +139,17 @@ class TransientError(Neo4jError):
     """ The database cannot service the request right now, retrying later might yield a successful outcome.
     """
 
+    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 (
+            "Neo.TransientError.Transaction.Terminated",
+            "Neo.TransientError.Transaction.LockClientStopped",
+        ))
+
 
 class DatabaseUnavailable(TransientError):
     """

neo4j/work/simple.py

@@ -280,11 +280,12 @@ def _run_transaction(self, access_mode, unit_of_work, *args, **kwargs):
         metadata = getattr(unit_of_work, "metadata", None)
         timeout = getattr(unit_of_work, "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 = []
-        t0 = perf_counter()
+
+        t0 = -1  # Timer
+
         while True:
             try:
                 self._open_transaction(access_mode=access_mode, database=self._config.database, metadata=metadata, timeout=timeout)
@@ -299,20 +300,21 @@ def _run_transaction(self, access_mode, unit_of_work, *args, **kwargs):
             except (ServiceUnavailable, SessionExpired) as error:
                 errors.append(error)
                 self._disconnect()
-            except TransientError as error:
-                if is_retriable_transient_error(error):
-                    errors.append(error)
-                else:
+            except TransientError as transient_error:
+                if not transient_error.is_retriable():
                     raise
+                errors.append(transient_error)
             else:
                 return result
+            if t0 == -1:
+                t0 = perf_counter()  # The timer should be started after the first attempt
             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:
             raise errors[-1]
         else:
@@ -418,10 +420,3 @@ def retry_delay_generator(initial_delay, multiplier, jitter_factor):
         jitter = jitter_factor * delay
         yield delay - jitter + (2 * jitter * random())
         delay *= multiplier
-
-
-def is_retriable_transient_error(error):
-    """
-    :type error: TransientError
-    """
-    return not (error.code in ("Neo.TransientError.Transaction.Terminated", "Neo.TransientError.Transaction.LockClientStopped"))

tests/integration/test_tx_functions.py

@@ -22,9 +22,11 @@
 import pytest
 from uuid import uuid4
 
-
 from neo4j.work.simple import unit_of_work
-from neo4j.exceptions import ClientError
+from neo4j.exceptions import (
+    Neo4jError,
+    ClientError,
+)
 
 # python -m pytest tests/integration/test_tx_functions.py -s -v
 
@@ -100,3 +102,27 @@ def f(tx, uuid):
 
     with pytest.raises(TypeError):
         session.write_transaction(f)
+
+
+def test_retry_logic(driver):
+    # python -m pytest tests/integration/test_tx_functions.py -s -v -k test_retry_logic
+
+    pytest.global_counter = 0
+
+    def get_one(tx):
+        result = tx.run("UNWIND [1,2,3,4] AS x RETURN x")
+        records = list(result)
+        pytest.global_counter += 1
+
+        if pytest.global_counter < 3:
+            database_unavailable = Neo4jError.hydrate(message="The database is not currently available to serve your request, refer to the database logs for more details. Retrying your request at a later time may succeed.", code="Neo.TransientError.Database.DatabaseUnavailable")
+            raise database_unavailable
+
+        return records
+
+    with driver.session() as session:
+        records = session.read_transaction(get_one)
+
+        assert pytest.global_counter == 3
+
+    del pytest.global_counter

tests/unit/test_exceptions.py

@@ -217,3 +217,24 @@ def test_neo4jerror_hydrate_with_message_and_code_client():
     assert error.metadata == {}
     assert error.message == "Test error message"
     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