fix: allow CheckFailure with no failed indexes

The denokv self-hosted implementation does not return indexes of failed
checks, so we need to allow this. The spec does not say that servers
MUST report indexes of failed checks, only that clients SHOULD report
failed indexes. See: denoland/denokv#110

Author: h4l

Diff for: src/denokv/datapath.py

@@ -224,18 +224,19 @@ class CheckFailure(DataPathDenoKvError):
 
     all_checks: tuple[Check, ...]
     """All of the Checks sent with the AtomicWrite."""
-    failed_check_indexes: AbstractSet[int]
+    failed_check_indexes: AbstractSet[int] | None
     """
     The indexes of Checks in all_checks keys whose versionstamp check failed.
 
-    The set is sorted with ascending iteration order.
+    The set is sorted with ascending iteration order. Will be None if the
+    database does not support reporting which checks failed.
     """
 
     def __init__(
         self,
         message: str,
         all_checks: Iterable[Check],
-        failed_check_indexes: Iterable[int],
+        failed_check_indexes: Iterable[int] | None,
         *args: object,
         endpoint: EndpointInfo,
     ) -> None:
@@ -244,12 +245,15 @@ def __init__(
         self.all_checks = tuple(all_checks)
         if len(self.all_checks) == 0:
             raise ValueError("all_checks is empty")
-        ordered_indexes = sorted(failed_check_indexes)
-        if len(ordered_indexes) == 0:
-            raise ValueError("failed_check_indexes is empty")
-        if ordered_indexes[0] < 0 or ordered_indexes[-1] >= len(self.all_checks):
+
+        ordered_indexes = sorted(failed_check_indexes) if failed_check_indexes else []
+        if len(ordered_indexes) > 0 and (
+            ordered_indexes[0] < 0 or ordered_indexes[-1] >= len(self.all_checks)
+        ):
             raise IndexError("failed_check_indexes contains out-of-bounds index")
-        self.failed_check_indexes = {i: True for i in ordered_indexes}.keys()
+        self.failed_check_indexes = (
+            {i: True for i in ordered_indexes}.keys() if ordered_indexes else None
+        )
 
 
 DataPathError: TypeAlias = (
@@ -561,15 +565,6 @@ async def atomic_write(
             )
             err.__cause__ = e
             return Err(err)
-        except ValueError as e:
-            err = ProtocolViolation(
-                "Server responded to Data Path Atomic Write with "
-                "CHECK_FAILURE containing no failed checks",
-                data=write_output,
-                endpoint=endpoint,
-            )
-            err.__cause__ = e
-            return Err(err)
     elif write_output.status == AtomicWriteStatus.AW_WRITE_DISABLED:
         return Err(
             EndpointNotUsable(

Diff for: test/test_datapath.py

@@ -5,6 +5,7 @@
 import struct
 from typing import Awaitable
 from typing import Callable
+from typing import Iterable
 from typing import Literal
 from typing import Mapping
 from typing import Sequence
@@ -261,7 +262,10 @@ async def violation_atomic_write_check_failure_with_out_of_bounds_index(
             ).SerializeToString(),
         )
 
-    async def violation_atomic_write_check_failure_without_failed_checks(
+    # The denokv self-hosted implementation does not return indexes of failed
+    # checks.
+    # https://github.com/denoland/denokv/issues/110
+    async def quirk_atomic_write_check_failure_without_failed_checks(
         request: web.Request,
     ) -> web.Response:
         write = AtomicWrite()
@@ -363,7 +367,7 @@ def add_datapath_post(app: web.Application, path: str, handler: Handler) -> None
     )
     app.router.add_post(
         "/check_failure_without_failed_checks/atomic_write",
-        violation_atomic_write_check_failure_without_failed_checks,
+        quirk_atomic_write_check_failure_without_failed_checks,
     )
     app.router.add_post("/unusable/atomic_write", unusable_atomic_write)
     app.router.add_post(
@@ -781,10 +785,12 @@ async def test_atomic_write__raises_when_given_endpoint_without_strong_consisten
         ),
         (
             "/check_failure_without_failed_checks",
-            lambda endpoint: ProtocolViolation(
-                "Server responded to Data Path Atomic Write with CHECK_FAILURE "
-                "containing no failed checks",
-                data=AtomicWriteOutput(status=AtomicWriteStatus.AW_CHECK_FAILURE),
+            lambda endpoint: CheckFailure(
+                "Not all checks required by the Atomic Write passed",
+                all_checks=[
+                    Check(key=pack_key(("x",)), versionstamp=bytes(VersionStamp(0)))
+                ],
+                failed_check_indexes=[],
                 endpoint=endpoint,
             ),
         ),
@@ -1385,6 +1391,26 @@ def test_CheckFailure(example_endpoint: EndpointInfo) -> None:
     assert msg in str(e)
 
 
+@pytest.mark.parametrize("failed_check_indexes", [None, ()])
+def test_CheckFailure__failed_check_indexes_is_None_when_no_indexes(
+    failed_check_indexes: Iterable[int] | None, example_endpoint: EndpointInfo
+) -> None:
+    checks = [
+        Check(key=bytes(KvKey(f"a{i}")), versionstamp=bytes(VersionStamp(i)))
+        for i in range(4)
+    ]
+    # Failed_check_indexes can be empty (the self-hosted sqlite implementation
+    # does not return the indexes of failed checks).
+    e = CheckFailure(
+        "Foo",
+        all_checks=iter(checks),
+        failed_check_indexes=failed_check_indexes,
+        endpoint=example_endpoint,
+    )
+    assert e.all_checks == tuple(checks)
+    assert e.failed_check_indexes is None
+
+
 def test_CheckFailure__validates_constructor_args(
     example_endpoint: EndpointInfo,
 ) -> None:
@@ -1395,11 +1421,6 @@ def test_CheckFailure__validates_constructor_args(
             "Foo", all_checks=[], failed_check_indexes=[], endpoint=example_endpoint
         )
 
-    with pytest.raises(ValueError, match=r"failed_check_indexes is empty"):
-        CheckFailure(
-            "Foo", all_checks=checks, failed_check_indexes=[], endpoint=example_endpoint
-        )
-
     with pytest.raises(
         IndexError, match=r"failed_check_indexes contains out-of-bounds index"
     ):