Skip to content

Commit 8d7b04a

Browse files
robsdeduderobsdedude
authored
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.
1 parent 03de905 commit 8d7b04a

28 files changed

+1324
-120
lines changed

‎src/neo4j/_async/io/_bolt.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -947,3 +947,32 @@ def is_idle_for(self, timeout):
947947

948948

949949
AsyncBoltSocket.Bolt = AsyncBolt # type: ignore
950+
951+
952+
def tx_timeout_as_ms(timeout: float) -> int:
953+
"""Round transaction timeout to milliseconds.
954+
955+
Values in (0, 1], else values are rounded using the built-in round()
956+
function (round n.5 values to nearest even).
957+
958+
:param timeout: timeout in seconds (must be >= 0)
959+
960+
:returns: timeout in milliseconds (rounded)
961+
962+
:raise ValueError: if timeout is negative
963+
"""
964+
try:
965+
timeout = float(timeout)
966+
except (TypeError, ValueError) as e:
967+
err_type = type(e)
968+
msg = "Timeout must be specified as a number of seconds"
969+
raise err_type(msg) from None
970+
if timeout < 0:
971+
raise ValueError("Timeout must be a positive number or 0.")
972+
ms = int(round(1000 * timeout))
973+
if ms == 0 and timeout > 0:
974+
# Special case for 0 < timeout < 0.5 ms.
975+
# This would be rounded to 0 ms, but the server interprets this as
976+
# infinite timeout. So we round to the smallest possible timeout: 1 ms.
977+
ms = 1
978+
return ms

