Fix handling of sub-ms transaction timeouts (#940)

Transaction timeouts are specified in seconds as float.
However, the server expects it in milliseconds as int.
This would lead to
 1) rounding issues: previously, the driver would multiply by 1000 and
    then truncate to int. E.g., 256.4 seconds would be turned into 256399 ms
    because of float imprecision.
    Therefore, the built-in `round` is now used instead.
 2) values below 1 ms (e.g., 0.0001) would be rounded down to 0. However, 0 is
    a special value that instructs the server to not apply any timeout. This
    is likely to surprise the user which specified a non-zero timeout. In this
    special case, the driver now rounds up to 1 ms.

Author: robsdedude

src/neo4j/_async/io/_bolt.py

@@ -947,3 +947,32 @@ def is_idle_for(self, timeout):
 
 
 AsyncBoltSocket.Bolt = AsyncBolt  # type: ignore
+
+
+def tx_timeout_as_ms(timeout: float) -> int:
+    """Round transaction timeout to milliseconds.
+
+    Values in (0, 1], else values are rounded using the built-in round()
+    function (round n.5 values to nearest even).
+
+    :param timeout: timeout in seconds (must be >= 0)
+
+    :returns: timeout in milliseconds (rounded)
+
+    :raise ValueError: if timeout is negative
+    """
+    try:
+        timeout = float(timeout)
+    except (TypeError, ValueError) as e:
+        err_type = type(e)
+        msg = "Timeout must be specified as a number of seconds"
+        raise err_type(msg) from None
+    if timeout < 0:
+        raise ValueError("Timeout must be a positive number or 0.")
+    ms = int(round(1000 * timeout))
+    if ms == 0 and timeout > 0:
+        # Special case for 0 < timeout < 0.5 ms.
+        # This would be rounded to 0 ms, but the server interprets this as
+        # infinite timeout. So we round to the smallest possible timeout: 1 ms.
+        ms = 1
+    return ms

src/neo4j/_async/io/_bolt3.py