‎src/neo4j/_async/io/_bolt3.py

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
from ._bolt import (
4040
AsyncBolt,
4141
ServerStateManagerBase,
42+
tx_timeout_as_ms,
4243
)
4344
from ._common import (
4445
check_supported_server_product,
@@ -262,12 +263,7 @@ def run(self, query, parameters=None, mode=None, bookmarks=None,
262263
except TypeError:
263264
raise TypeError("Metadata must be coercible to a dict")
264265
if timeout is not None:
265-
try:
266-
extra["tx_timeout"] = int(1000 * float(timeout))
267-
except TypeError:
268-
raise TypeError("Timeout must be specified as a number of seconds")
269-
if extra["tx_timeout"] < 0:
270-
raise ValueError("Timeout must be a positive number or 0.")
266+
extra["tx_timeout"] = tx_timeout_as_ms(timeout)
271267
fields = (query, parameters, extra)
272268
log.debug("[#%04X] C: RUN %s", self.local_port, " ".join(map(repr, fields)))
273269
self._append(b"\x10", fields,
@@ -327,12 +323,7 @@ def begin(self, mode=None, bookmarks=None, metadata=None, timeout=None,
327323
except TypeError:
328324
raise TypeError("Metadata must be coercible to a dict")
329325
if timeout is not None:
330-
try:
331-
extra["tx_timeout"] = int(1000 * float(timeout))
332-
except TypeError:
333-
raise TypeError("Timeout must be specified as a number of seconds")
334-
if extra["tx_timeout"] < 0:
335-
raise ValueError("Timeout must be a positive number or 0.")
326+
extra["tx_timeout"] = tx_timeout_as_ms(timeout)
336327
log.debug("[#%04X] C: BEGIN %r", self.local_port, extra)
337328
self._append(b"\x11", (extra,),
338329
Response(self, "begin", hydration_hooks, **handlers),

‎src/neo4j/_async/io/_bolt4.py

Lines changed: 5 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
from ._bolt import (
3737
AsyncBolt,
3838
ServerStateManagerBase,
39+
tx_timeout_as_ms,
3940
)
4041
from ._bolt3 import (
4142
ServerStateManager,
@@ -212,12 +213,7 @@ def run(self, query, parameters=None, mode=None, bookmarks=None,
212213
except TypeError:
213214
raise TypeError("Metadata must be coercible to a dict")
214215
if timeout is not None:
215-
try:
216-
extra["tx_timeout"] = int(1000 * float(timeout))
217-
except TypeError:
218-
raise TypeError("Timeout must be specified as a number of seconds")
219-
if extra["tx_timeout"] < 0:
220-
raise ValueError("Timeout must be a positive number or 0.")
216+
extra["tx_timeout"] = tx_timeout_as_ms(timeout)
221217
fields = (query, parameters, extra)
222218
log.debug("[#%04X] C: RUN %s", self.local_port, " ".join(map(repr, fields)))
223219
self._append(b"\x10", fields,
@@ -276,12 +272,7 @@ def begin(self, mode=None, bookmarks=None, metadata=None, timeout=None,
276272
except TypeError:
277273
raise TypeError("Metadata must be coercible to a dict")
278274
if timeout is not None:
279-
try:
280-
extra["tx_timeout"] = int(1000 * float(timeout))
281-
except TypeError:
282-
raise TypeError("Timeout must be specified as a number of seconds")
283-
if extra["tx_timeout"] < 0:
284-
raise ValueError("Timeout must be a positive number or 0.")
275+
extra["tx_timeout"] = tx_timeout_as_ms(timeout)
285276
log.debug("[#%04X] C: BEGIN %r", self.local_port, extra)
286277
self._append(b"\x11", (extra,),
287278
Response(self, "begin", hydration_hooks, **handlers),
@@ -555,12 +546,7 @@ def run(self, query, parameters=None, mode=None, bookmarks=None,
555546
except TypeError:
556547
raise TypeError("Metadata must be coercible to a dict")
557548
if timeout is not None:
558-
try:
559-
extra["tx_timeout"] = int(1000 * float(timeout))
560-
except TypeError:
561-
raise TypeError("Timeout must be specified as a number of seconds")
562-
if extra["tx_timeout"] < 0:
563-
raise ValueError("Timeout must be a positive number or 0.")
549+
extra["tx_timeout"] = tx_timeout_as_ms(timeout)
564550
fields = (query, parameters, extra)
565551
log.debug("[#%04X] C: RUN %s", self.local_port,
566552
" ".join(map(repr, fields)))
@@ -596,12 +582,7 @@ def begin(self, mode=None, bookmarks=None, metadata=None, timeout=None,
596582
except TypeError:
597583
raise TypeError("Metadata must be coercible to a dict")
598584
if timeout is not None:
599-
try:
600-
extra["tx_timeout"] = int(1000 * float(timeout))
601-
except TypeError:
602-
raise TypeError("Timeout must be specified as a number of seconds")
603-
if extra["tx_timeout"] < 0:
604-
raise ValueError("Timeout must be a positive number or 0.")
585+
extra["tx_timeout"] = tx_timeout_as_ms(timeout)
605586
log.debug("[#%04X] C: BEGIN %r", self.local_port, extra)
606587
self._append(b"\x11", (extra,),
607588
Response(self, "begin", hydration_hooks, **handlers),

‎src/neo4j/_async/io/_bolt5.py

Lines changed: 5 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
from ._bolt import (
3939
AsyncBolt,
4040
ServerStateManagerBase,
41+
tx_timeout_as_ms,
4142
)
4243
from ._bolt3 import (
4344
ServerStateManager,
@@ -209,12 +210,7 @@ def run(self, query, parameters=None, mode=None, bookmarks=None,
209210
except TypeError:
210211
raise TypeError("Metadata must be coercible to a dict")
211212
if timeout is not None:
212-
try:
213-
extra["tx_timeout"] = int(1000 * float(timeout))
214-
except TypeError:
215-
raise TypeError("Timeout must be a number (in seconds)")
216-
if extra["tx_timeout"] < 0:
217-
raise ValueError("Timeout must be a number >= 0")
213+
extra["tx_timeout"] = tx_timeout_as_ms(timeout)
218214
fields = (query, parameters, extra)
219215
log.debug("[#%04X] C: RUN %s", self.local_port,
220216
" ".join(map(repr, fields)))
@@ -270,12 +266,7 @@ def begin(self, mode=None, bookmarks=None, metadata=None, timeout=None,
270266
except TypeError:
271267
raise TypeError("Metadata must be coercible to a dict")
272268
if timeout is not None:
273-
try:
274-
extra["tx_timeout"] = int(1000 * float(timeout))
275-
except TypeError:
276-
raise TypeError("Timeout must be a number (in seconds)")
277-
if extra["tx_timeout"] < 0:
278-
raise ValueError("Timeout must be a number >= 0")
269+
extra["tx_timeout"] = tx_timeout_as_ms(timeout)
279270
log.debug("[#%04X] C: BEGIN %r", self.local_port, extra)
280271
self._append(b"\x11", (extra,),
281272
Response(self, "begin", hydration_hooks, **handlers),
@@ -567,12 +558,7 @@ def run(self, query, parameters=None, mode=None, bookmarks=None,
567558
except TypeError:
568559
raise TypeError("Metadata must be coercible to a dict")
569560
if timeout is not None:
570-
try:
571-
extra["tx_timeout"] = int(1000 * float(timeout))
572-
except TypeError:
573-
raise TypeError("Timeout must be a number (in seconds)")
574-
if extra["tx_timeout"] < 0:
575-
raise ValueError("Timeout must be a number >= 0")
561+
extra["tx_timeout"] = tx_timeout_as_ms(timeout)
576562
fields = (query, parameters, extra)
577563
log.debug("[#%04X] C: RUN %s", self.local_port,
578564
" ".join(map(repr, fields)))
@@ -603,12 +589,7 @@ def begin(self, mode=None, bookmarks=None, metadata=None, timeout=None,
603589
except TypeError:
604590
raise TypeError("Metadata must be coercible to a dict")
605591
if timeout is not None:
606-
try:
607-
extra["tx_timeout"] = int(1000 * float(timeout))
608-
except TypeError:
609-
raise TypeError("Timeout must be a number (in seconds)")
610-
if extra["tx_timeout"] < 0:
611-
raise ValueError("Timeout must be a number >= 0")
592+
extra["tx_timeout"] = tx_timeout_as_ms(timeout)
612593
if notifications_min_severity is not None:
613594
extra["notifications_minimum_severity"] = \
614595
notifications_min_severity

‎src/neo4j/_sync/io/_bolt.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -947,3 +947,32 @@ def is_idle_for(self, timeout):
947947

948948

949949
BoltSocket.Bolt = Bolt # type: ignore
950+
951+
952+
def tx_timeout_as_ms(timeout: float) -> int:
953+
"""Round transaction timeout to milliseconds.
954+
955+
Values in (0, 1], else values are rounded using the built-in round()
956+
function (round n.5 values to nearest even).
957+
958+
:param timeout: timeout in seconds (must be >= 0)
959+
960+
:returns: timeout in milliseconds (rounded)
961+
962+
:raise ValueError: if timeout is negative
963+
"""
964+
try:
965+
timeout = float(timeout)
966+
except (TypeError, ValueError) as e:
967+
err_type = type(e)
968+
msg = "Timeout must be specified as a number of seconds"
969+
raise err_type(msg) from None
970+
if timeout < 0:
971+
raise ValueError("Timeout must be a positive number or 0.")
972+
ms = int(round(1000 * timeout))
973+
if ms == 0 and timeout > 0:
974+
# Special case for 0 < timeout < 0.5 ms.
975+
# This would be rounded to 0 ms, but the server interprets this as
976+
# infinite timeout. So we round to the smallest possible timeout: 1 ms.
977+
ms = 1
978+
return ms

‎src/neo4j/_sync/io/_bolt3.py

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
from ._bolt import (
4040
Bolt,
4141
ServerStateManagerBase,
42+
tx_timeout_as_ms,
4243
)
4344
from ._common import (
4445
check_supported_server_product,
@@ -262,12 +263,7 @@ def run(self, query, parameters=None, mode=None, bookmarks=None,
262263
except TypeError:
263264
raise TypeError("Metadata must be coercible to a dict")
264265
if timeout is not None:
265-
try:
266-
extra["tx_timeout"] = int(1000 * float(timeout))
267-
except TypeError:
268-
raise TypeError("Timeout must be specified as a number of seconds")
269-
if extra["tx_timeout"] < 0:
270-
raise ValueError("Timeout must be a positive number or 0.")
266+
extra["tx_timeout"] = tx_timeout_as_ms(timeout)
271267
fields = (query, parameters, extra)
272268
log.debug("[#%04X] C: RUN %s", self.local_port, " ".join(map(repr, fields)))
273269
self._append(b"\x10", fields,
@@ -327,12 +323,7 @@ def begin(self, mode=None, bookmarks=None, metadata=None, timeout=None,
327323
except TypeError:
328324
raise TypeError("Metadata must be coercible to a dict")
329325
if timeout is not None:
330-
try:
331-
extra["tx_timeout"] = int(1000 * float(timeout))
332-
except TypeError:
333-
raise TypeError("Timeout must be specified as a number of seconds")
334-
if extra["tx_timeout"] < 0:
335-
raise ValueError("Timeout must be a positive number or 0.")
326+
extra["tx_timeout"] = tx_timeout_as_ms(timeout)
336327
log.debug("[#%04X] C: BEGIN %r", self.local_port, extra)
337328
self._append(b"\x11", (extra,),
338329
Response(self, "begin", hydration_hooks, **handlers),

‎src/neo4j/_sync/io/_bolt4.py

Lines changed: 5 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
from ._bolt import (
3737
Bolt,
3838
ServerStateManagerBase,
39+
tx_timeout_as_ms,
3940
)
4041
from ._bolt3 import (
4142
ServerStateManager,
@@ -212,12 +213,7 @@ def run(self, query, parameters=None, mode=None, bookmarks=None,
212213
except TypeError:
213214
raise TypeError("Metadata must be coercible to a dict")
214215
if timeout is not None:
215-
try:
216-
extra["tx_timeout"] = int(1000 * float(timeout))
217-
except TypeError:
218-
raise TypeError("Timeout must be specified as a number of seconds")
219-
if extra["tx_timeout"] < 0:
220-
raise ValueError("Timeout must be a positive number or 0.")
216+
extra["tx_timeout"] = tx_timeout_as_ms(timeout)
221217
fields = (query, parameters, extra)
222218
log.debug("[#%04X] C: RUN %s", self.local_port, " ".join(map(repr, fields)))
223219
self._append(b"\x10", fields,
@@ -276,12 +272,7 @@ def begin(self, mode=None, bookmarks=None, metadata=None, timeout=None,
276272
except TypeError:
277273
raise TypeError("Metadata must be coercible to a dict")
278274
if timeout is not None:
279-
try:
280-
extra["tx_timeout"] = int(1000 * float(timeout))
281-
except TypeError:
282-
raise TypeError("Timeout must be specified as a number of seconds")
283-
if extra["tx_timeout"] < 0:
284-
raise ValueError("Timeout must be a positive number or 0.")
275+
extra["tx_timeout"] = tx_timeout_as_ms(timeout)
285276
log.debug("[#%04X] C: BEGIN %r", self.local_port, extra)
286277
self._append(b"\x11", (extra,),
287278
Response(self, "begin", hydration_hooks, **handlers),
@@ -555,12 +546,7 @@ def run(self, query, parameters=None, mode=None, bookmarks=None,
555546
except TypeError:
556547
raise TypeError("Metadata must be coercible to a dict")
557548
if timeout is not None:
558-
try:
559-
extra["tx_timeout"] = int(1000 * float(timeout))
560-
except TypeError:
561-
raise TypeError("Timeout must be specified as a number of seconds")
562-
if extra["tx_timeout"] < 0:
563-
raise ValueError("Timeout must be a positive number or 0.")
549+
extra["tx_timeout"] = tx_timeout_as_ms(timeout)
564550
fields = (query, parameters, extra)
565551
log.debug("[#%04X] C: RUN %s", self.local_port,
566552
" ".join(map(repr, fields)))
@@ -596,12 +582,7 @@ def begin(self, mode=None, bookmarks=None, metadata=None, timeout=None,
596582
except TypeError:
597583
raise TypeError("Metadata must be coercible to a dict")
598584
if timeout is not None:
599-
try:
600-
extra["tx_timeout"] = int(1000 * float(timeout))
601-
except TypeError:
602-
raise TypeError("Timeout must be specified as a number of seconds")
603-
if extra["tx_timeout"] < 0:
604-
raise ValueError("Timeout must be a positive number or 0.")
585+
extra["tx_timeout"] = tx_timeout_as_ms(timeout)
605586
log.debug("[#%04X] C: BEGIN %r", self.local_port, extra)
606587
self._append(b"\x11", (extra,),
607588
Response(self, "begin", hydration_hooks, **handlers),

‎src/neo4j/_sync/io/_bolt5.py

Lines changed: 5 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
from ._bolt import (
3939
Bolt,
4040
ServerStateManagerBase,
41+
tx_timeout_as_ms,
4142
)
4243
from ._bolt3 import (
4344
ServerStateManager,
@@ -209,12 +210,7 @@ def run(self, query, parameters=None, mode=None, bookmarks=None,
209210
except TypeError:
210211
raise TypeError("Metadata must be coercible to a dict")
211212
if timeout is not None:
212-
try:
213-
extra["tx_timeout"] = int(1000 * float(timeout))
214-
except TypeError:
215-
raise TypeError("Timeout must be a number (in seconds)")
216-
if extra["tx_timeout"] < 0:
217-
raise ValueError("Timeout must be a number >= 0")
213+
extra["tx_timeout"] = tx_timeout_as_ms(timeout)
218214
fields = (query, parameters, extra)
219215
log.debug("[#%04X] C: RUN %s", self.local_port,
220216
" ".join(map(repr, fields)))
@@ -270,12 +266,7 @@ def begin(self, mode=None, bookmarks=None, metadata=None, timeout=None,
270266
except TypeError:
271267
raise TypeError("Metadata must be coercible to a dict")
272268
if timeout is not None:
273-
try:
274-
extra["tx_timeout"] = int(1000 * float(timeout))
275-
except TypeError:
276-
raise TypeError("Timeout must be a number (in seconds)")
277-
if extra["tx_timeout"] < 0:
278-
raise ValueError("Timeout must be a number >= 0")
269+
extra["tx_timeout"] = tx_timeout_as_ms(timeout)
279270
log.debug("[#%04X] C: BEGIN %r", self.local_port, extra)
280271
self._append(b"\x11", (extra,),
281272
Response(self, "begin", hydration_hooks, **handlers),
@@ -567,12 +558,7 @@ def run(self, query, parameters=None, mode=None, bookmarks=None,
567558
except TypeError:
568559
raise TypeError("Metadata must be coercible to a dict")
569560
if timeout is not None:
570-
try:
571-
extra["tx_timeout"] = int(1000 * float(timeout))
572-
except TypeError:
573-
raise TypeError("Timeout must be a number (in seconds)")
574-
if extra["tx_timeout"] < 0:
575-
raise ValueError("Timeout must be a number >= 0")
561+
extra["tx_timeout"] = tx_timeout_as_ms(timeout)
576562
fields = (query, parameters, extra)
577563
log.debug("[#%04X] C: RUN %s", self.local_port,
578564
" ".join(map(repr, fields)))
@@ -603,12 +589,7 @@ def begin(self, mode=None, bookmarks=None, metadata=None, timeout=None,
603589
except TypeError:
604590
raise TypeError("Metadata must be coercible to a dict")
605591
if timeout is not None:
606-
try:
607-
extra["tx_timeout"] = int(1000 * float(timeout))
608-
except TypeError:
609-
raise TypeError("Timeout must be a number (in seconds)")
610-
if extra["tx_timeout"] < 0:
611-
raise ValueError("Timeout must be a number >= 0")
592+
extra["tx_timeout"] = tx_timeout_as_ms(timeout)
612593
if notifications_min_severity is not None:
613594
extra["notifications_minimum_severity"] = \
614595
notifications_min_severity

0 commit comments

Comments
 (0)