@@ -39,6 +39,7 @@
 from ._bolt import (
     AsyncBolt,
     ServerStateManagerBase,
+    tx_timeout_as_ms,
 )
 from ._common import (
     check_supported_server_product,
@@ -262,12 +263,7 @@ def run(self, query, parameters=None, mode=None, bookmarks=None,
             except TypeError:
                 raise TypeError("Metadata must be coercible to a dict")
         if timeout is not None:
-            try:
-                extra["tx_timeout"] = int(1000 * float(timeout))
-            except TypeError:
-                raise TypeError("Timeout must be specified as a number of seconds")
-            if extra["tx_timeout"] < 0:
-                raise ValueError("Timeout must be a positive number or 0.")
+            extra["tx_timeout"] = tx_timeout_as_ms(timeout)
         fields = (query, parameters, extra)
         log.debug("[#%04X]  C: RUN %s", self.local_port, " ".join(map(repr, fields)))
         self._append(b"\x10", fields,
@@ -327,12 +323,7 @@ def begin(self, mode=None, bookmarks=None, metadata=None, timeout=None,
             except TypeError:
                 raise TypeError("Metadata must be coercible to a dict")
         if timeout is not None:
-            try:
-                extra["tx_timeout"] = int(1000 * float(timeout))
-            except TypeError:
-                raise TypeError("Timeout must be specified as a number of seconds")
-            if extra["tx_timeout"] < 0:
-                raise ValueError("Timeout must be a positive number or 0.")
+            extra["tx_timeout"] = tx_timeout_as_ms(timeout)
         log.debug("[#%04X]  C: BEGIN %r", self.local_port, extra)
         self._append(b"\x11", (extra,),
                      Response(self, "begin", hydration_hooks, **handlers),

src/neo4j/_async/io/_bolt4.py

@@ -36,6 +36,7 @@
 from ._bolt import (
     AsyncBolt,
     ServerStateManagerBase,
+    tx_timeout_as_ms,
 )
 from ._bolt3 import (
     ServerStateManager,
@@ -212,12 +213,7 @@ def run(self, query, parameters=None, mode=None, bookmarks=None,
             except TypeError:
                 raise TypeError("Metadata must be coercible to a dict")
         if timeout is not None:
-            try:
-                extra["tx_timeout"] = int(1000 * float(timeout))
-            except TypeError:
-                raise TypeError("Timeout must be specified as a number of seconds")
-            if extra["tx_timeout"] < 0:
-                raise ValueError("Timeout must be a positive number or 0.")
+            extra["tx_timeout"] = tx_timeout_as_ms(timeout)
         fields = (query, parameters, extra)
         log.debug("[#%04X]  C: RUN %s", self.local_port, " ".join(map(repr, fields)))
         self._append(b"\x10", fields,
@@ -276,12 +272,7 @@ def begin(self, mode=None, bookmarks=None, metadata=None, timeout=None,
             except TypeError:
                 raise TypeError("Metadata must be coercible to a dict")
         if timeout is not None:
-            try:
-                extra["tx_timeout"] = int(1000 * float(timeout))
-            except TypeError:
-                raise TypeError("Timeout must be specified as a number of seconds")
-            if extra["tx_timeout"] < 0:
-                raise ValueError("Timeout must be a positive number or 0.")
+            extra["tx_timeout"] = tx_timeout_as_ms(timeout)
         log.debug("[#%04X]  C: BEGIN %r", self.local_port, extra)
         self._append(b"\x11", (extra,),
                      Response(self, "begin", hydration_hooks, **handlers),
@@ -555,12 +546,7 @@ def run(self, query, parameters=None, mode=None, bookmarks=None,
             except TypeError:
                 raise TypeError("Metadata must be coercible to a dict")
         if timeout is not None:
-            try:
-                extra["tx_timeout"] = int(1000 * float(timeout))
-            except TypeError:
-                raise TypeError("Timeout must be specified as a number of seconds")
-            if extra["tx_timeout"] < 0:
-                raise ValueError("Timeout must be a positive number or 0.")
+            extra["tx_timeout"] = tx_timeout_as_ms(timeout)
         fields = (query, parameters, extra)
         log.debug("[#%04X]  C: RUN %s", self.local_port,
                   " ".join(map(repr, fields)))
@@ -596,12 +582,7 @@ def begin(self, mode=None, bookmarks=None, metadata=None, timeout=None,
             except TypeError:
                 raise TypeError("Metadata must be coercible to a dict")
         if timeout is not None:
-            try:
-                extra["tx_timeout"] = int(1000 * float(timeout))
-            except TypeError:
-                raise TypeError("Timeout must be specified as a number of seconds")
-            if extra["tx_timeout"] < 0:
-                raise ValueError("Timeout must be a positive number or 0.")
+            extra["tx_timeout"] = tx_timeout_as_ms(timeout)
         log.debug("[#%04X]  C: BEGIN %r", self.local_port, extra)
         self._append(b"\x11", (extra,),
                      Response(self, "begin", hydration_hooks, **handlers),

src/neo4j/_async/io/_bolt5.py

@@ -38,6 +38,7 @@
 from ._bolt import (
     AsyncBolt,
     ServerStateManagerBase,
+    tx_timeout_as_ms,
 )
 from ._bolt3 import (
     ServerStateManager,
@@ -209,12 +210,7 @@ def run(self, query, parameters=None, mode=None, bookmarks=None,
             except TypeError:
                 raise TypeError("Metadata must be coercible to a dict")
         if timeout is not None:
-            try:
-                extra["tx_timeout"] = int(1000 * float(timeout))
-            except TypeError:
-                raise TypeError("Timeout must be a number (in seconds)")
-            if extra["tx_timeout"] < 0:
-                raise ValueError("Timeout must be a number >= 0")
+            extra["tx_timeout"] = tx_timeout_as_ms(timeout)
         fields = (query, parameters, extra)
         log.debug("[#%04X]  C: RUN %s", self.local_port,
                   " ".join(map(repr, fields)))
@@ -270,12 +266,7 @@ def begin(self, mode=None, bookmarks=None, metadata=None, timeout=None,
             except TypeError:
                 raise TypeError("Metadata must be coercible to a dict")
         if timeout is not None:
-            try:
-                extra["tx_timeout"] = int(1000 * float(timeout))
-            except TypeError:
-                raise TypeError("Timeout must be a number (in seconds)")
-            if extra["tx_timeout"] < 0:
-                raise ValueError("Timeout must be a number >= 0")
+            extra["tx_timeout"] = tx_timeout_as_ms(timeout)
         log.debug("[#%04X]  C: BEGIN %r", self.local_port, extra)
         self._append(b"\x11", (extra,),
                      Response(self, "begin", hydration_hooks, **handlers),
@@ -567,12 +558,7 @@ def run(self, query, parameters=None, mode=None, bookmarks=None,
             except TypeError:
                 raise TypeError("Metadata must be coercible to a dict")
         if timeout is not None:
-            try:
-                extra["tx_timeout"] = int(1000 * float(timeout))
-            except TypeError:
-                raise TypeError("Timeout must be a number (in seconds)")
-            if extra["tx_timeout"] < 0:
-                raise ValueError("Timeout must be a number >= 0")
+            extra["tx_timeout"] = tx_timeout_as_ms(timeout)
         fields = (query, parameters, extra)
         log.debug("[#%04X]  C: RUN %s", self.local_port,
                   " ".join(map(repr, fields)))
@@ -603,12 +589,7 @@ def begin(self, mode=None, bookmarks=None, metadata=None, timeout=None,
             except TypeError:
                 raise TypeError("Metadata must be coercible to a dict")
         if timeout is not None:
-            try:
-                extra["tx_timeout"] = int(1000 * float(timeout))
-            except TypeError:
-                raise TypeError("Timeout must be a number (in seconds)")
-            if extra["tx_timeout"] < 0:
-                raise ValueError("Timeout must be a number >= 0")
+            extra["tx_timeout"] = tx_timeout_as_ms(timeout)
         if notifications_min_severity is not None:
             extra["notifications_minimum_severity"] = \
                 notifications_min_severity

src/neo4j/_sync/io/_bolt.py

@@ -947,3 +947,32 @@ def is_idle_for(self, timeout):
 
 
 BoltSocket.Bolt = Bolt  # type: ignore
+
+
+def tx_timeout_as_ms(timeout: float) -> int:
+    """Round transaction timeout to milliseconds.
+
+    Values in (0, 1], else values are rounded using the built-in round()
+    function (round n.5 values to nearest even).
+
+    :param timeout: timeout in seconds (must be >= 0)
+
+    :returns: timeout in milliseconds (rounded)
+
+    :raise ValueError: if timeout is negative
+    """
+    try:
+        timeout = float(timeout)
+    except (TypeError, ValueError) as e:
+        err_type = type(e)
+        msg = "Timeout must be specified as a number of seconds"
+        raise err_type(msg) from None
+    if timeout < 0:
+        raise ValueError("Timeout must be a positive number or 0.")
+    ms = int(round(1000 * timeout))
+    if ms == 0 and timeout > 0:
+        # Special case for 0 < timeout < 0.5 ms.
+        # This would be rounded to 0 ms, but the server interprets this as
+        # infinite timeout. So we round to the smallest possible timeout: 1 ms.
+        ms = 1
+    return ms

src/neo4j/_sync/io/_bolt3.py

@@ -39,6 +39,7 @@
 from ._bolt import (
     Bolt,
     ServerStateManagerBase,
+    tx_timeout_as_ms,
 )
 from ._common import (
     check_supported_server_product,
@@ -262,12 +263,7 @@ def run(self, query, parameters=None, mode=None, bookmarks=None,
             except TypeError:
                 raise TypeError("Metadata must be coercible to a dict")
         if timeout is not None:
-            try:
-                extra["tx_timeout"] = int(1000 * float(timeout))
-            except TypeError:
-                raise TypeError("Timeout must be specified as a number of seconds")
-            if extra["tx_timeout"] < 0:
-                raise ValueError("Timeout must be a positive number or 0.")
+            extra["tx_timeout"] = tx_timeout_as_ms(timeout)
         fields = (query, parameters, extra)
         log.debug("[#%04X]  C: RUN %s", self.local_port, " ".join(map(repr, fields)))
         self._append(b"\x10", fields,
@@ -327,12 +323,7 @@ def begin(self, mode=None, bookmarks=None, metadata=None, timeout=None,
             except TypeError:
                 raise TypeError("Metadata must be coercible to a dict")
         if timeout is not None:
-            try:
-                extra["tx_timeout"] = int(1000 * float(timeout))
-            except TypeError:
-                raise TypeError("Timeout must be specified as a number of seconds")
-            if extra["tx_timeout"] < 0:
-                raise ValueError("Timeout must be a positive number or 0.")
+            extra["tx_timeout"] = tx_timeout_as_ms(timeout)
         log.debug("[#%04X]  C: BEGIN %r", self.local_port, extra)
         self._append(b"\x11", (extra,),
                      Response(self, "begin", hydration_hooks, **handlers),

src/neo4j/_sync/io/_bolt4.py

@@ -36,6 +36,7 @@
 from ._bolt import (
     Bolt,
     ServerStateManagerBase,
+    tx_timeout_as_ms,
 )
 from ._bolt3 import (
     ServerStateManager,
@@ -212,12 +213,7 @@ def run(self, query, parameters=None, mode=None, bookmarks=None,
             except TypeError:
                 raise TypeError("Metadata must be coercible to a dict")
         if timeout is not None:
-            try:
-                extra["tx_timeout"] = int(1000 * float(timeout))
-            except TypeError:
-                raise TypeError("Timeout must be specified as a number of seconds")
-            if extra["tx_timeout"] < 0:
-                raise ValueError("Timeout must be a positive number or 0.")
+            extra["tx_timeout"] = tx_timeout_as_ms(timeout)
         fields = (query, parameters, extra)
         log.debug("[#%04X]  C: RUN %s", self.local_port, " ".join(map(repr, fields)))
         self._append(b"\x10", fields,
@@ -276,12 +272,7 @@ def begin(self, mode=None, bookmarks=None, metadata=None, timeout=None,
             except TypeError:
                 raise TypeError("Metadata must be coercible to a dict")
         if timeout is not None:
-            try:
-                extra["tx_timeout"] = int(1000 * float(timeout))
-            except TypeError:
-                raise TypeError("Timeout must be specified as a number of seconds")
-            if extra["tx_timeout"] < 0:
-                raise ValueError("Timeout must be a positive number or 0.")
+            extra["tx_timeout"] = tx_timeout_as_ms(timeout)
         log.debug("[#%04X]  C: BEGIN %r", self.local_port, extra)
         self._append(b"\x11", (extra,),
                      Response(self, "begin", hydration_hooks, **handlers),
@@ -555,12 +546,7 @@ def run(self, query, parameters=None, mode=None, bookmarks=None,
             except TypeError:
                 raise TypeError("Metadata must be coercible to a dict")
         if timeout is not None:
-            try:
-                extra["tx_timeout"] = int(1000 * float(timeout))
-            except TypeError:
-                raise TypeError("Timeout must be specified as a number of seconds")
-            if extra["tx_timeout"] < 0:
-                raise ValueError("Timeout must be a positive number or 0.")
+            extra["tx_timeout"] = tx_timeout_as_ms(timeout)
         fields = (query, parameters, extra)
         log.debug("[#%04X]  C: RUN %s", self.local_port,
                   " ".join(map(repr, fields)))
@@ -596,12 +582,7 @@ def begin(self, mode=None, bookmarks=None, metadata=None, timeout=None,
             except TypeError:
                 raise TypeError("Metadata must be coercible to a dict")
         if timeout is not None:
-            try:
-                extra["tx_timeout"] = int(1000 * float(timeout))
-            except TypeError:
-                raise TypeError("Timeout must be specified as a number of seconds")
-            if extra["tx_timeout"] < 0:
-                raise ValueError("Timeout must be a positive number or 0.")
+            extra["tx_timeout"] = tx_timeout_as_ms(timeout)
         log.debug("[#%04X]  C: BEGIN %r", self.local_port, extra)
         self._append(b"\x11", (extra,),
                      Response(self, "begin", hydration_hooks, **handlers),

src/neo4j/_sync/io/_bolt5.py

@@ -38,6 +38,7 @@
 from ._bolt import (
     Bolt,
     ServerStateManagerBase,
+    tx_timeout_as_ms,
 )
 from ._bolt3 import (
     ServerStateManager,
@@ -209,12 +210,7 @@ def run(self, query, parameters=None, mode=None, bookmarks=None,
             except TypeError:
                 raise TypeError("Metadata must be coercible to a dict")
         if timeout is not None:
-            try:
-                extra["tx_timeout"] = int(1000 * float(timeout))
-            except TypeError:
-                raise TypeError("Timeout must be a number (in seconds)")
-            if extra["tx_timeout"] < 0:
-                raise ValueError("Timeout must be a number >= 0")
+            extra["tx_timeout"] = tx_timeout_as_ms(timeout)
         fields = (query, parameters, extra)
         log.debug("[#%04X]  C: RUN %s", self.local_port,
                   " ".join(map(repr, fields)))
@@ -270,12 +266,7 @@ def begin(self, mode=None, bookmarks=None, metadata=None, timeout=None,
             except TypeError:
                 raise TypeError("Metadata must be coercible to a dict")
         if timeout is not None:
-            try:
-                extra["tx_timeout"] = int(1000 * float(timeout))
-            except TypeError:
-                raise TypeError("Timeout must be a number (in seconds)")
-            if extra["tx_timeout"] < 0:
-                raise ValueError("Timeout must be a number >= 0")
+            extra["tx_timeout"] = tx_timeout_as_ms(timeout)
         log.debug("[#%04X]  C: BEGIN %r", self.local_port, extra)
         self._append(b"\x11", (extra,),
                      Response(self, "begin", hydration_hooks, **handlers),
@@ -567,12 +558,7 @@ def run(self, query, parameters=None, mode=None, bookmarks=None,
             except TypeError:
                 raise TypeError("Metadata must be coercible to a dict")
         if timeout is not None:
-            try:
-                extra["tx_timeout"] = int(1000 * float(timeout))
-            except TypeError:
-                raise TypeError("Timeout must be a number (in seconds)")
-            if extra["tx_timeout"] < 0:
-                raise ValueError("Timeout must be a number >= 0")
+            extra["tx_timeout"] = tx_timeout_as_ms(timeout)
         fields = (query, parameters, extra)
         log.debug("[#%04X]  C: RUN %s", self.local_port,
                   " ".join(map(repr, fields)))
@@ -603,12 +589,7 @@ def begin(self, mode=None, bookmarks=None, metadata=None, timeout=None,
             except TypeError:
                 raise TypeError("Metadata must be coercible to a dict")
         if timeout is not None:
-            try:
-                extra["tx_timeout"] = int(1000 * float(timeout))
-            except TypeError:
-                raise TypeError("Timeout must be a number (in seconds)")
-            if extra["tx_timeout"] < 0:
-                raise ValueError("Timeout must be a number >= 0")
+            extra["tx_timeout"] = tx_timeout_as_ms(timeout)
         if notifications_min_severity is not None:
             extra["notifications_minimum_severity"] = \
                 notifications_min_severity