diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 271c1f5..42c1e05 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -140,10 +140,10 @@ jobs: services: coordinode: # Pinned by digest, not by tag: the proto submodule pins a server - # version, and a tag can be re-pushed, so `:0.5.5` alone does not name - # one fixed server. The digest below is 0.5.5; bump both together with + # version, and a tag can be re-pushed, so `:0.5.7` alone does not name + # one fixed server. The digest below is 0.5.7; bump both together with # the submodule. - image: ghcr.io/structured-world/coordinode@sha256:8d3554be7680aa2cea7b6e773037aee97513865f4698b6df0ffc2daf845c42b8 + image: ghcr.io/structured-world/coordinode@sha256:75a6242beb4cea8ab6726c842898fafc03ca9f59c6f7e36e462934a4ca64874c ports: - 7080:7080 - 7084:7084 diff --git a/README.md b/README.md index 3713fc4..b071ba8 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ Graph + Vector + Full-Text in a single transactional engine. One client, one que ## Try It in Google Colab -No setup required — runs entirely in-browser using the embedded engine: +The first four need no setup and run entirely in-browser on the embedded engine: | Notebook | Open | |----------|------| @@ -27,9 +27,16 @@ No setup required — runs entirely in-browser using the embedded engine: | 01 · LlamaIndex PropertyGraph query | [![Open in Colab](https://colab.research.google.com/assets/colab-badge.svg)](https://colab.research.google.com/github/structured-world/coordinode-python/blob/main/demo/notebooks/01_llama_index_property_graph.ipynb) | | 02 · LangChain GraphCypherQAChain | [![Open in Colab](https://colab.research.google.com/assets/colab-badge.svg)](https://colab.research.google.com/github/structured-world/coordinode-python/blob/main/demo/notebooks/02_langchain_graph_chain.ipynb) | | 03 · LangGraph agent over graph | [![Open in Colab](https://colab.research.google.com/assets/colab-badge.svg)](https://colab.research.google.com/github/structured-world/coordinode-python/blob/main/demo/notebooks/03_langgraph_agent.ipynb) | +| 04 · What 0.5 added, transactions included | [![Open in Colab](https://colab.research.google.com/assets/colab-badge.svg)](https://colab.research.google.com/github/structured-world/coordinode-python/blob/main/demo/notebooks/04_whats_new_in_0_5.ipynb) | -> Start with **00** to seed the graph — the other notebooks read from it. +> Start with **00** to seed the graph, which the other notebooks read from. > The first cell installs pre-built wheels from PyPI (~30 sec). +> +> **04 is the exception:** batch writes, consistency levels, time travel and +> transactions are distribution and durability features, so it needs a server +> rather than the embedded engine. Point `COORDINODE_ADDR` at one, or run the +> Docker Compose stack in `demo/`. Without it the notebook stops with an +> explanation instead of failing cell by cell. ## Quick Start @@ -56,6 +63,83 @@ with CoordinodeClient("localhost:7080") as db: print(row["name"]) ``` +## Transactions + +`db.cypher(...)` commits each statement on its own. To make several statements +land together, or not at all, run them in a transaction (each snippet opens +its own client, so it runs as pasted): + +```python +from coordinode import CoordinodeClient + +with CoordinodeClient("localhost:7080") as db: + with db.transaction() as tx: + tx.cypher("CREATE (:Person {name: $n})", params={"n": "Alice"}) + tx.cypher("CREATE (:Person {name: $n})", params={"n": "Bob"}) + # commits here; an exception anywhere in the block rolls back + # instead, leaving neither person in the database +``` + +The same surface is on `AsyncCoordinodeClient`, with `async with` and awaited +statements. When the commit point sits outside a block, drive it by hand: + +```python +from contextlib import suppress + +from coordinode import CoordinodeClient + +with CoordinodeClient("localhost:7080") as db: + tx = db.begin_transaction() + try: + tx.cypher("MERGE (n:Entity {name: $n})", params={"n": "Alice"}) + applied_index = tx.commit() + except BaseException: + # BaseException so an interrupt (Ctrl-C) still frees the server-side + # transaction; the rollback failure is suppressed so it cannot + # replace the error that caused it. + with suppress(Exception): + tx.rollback() + raise +``` + +Requires a CoordiNode server of **v0.5.7 or newer** — the release this client +is integration-tested against. `health()` exercises a different service, so a +server without the transaction RPCs passes the health check and then refuses +`transaction()`. + +Each statement reads the snapshot taken when the transaction began, so the +transaction sees a stable view of the database plus its own uncommitted writes, +which nobody else can see until the commit. A conflict with another transaction +that wrote the same data is reported by `commit()`, not by the statement, and a +rejected commit applies nothing. `commit()` returns the Raft applied index, which +a later read can pass as `after_index` (with `read_concern="majority"`) when it +must observe these writes. + +`tx.cypher()` takes no consistency arguments, unlike `db.cypher()`: the snapshot +is already fixed and durability is decided once at the commit, so a per-statement +read or write concern has nothing left to mean. + +Three constraints are worth knowing before holding a transaction open: + +- **It belongs to one node.** The handle lives in the memory of the server that + opened it, so every request of the transaction must reach that same node. + Connect to a node's own address, or through a proxy configured for backend + affinity. A single client is *not* by itself a guarantee: against a layer-7 + or per-request gRPC balancer the calls can be spread across backends, and a + reconnection can move to another backend mid-transaction, after which the + next statement fails with an unknown transaction id. +- **Idle transactions are collected.** The server reaps one that has been idle + (30 seconds by default), and it sweeps when another transaction begins rather + than on a timer, so a long pause between statements can lose the handle. A + failed statement also ends the transaction outright: its writes are discarded + and the handle is closed, so reusing it raises rather than reporting a + confusing error from the server. +- **A lost reply is not an abort.** If the connection drops or a deadline + expires while committing, the server may have applied everything or nothing, + and the client cannot tell. The transaction is marked indeterminate: later + calls on it say so, and `rollback()` raises instead of promising a discard. + Verify the data rather than blindly retrying, which can duplicate the writes. + ## LangChain — GraphRAG Pipeline ```python diff --git a/coordinode-embedded/Cargo.lock b/coordinode-embedded/Cargo.lock index 0a9c022..bea3925 100644 --- a/coordinode-embedded/Cargo.lock +++ b/coordinode-embedded/Cargo.lock @@ -525,7 +525,7 @@ dependencies = [ [[package]] name = "coordinode-cluster" -version = "0.5.5" +version = "0.5.7" dependencies = [ "coordinode-storage", "parking_lot", @@ -538,7 +538,7 @@ dependencies = [ [[package]] name = "coordinode-core" -version = "0.5.5" +version = "0.5.7" dependencies = [ "bytes", "rmp", @@ -554,7 +554,7 @@ dependencies = [ [[package]] name = "coordinode-embed" -version = "0.5.5" +version = "0.5.7" dependencies = [ "coordinode-core", "coordinode-lsm-tree", @@ -575,7 +575,7 @@ dependencies = [ [[package]] name = "coordinode-embedded" -version = "1.0.6" +version = "2.0.0" dependencies = [ "coordinode-core", "coordinode-embed", @@ -588,9 +588,9 @@ dependencies = [ [[package]] name = "coordinode-lsm-tree" -version = "5.7.0" +version = "5.8.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c18dafd43ebda2d9f2cc29b7aaf7897c168bc62896a6d4ec2e81a45bc02791dd" +checksum = "ad26b0c73ee5b2070f376602394efa2a54797d8f29a0ef2caa75a0a0d6e04e6d" dependencies = [ "arc-swap", "cpufeatures 0.3.0", @@ -599,7 +599,7 @@ dependencies = [ "libc", "libm", "log", - "lz4_flex 0.13.0", + "lz4_flex 0.14.0", "once_cell", "parking_lot", "portable-atomic", @@ -613,7 +613,7 @@ dependencies = [ [[package]] name = "coordinode-modality" -version = "0.5.5" +version = "0.5.7" dependencies = [ "coordinode-core", "coordinode-lsm-tree", @@ -629,7 +629,7 @@ dependencies = [ [[package]] name = "coordinode-query" -version = "0.5.5" +version = "0.5.7" dependencies = [ "coordinode-cluster", "coordinode-core", @@ -655,7 +655,7 @@ dependencies = [ [[package]] name = "coordinode-raft" -version = "0.5.5" +version = "0.5.7" dependencies = [ "bytes", "coordinode-core", @@ -678,7 +678,7 @@ dependencies = [ [[package]] name = "coordinode-search" -version = "0.5.5" +version = "0.5.7" dependencies = [ "aes-gcm", "coordinode-storage", @@ -695,7 +695,7 @@ dependencies = [ [[package]] name = "coordinode-storage" -version = "0.5.5" +version = "0.5.7" dependencies = [ "byteorder-lite", "bytes", @@ -722,7 +722,7 @@ dependencies = [ [[package]] name = "coordinode-vector" -version = "0.5.5" +version = "0.5.7" dependencies = [ "coordinode-core", "coordinode-storage", @@ -737,7 +737,7 @@ dependencies = [ [[package]] name = "coordinode-wire" -version = "0.5.5" +version = "0.5.7" dependencies = [ "bytes", "prost", @@ -1147,7 +1147,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.61.2", + "windows-sys 0.52.0", ] [[package]] @@ -1900,7 +1900,7 @@ version = "0.50.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7957b9740744892f114936ab4a57b3f487491bbeafaf8083688b16841a4240e5" dependencies = [ - "windows-sys 0.61.2", + "windows-sys 0.59.0", ] [[package]] @@ -2798,7 +2798,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys", - "windows-sys 0.61.2", + "windows-sys 0.52.0", ] [[package]] @@ -3343,7 +3343,7 @@ dependencies = [ "getrandom 0.4.2", "once_cell", "rustix", - "windows-sys 0.61.2", + "windows-sys 0.52.0", ] [[package]] diff --git a/coordinode-embedded/Cargo.toml b/coordinode-embedded/Cargo.toml index f24ddf7..45fc909 100644 --- a/coordinode-embedded/Cargo.toml +++ b/coordinode-embedded/Cargo.toml @@ -18,6 +18,11 @@ name = "_coordinode_embedded" crate-type = ["cdylib"] [dependencies] +# Moving the coordinode-rs submodule means refreshing Cargo.lock here in the +# same commit. These are path dependencies, so their own registry dependencies +# are resolved by THIS lock: an engine release that needs a newer storage +# engine still compiles against whatever version the lock names, and fails on +# the API it expected to find. pyo3 = { version = "0.24", features = ["extension-module", "abi3-py311"] } numpy = "0.24" coordinode-embed = { path = "../coordinode-rs/crates/coordinode-embed" } diff --git a/coordinode-rs b/coordinode-rs index c4488d7..ff122a3 160000 --- a/coordinode-rs +++ b/coordinode-rs @@ -1 +1 @@ -Subproject commit c4488d7315128e797a3e27228f0bf010b207610e +Subproject commit ff122a3dc17bfc14bf96336ab6cb414cb10db196 diff --git a/coordinode/README.md b/coordinode/README.md index 1897bec..e93ff66 100644 --- a/coordinode/README.md +++ b/coordinode/README.md @@ -171,17 +171,22 @@ db.cypher( read_concern="majority", ) -# Majority write (required for causal reads) +# Durable write, acknowledged by a majority of the cluster db.cypher("CREATE (n:Event {t: timestamp()})", write_concern="majority") -# Causal read: see at least state at raft index 42 -db.cypher("MATCH (n) RETURN count(n) AS total", after_index=42) +# Causal read: see at least the state at raft index 42. The fence is about +# which replica may answer, so it is the READ concern that has to be majority. +db.cypher( + "MATCH (n) RETURN count(n) AS total", + after_index=42, + read_concern="majority", +) ``` Accepted values: -- ``read_concern``: ``local`` (default) · ``majority`` · ``linearizable`` · ``snapshot`` -- ``write_concern``: ``w0`` · ``w1`` (default) · ``majority`` +- ``read_concern``: ``local`` (default) · ``majority`` · ``linearizable`` · ``snapshot``. Causal reads (``after_index`` > 0) require ``majority`` here. +- ``write_concern``: ``w0`` · ``memory`` · ``cache`` · ``w1`` (default) · ``majority`` - ``read_preference``: ``primary`` (default) · ``primary_preferred`` · ``secondary`` · ``secondary_preferred`` · ``nearest`` ## Related Packages diff --git a/coordinode/coordinode/__init__.py b/coordinode/coordinode/__init__.py index 7c64c2c..16c2b55 100644 --- a/coordinode/coordinode/__init__.py +++ b/coordinode/coordinode/__init__.py @@ -1,5 +1,5 @@ """ -CoordiNode Python SDK — graph + vector + full-text in one query. +CoordiNode Python SDK: graph + vector + full-text in one query. Quick start:: @@ -21,6 +21,7 @@ from coordinode._types import MultiVector, Path from coordinode.client import ( AsyncCoordinodeClient, + AsyncTransaction, CoordinodeClient, EdgeResult, EdgeTypeInfo, @@ -29,6 +30,7 @@ PropertyDefinitionInfo, TextIndexInfo, TextResult, + Transaction, TraverseResult, VectorResult, ) @@ -40,6 +42,11 @@ __all__ = [ "CoordinodeClient", "AsyncCoordinodeClient", + # Handles for a group of statements that commits or rolls back as one. + # Exported for type annotations; both come from a client, never built + # directly, since only the server can hand out the transaction id. + "Transaction", + "AsyncTransaction", # Values whose wire type only a tag can carry: a plain nested list encodes # as a list and a plain dict as a map, so sending either of these types # requires the constructor, not just the shape. diff --git a/coordinode/coordinode/client.py b/coordinode/coordinode/client.py index f462635..77ad413 100644 --- a/coordinode/coordinode/client.py +++ b/coordinode/coordinode/client.py @@ -1,5 +1,5 @@ """ -CoordinodeClient — synchronous and asynchronous gRPC client for CoordiNode. +CoordinodeClient: synchronous and asynchronous gRPC client for CoordiNode. """ from __future__ import annotations @@ -7,7 +7,8 @@ import asyncio import logging import re -from collections.abc import Sequence +from collections.abc import AsyncIterator, Iterator, Sequence +from contextlib import asynccontextmanager, contextmanager, suppress from typing import Any import grpc @@ -33,6 +34,87 @@ # names/labels/properties into DDL strings to surface clear errors early. _CYPHER_IDENT_RE = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$") +# gRPC codes that do NOT prove the server processed the request: the request or +# its reply was lost somewhere in transit. Every other code is the server +# answering, and an answered failure consumes the transaction's handle. +# +# This is consulted for the COMMIT only, where the question is "were the writes +# applied", which no follow-up request can answer. Statement failures do not +# consult it: there the only question is whether server-side state needs +# freeing, and asking for that is harmless whatever the answer, so the +# statement path cleans up unconditionally rather than risk misjudging a code. +# Codes the transport layer never fabricates on its own: seeing one proves +# the server answered, so the commit was definitively rejected and nothing +# was applied. The list is deliberately NOT the complement of "transport +# codes": RESOURCE_EXHAUSTED, INTERNAL, DATA_LOSS and the like can be +# generated inside the CLIENT while receiving or decoding a reply the server +# already acted on, so on their own they prove nothing. +_DEFINITIVE_REJECTION_CODES = frozenset( + { + grpc.StatusCode.ABORTED, + grpc.StatusCode.NOT_FOUND, + grpc.StatusCode.INVALID_ARGUMENT, + grpc.StatusCode.FAILED_PRECONDITION, + grpc.StatusCode.ALREADY_EXISTS, + grpc.StatusCode.OUT_OF_RANGE, + grpc.StatusCode.PERMISSION_DENIED, + grpc.StatusCode.UNAUTHENTICATED, + grpc.StatusCode.UNIMPLEMENTED, + } +) + + +# The transient states an operation passes through while its RPC is in +# flight, mapped to what to call the operation in a message. A handle in one +# of these is neither open nor settled: nothing else may run on it, and a +# context manager exiting over one is exiting over unfinished work. +_IN_FLIGHT_STATES = { + "executing": "statement", + "committing": "commit", + "rolling_back": "rollback", +} + + +def _caller_is_being_cancelled() -> bool: + """Whether a cancellation of the running task is actually pending. + + A cleanup RPC can raise :exc:`asyncio.CancelledError` on its own — a + channel closing under it, say — with nobody having cancelled this task. + Propagating that would replace the outcome the caller needs (the + statement's failure, the commit's error, a clean exit) with a + cancellation that never happened. Only a real pending cancellation + supersedes those, and asyncio does not re-inject a swallowed one, so it + must be re-raised rather than dropped. + """ + task = asyncio.current_task() + return task is not None and bool(task.cancelling()) + + +def _rpc_outcome_is_ambiguous(exc: grpc.RpcError) -> bool: + """Whether this failure leaves the server's state unknowable. + + Only positive proof that the server ANSWERED reads as a decided rejection: + either a status code the transport never generates locally, or the + server's structured error details riding the trailing metadata (every + rejection the server classifies carries them). Everything else — codes a + client can produce mid-reply, an error that cannot even report a code — + is ambiguous, because the two mistakes are not symmetric: warning about an + outcome that turned out to be a clean rejection costs the caller one + verification, while a wrongly claimed abort invites a retry that + duplicates every write. + """ + try: + if exc.code() in _DEFINITIVE_REJECTION_CODES: + return False + trailing = exc.trailing_metadata() + for entry in trailing or (): + key = entry[0] if isinstance(entry, tuple) else getattr(entry, "key", None) + if key == "grpc-status-details-bin": + return False + return True + except Exception: + return True + def _validate_cypher_identifier(value: str, param_name: str) -> None: """Raise :exc:`ValueError` if *value* is not a valid Cypher identifier.""" @@ -198,6 +280,521 @@ def __repr__(self) -> str: # ── Async client ───────────────────────────────────────────────────────────── +# Deadline for cleanup RPCs whose result nobody reads (the best-effort +# rollback). Deliberately short and NOT the client's request timeout: cleanup +# runs on paths that may already have burned the full deadline, and doubling +# a 30-second worst case for an ignored answer helps no one. The server's +# idle sweep remains the backstop when even this expires. +_CLEANUP_TIMEOUT_SECS = 5.0 + + +class AsyncTransaction: + """One interactive transaction, held open across several statements. + + Obtained from :meth:`AsyncCoordinodeClient.begin_transaction`, or from + :meth:`AsyncCoordinodeClient.transaction`, which commits on a clean exit and + rolls back on an exception. + + Every statement reads the snapshot pinned when the transaction began, and + its writes buffer on the server until :meth:`commit`. So the transaction + sees a stable view of the database plus its own writes, and a conflict with + another transaction that touched the same data surfaces at the commit rather + than at the statement. + + Two properties of the server matter before holding one open. The handle + lives in the memory of the node that served the begin, so every request of + the transaction has to reach that same node: connect to a node's own + address, or through a balancer configured for backend affinity. A balancer + that routes each request independently breaks this even through a single + client, since a reconnection can land on another backend mid-transaction, + and the next statement then fails with an unknown transaction id. And an + idle transaction is reaped, after 30 seconds by default, swept when some + other transaction begins rather than on a timer, so a long pause between + statements can lose the handle without a wall-clock guarantee of when. + """ + + def __init__(self, client: AsyncCoordinodeClient, transaction_id: int) -> None: + self._client = client + self._id = transaction_id + # open -> committed | rolled_back | aborted | indeterminate, with two + # transient in-flight states (executing, committing) that strictly + # serialize operations on one handle. + # "aborted" is the server having closed the transaction under us, which + # it does on any statement error and on a rejected commit. + # "indeterminate" is a commit whose reply was lost in transit: the + # writes may all be applied or none may be, and nothing on the client + # can tell which. + self._state = "open" + # Whether the last best-effort cleanup RPC actually completed. False + # after a cleanup that itself failed (both RPCs lost to the same + # outage, say): the server may still hold the transaction, so an + # explicit rollback() on the aborted handle retries the cleanup + # instead of declaring the transaction already gone. + self._cleanup_confirmed = True + # Set by the context manager when it exits while an operation started + # inside the block is still in flight: the straggler, on completion, + # hands the transaction to cleanup instead of returning the handle to + # "open" with nobody left to commit or roll it back. + self._abandoned = False + # The pending detached cleanup, if any: several unwinding layers can + # each ask for one (rollback() itself, then the context manager), and + # one bounded attempt per transaction is enough. + self._cleanup_task: asyncio.Task[None] | None = None + + def __repr__(self) -> str: + return f"AsyncTransaction(id={self._id}, state={self._state})" + + @property + def transaction_id(self) -> int: + """Server-side handle for this transaction. Non-zero while it exists.""" + return self._id + + @property + def is_open(self) -> bool: + """True while the transaction can still take statements and be committed.""" + return self._state == "open" + + def _require_open(self, action: str) -> None: + if self._state == "open": + return + if self._state in _IN_FLIGHT_STATES: + in_flight = _IN_FLIGHT_STATES[self._state] + raise RuntimeError( + f"Cannot {action} this transaction: a {in_flight} on it is already in " + "flight. Concurrent operations on one transaction handle would race " + "its outcome; await the in-flight operation instead." + ) + if self._state == "aborted": + raise RuntimeError( + f"Cannot {action} this transaction: an earlier failure closed it on the " + "server, which discards its buffered writes. Nothing was applied; begin " + "a new transaction to retry." + ) + if self._state == "indeterminate": + raise RuntimeError( + f"Cannot {action} this transaction: the commit's reply was lost and the " + "outcome is unknown. The writes may or may not be applied; verify the " + "data before retrying, since a blind retry can duplicate them." + ) + raise RuntimeError(f"Cannot {action} this transaction: it was already {self._state.replace('_', ' ')}.") + + def _spawn_cleanup(self) -> None: + """Run :meth:`_best_effort_rollback` as a detached task. + + Used from cancellation handlers, where awaiting the rollback in place + would only be cancelled again. The task is tracked on the CLIENT so + that (a) it stays referenced until done — an unreferenced task can be + garbage-collected mid-flight — and (b) `close()` drains it before the + channel goes away, so the cleanup never races the transport it needs. + If the event loop stops before it runs, the server's idle sweep + remains the backstop, which is exactly what best-effort means. + """ + # Unconfirmed from the moment the cleanup is REQUESTED, not from its + # first step — including when the closing gate below skips the spawn + # entirely: the statement may have reached the server, and an + # explicit rollback() (before the detached task's first turn, or + # after a reconnect) must see the cleanup as not-yet-done and retry + # it rather than trust one that never ran. + self._cleanup_confirmed = False + if self._client._closing: + # The channel is (about to be) gone: a spawn now could only fail + # against it. The server's idle sweep collects the transaction. + return + if self._cleanup_task is not None and not self._cleanup_task.done(): + # Several unwinding layers can each request the cleanup (the + # rollback that was cancelled, then the context manager on its + # way out); one pending bounded attempt is enough. + return + pending = self._client._pending_cleanups + task = asyncio.get_running_loop().create_task(self._best_effort_rollback()) + self._cleanup_task = task + pending.add(task) + task.add_done_callback(pending.discard) + + def _settle_cancelled_scope(self) -> None: + """Settle this handle when its owning scope is unwound by cancellation. + + A scope unwound this way must not hold for a rollback round trip: the + caught CancelledError is not re-injected, so an inline await could + overrun a surrounding timeout by the whole cleanup deadline. Every + branch therefore detaches (bounded, drained at close) and returns at + once. Synchronous by design, so a cancellation arriving at any await + of the exit path can call it. + """ + if self.is_open: + self._state = "aborted" + self._spawn_cleanup() + elif self._state == "indeterminate": + # A manual commit() inside the block was cancelled mid-flight: + # its request may never have reached the server, leaving the + # transaction open there with the caller gone. The verdict stays + # indeterminate either way. + self._spawn_cleanup() + elif self._state in _IN_FLIGHT_STATES: + # An operation started in a background task is still in flight + # while this scope unwinds; it cannot be awaited or cancelled + # from here, so the straggler is marked to hand the transaction + # to cleanup when it completes. An in-flight COMMIT is + # additionally contested with a detached rollback: a successful + # commit cannot be retracted afterwards, so the only honest shot + # at the rollback-on-cancellation contract is letting the server + # race decide which request wins. + self._abandoned = True + if self._state == "committing": + self._spawn_cleanup() + elif self._state == "aborted" and not self._cleanup_confirmed: + # A manual rollback() cancelled mid-RPC: the request was + # interrupted, not answered, so the server may still hold the + # transaction. Retry detached before the scope is gone. + self._spawn_cleanup() + + async def _best_effort_rollback(self) -> None: + """Ask the server to drop the transaction, ignoring every failure. + + Used where the rollback is cleanup rather than the caller's request: a + transaction that may or may not still exist server-side. When it does + exist this frees its buffered writes now instead of at the idle sweep; + when it does not, the server answers "unknown transaction id" and there + was nothing to free. Neither answer changes what the caller is told. + """ + from coordinode._proto.coordinode.v1.query.cypher_pb2 import ( # type: ignore[import] + RollbackTransactionRequest, + ) + + self._cleanup_confirmed = False + try: + await self._client._cypher_stub.RollbackTransaction( + RollbackTransactionRequest(transaction_id=self._id), + # Capped by the client's own timeout too: a caller who + # configured 100ms requests must not find a failed statement + # holding the line for a 5-second cleanup. + timeout=min(_CLEANUP_TIMEOUT_SECS, self._client._timeout), + ) + self._cleanup_confirmed = True + except grpc.RpcError as exc: + # NOT_FOUND is the server's "unknown transaction id": a definitive + # statement that nothing is held under this id any more — as + # settled as a successful rollback. Every other failure (a lost + # request most of all) leaves the cleanup unconfirmed, retriable. + with suppress(Exception): + if exc.code() == grpc.StatusCode.NOT_FOUND: + self._cleanup_confirmed = True + except Exception: + pass + + async def _cleanup_preserving_outcome(self) -> None: + """Run the bounded cleanup where the outcome is already decided. + + Every exit path that reaches here has something of its own to report: + the block's exception, or a clean exit. A cleanup failure is never + worth replacing that, so it stays swallowed. A REAL cancellation is + the exception, because asyncio does not re-inject a swallowed one and + silence would turn a cancelled exit into a successful-looking one; it + propagates, handing the interrupted request to a detached retry first + so the server transaction is still freed. + """ + try: + await self._best_effort_rollback() + except asyncio.CancelledError: + self._spawn_cleanup() + if _caller_is_being_cancelled(): + raise + + async def cypher( + self, + query: str, + params: dict[str, PyValue] | None = None, + ) -> list[dict[str, Any]]: + """Run one statement inside this transaction and return its rows. + + The write is buffered rather than applied, so it is visible to later + statements of this transaction and to nobody else until :meth:`commit`. + + This deliberately takes no consistency arguments, unlike + :meth:`AsyncCoordinodeClient.cypher`. Read concern, write concern, read + preference and ``after_index`` describe a single self-contained + statement; here the snapshot was already fixed at the begin and + durability is decided once at the commit, so the server ignores them. + Accepting them would only let a caller believe otherwise. + + A statement that fails on the server ends the transaction: the buffered + writes are discarded and the handle is consumed. The failure propagates + as-is, and any later use of this object raises instead of reporting the + server's "unknown transaction id". + """ + from coordinode._proto.coordinode.v1.query.cypher_pb2 import ( # type: ignore[import] + ExecuteCypherRequest, + ) + + self._require_open("run a statement in") + # The request is built BEFORE the state transition: parameter + # encoding can fail locally (an unsupported Python type), and that + # failure changes nothing server-side, so it must leave the handle + # open rather than marooned in an in-flight state. + req = ExecuteCypherRequest( + query=query, + parameters=dict_to_props(params or {}), + transaction_id=self._id, + ) + # Transition BEFORE the await, mirroring commit(): a concurrent + # commit slipping in while this statement is in flight could land + # without the statement's write, and the statement's late "unknown + # transaction" failure would then overwrite the real outcome. + self._state = "executing" + try: + resp = await self._client._cypher_stub.ExecuteCypher(req, timeout=self._client._timeout) + except grpc.RpcError: + # Always attempt the cleanup, without classifying the failure. + # Whether the server processed the statement decides only whether + # the rollback finds anything: if it aborted the transaction the + # request is answered "unknown transaction id" and swallowed, and + # if it kept the transaction open (a lost request, or a limit the + # CLIENT hit while receiving an oversized reply) the buffered + # writes are freed now instead of at the idle sweep. + # + # Classifying here was a way to miss cases: every code judged + # "answered" that the server did not actually act on leaks a + # transaction for the idle timeout. The cost of not classifying is + # one wasted RPC on a path that is already failing. + self._state = "aborted" + try: + await self._best_effort_rollback() + except asyncio.CancelledError: + # A cancellation landing DURING the inline cleanup would lose + # it (the handle is already closed, nothing retries later): + # detach a fresh attempt. Only a REAL pending cancellation of + # this task then supersedes the statement's own failure; a + # CancelledError raised by the transport itself (a closing + # channel) must not replace the error the caller needs. + self._spawn_cleanup() + if _caller_is_being_cancelled(): + raise + raise + except asyncio.CancelledError: + # Cancellation is a BaseException, so the handler above never sees + # it. No commit was sent, so nothing of this transaction can apply; + # the handle is closed rather than left open for reuse. The + # cancellation may still have arrived AFTER the server accepted + # the statement, leaving the transaction alive there with its + # buffered writes and a pinned snapshot — and with the handle + # closed, nothing later can free it before the idle sweep. The + # cleanup therefore runs as its own task: awaiting it here would + # only be cancelled again, while a detached task survives this + # task's cancellation. (A commit is different — see commit().) + self._state = "aborted" + self._spawn_cleanup() + raise + except BaseException: + # KeyboardInterrupt / SystemExit raised from inside the awaited + # call (or any failure that is neither gRPC nor cancellation) + # matches none of the handlers above and would park the handle + # in "executing" forever. Settle conservatively: no commit was + # sent, so nothing can apply — close as aborted with a detached + # bounded cleanup, and let the exception propagate. + self._state = "aborted" + self._spawn_cleanup() + raise + if self._abandoned: + # The owning context exited while this statement was still in + # flight: nobody is left to commit or roll back, so the buffered + # writes and pinned snapshot are handed to cleanup now instead of + # leaking until the idle sweep. The rows still go to whoever + # awaits the straggler. + self._state = "aborted" + self._spawn_cleanup() + return _rows_to_dicts(resp) + self._state = "open" + return _rows_to_dicts(resp) + + async def commit(self) -> int: + """Apply every buffered write as one unit. + + Returns the Raft applied index of the commit, which a later read can + pass as ``after_index`` (with ``read_concern="majority"``) when it must + observe these writes. + + Raises if another transaction has written the same data since this one + began: conflicts are detected here, not at the statement. A rejected + commit applies nothing and closes the transaction. + + One failure is different from the rest: a commit whose reply is lost in + transit (a deadline, an unavailable channel). The server may have + applied everything before the failure, or never received the request, + and nothing on the client can tell which. The transaction is then + marked indeterminate rather than aborted, and every later call on it + says so: retrying such a commit blindly can duplicate the writes, so + the data has to be verified first. + """ + from coordinode._proto.coordinode.v1.query.cypher_pb2 import ( # type: ignore[import] + CommitTransactionRequest, + ) + + self._require_open("commit") + # Transition BEFORE the await: a concurrent operation on this handle + # would otherwise pass its own open-check while the commit is in + # flight, and the loser's "unknown transaction" rejection would + # overwrite the real outcome (committed) with a claimed abort. + self._state = "committing" + try: + resp = await self._client._cypher_stub.CommitTransaction( + CommitTransactionRequest(transaction_id=self._id), timeout=self._client._timeout + ) + except grpc.RpcError as exc: + if _rpc_outcome_is_ambiguous(exc): + self._state = "indeterminate" + if self._abandoned: + # The owning context is gone; nobody will run the + # indeterminate cleanup, so it goes detached from here. + self._spawn_cleanup() + else: + # An answered rejection (a write conflict, most commonly): the + # server consumed the handle and applied nothing, so a + # follow-up rollback would find nothing to discard. + self._state = "aborted" + raise + except asyncio.CancelledError: + # Cancellation is a BaseException, so the gRPC handler above never + # sees it, and a deadline enforced by `asyncio.timeout()` arrives + # this way rather than as DEADLINE_EXCEEDED. The request may have + # reached the server and applied everything, which is exactly the + # case the indeterminate state exists for: leaving the transaction + # open here would invite the retry that duplicates the writes. + self._state = "indeterminate" + if self._abandoned: + # The owning context is gone; nobody will run the + # indeterminate cleanup, so it goes detached from here. + self._spawn_cleanup() + raise + except BaseException: + # KeyboardInterrupt / SystemExit raised from inside the awaited + # call matches neither handler above and would park the handle in + # "committing" forever. The request may already have applied, so + # the only honest settlement is indeterminate — as the sync + # wrapper already does for interrupts at the loop boundary. + self._state = "indeterminate" + if self._abandoned: + self._spawn_cleanup() + raise + self._state = "committed" + return int(resp.applied_index) + + async def rollback(self) -> None: + """Discard every buffered write. Nothing reaches the database. + + After a commit whose reply was lost, that promise cannot be made: the + writes may already be applied. This then sends the rollback anyway, in + case the commit never arrived, and still raises, so nobody walks away + believing the discard is certain. + """ + from coordinode._proto.coordinode.v1.query.cypher_pb2 import ( # type: ignore[import] + RollbackTransactionRequest, + ) + + if self._state == "indeterminate": + # If the commit never reached the server this frees the + # transaction; if it was applied, nothing can un-apply it. + try: + await self._best_effort_rollback() + except asyncio.CancelledError: + # A direct caller has no context manager to retry for it: + # hand the interrupted cleanup to a detached task. Only a + # real cancellation then supersedes the indeterminate + # verdict below; a transport-raised one must not hide it. + self._spawn_cleanup() + if _caller_is_being_cancelled(): + raise + raise RuntimeError( + "Cannot promise a rollback: the commit's reply was lost, so its writes " + "may already be applied. A rollback request was sent in case the commit " + "never arrived, but verify the data rather than assuming either outcome." + ) + if self._state == "aborted": + # The failure that closed the transaction already discarded the + # writes, so this call's contract is met. Asking the server would + # only get "unknown transaction id" for a transaction that is + # correctly gone — UNLESS the cleanup that was supposed to free it + # never got through (both RPCs lost to the same outage): then the + # server may still hold it, and connectivity may have recovered + # since, so the cleanup is retried here before settling. + if not self._cleanup_confirmed: + try: + await self._best_effort_rollback() + except asyncio.CancelledError: + # Same rule as the open and indeterminate branches: a + # direct caller has no exit handler to retry for it, so + # the interrupted retry goes detached. Only a real + # cancellation propagates; a transport-raised one must + # not fail a rollback that has met its contract. + self._spawn_cleanup() + if _caller_is_being_cancelled(): + raise + if not self._cleanup_confirmed: + # The retry failed too. The discard promise still holds + # (no commit was ever sent), but the server may hold the + # transaction until its idle sweep — so stay "aborted", + # keeping a later explicit rollback() able to retry once + # connectivity recovers, instead of settling into a + # state that refuses to. + return + self._state = "rolled_back" + return + self._require_open("roll back") + # Closed before the call, not after it. If the request is lost the + # server may hold the transaction until the idle sweep, but no commit + # was ever sent, so nothing of it can apply and the discard this method + # promises still holds. What must not happen is the handle staying + # usable: a caller who asked to discard should not be able to add + # another statement, or commit, because their rollback did not land. + # The failure still propagates, so they know the request did not + # arrive. The state is transient rather than terminal until the RPC + # settles, so a context manager exiting over a rollback still in + # flight sees an in-flight operation rather than a finished one. + self._state = "rolling_back" + try: + await self._client._cypher_stub.RollbackTransaction( + RollbackTransactionRequest(transaction_id=self._id), timeout=self._client._timeout + ) + except asyncio.CancelledError: + # A cancelled owner may never call again, and a direct caller + # has no context manager to retry for it: hand the cleanup to a + # detached task (which also marks it unconfirmed) before the + # cancellation propagates. + self._state = "aborted" + self._spawn_cleanup() + raise + except grpc.RpcError as exc: + # NOT_FOUND means the server holds nothing under this id, which + # for a transaction that was never committed is the discard this + # method promises, already done — most often by the idle sweep + # reclaiming a long-open transaction. Reporting that as a failure + # would send the caller back for a second rollback that can only + # be answered the same way. Every other code leaves the request's + # fate unknown and falls through to the retriable handling below. + with suppress(Exception): + if exc.code() == grpc.StatusCode.NOT_FOUND: + self._cleanup_confirmed = True + self._state = "rolled_back" + return + self._state = "aborted" + self._cleanup_confirmed = False + if self._abandoned: + self._spawn_cleanup() + raise + except BaseException: + # The request may never have arrived: the discard promise still + # holds (no commit was ever sent), but the server may hold the + # transaction until its idle sweep. "aborted" with the cleanup + # unconfirmed keeps the handle unusable for statements while a + # later rollback() can still retry once connectivity recovers; + # the failure propagates so the caller knows it did not land. + self._state = "aborted" + self._cleanup_confirmed = False + if self._abandoned: + # The owning scope is gone; nobody is left to retry. + self._spawn_cleanup() + raise + self._state = "rolled_back" + class AsyncCoordinodeClient: """ @@ -242,6 +839,19 @@ def __init__( self._tls = tls self._timeout = timeout self._channel: grpc.aio.Channel | None = None + # Detached cleanup tasks spawned by cancellation handlers, referenced + # here until done (an unreferenced task can be garbage-collected + # mid-flight) and drained by close() BEFORE the channel goes away, so + # a cleanup never races the transport it needs. + self._pending_cleanups: set[asyncio.Task[None]] = set() + # Set once close() has finished draining AND released the channel: + # later cancellations forfeit their cleanup to the server's idle + # sweep instead of spawning a task that could only fail against the + # closed channel. + self._closing = False + # The single finalization task behind close(); shared by concurrent + # and repeated close() calls, and immune to their cancellation. + self._close_task: asyncio.Task[None] | None = None async def __aenter__(self) -> AsyncCoordinodeClient: await self.connect() @@ -251,6 +861,27 @@ async def __aexit__(self, *_: Any) -> None: await self.close() async def connect(self) -> None: + # A reconnect must not race an in-flight shutdown: when it resumes, + # the finalizer unconditionally clears the channel and raises the + # closing gate, which would clobber a transport installed under it — + # cleanup silently disabled, the new channel unreleasable. Wait for + # the old teardown to finish first (shielded, so cancelling this + # connect does not abandon that shutdown midway). + task = self._close_task + if task is not None and not task.done(): + try: + await asyncio.shield(task) + except asyncio.CancelledError: + if not task.cancelled(): + # It was the CALLER of connect() that got cancelled, not + # the finalizer; the cancellation must propagate. + raise + except Exception: + # A failed shutdown left a broken transport behind; that is + # no reason to refuse a fresh one. + pass + self._closing = False + self._close_task = None self._channel = _make_async_channel(self._host, self._port, self._tls) self._cypher_stub = _cypher_stub(self._channel) self._vector_stub = _vector_stub(self._channel) @@ -260,9 +891,59 @@ async def connect(self) -> None: self._health_stub = _health_stub(self._channel) async def close(self) -> None: - if self._channel: - await self._channel.close() + # Shutdown runs in its own task so that cancelling the CALLER of + # close() cannot abandon it midway: only the shielded await here is + # cancelled, while the finalization drains the cleanups and releases + # the channel to the end. A finalization that itself failed is not + # cached — the next close() starts over against whatever transport + # state the failure left behind. + task = self._close_task + if task is None or (task.done() and not task.cancelled() and task.exception() is not None): + task = asyncio.get_running_loop().create_task(self._finalize_close()) + self._close_task = task + await asyncio.shield(task) + + async def _finalize_close(self) -> None: + # Detached cancellation cleanups first: closing the channel under a + # cleanup in flight would strand its transaction on the server until + # the idle sweep. Drained until the set is STABLE, not from a single + # snapshot — a statement cancelled while the drain awaits an earlier + # cleanup adds its task after the snapshot, and one gather would + # strand it against a closed transport. Each task is bounded by the + # cleanup deadline and each round only exists because a new one was + # spawned, so the loop ends as soon as callers stop cancelling work. + while True: + while self._pending_cleanups: + batch = list(self._pending_cleanups) + await asyncio.gather(*batch, return_exceptions=True) + # Removed explicitly rather than trusting the done-callbacks: + # awaiting an already-finished task does not yield to the loop, + # so the callbacks may not have run yet and the while would spin. + self._pending_cleanups.difference_update(batch) + channel = self._channel + if channel is None: + break + # The closing gate stays OPEN through the channel close: a + # statement cancelled while the close is in progress spawns its + # cleanup, which runs concurrently against the still-live + # transport — and the next drain round above collects it. The + # rollback stays possible until the transport is conclusively + # gone, not merely scheduled to go. + # + # One window remains open BY DESIGN: a cleanup registered in the + # very loop turn in which the transport finishes closing is still + # drained below, but its request meets a dead channel and is + # suppressed. Closing that window would require holding the + # channel open until every in-flight operation settles, letting a + # single hung statement block shutdown forever; the server's idle + # sweep is the documented backstop for that residue instead. + await channel.close() self._channel = None + # Only now is the transport conclusively unavailable: a statement + # cancelled later must not spawn a cleanup that could only fail + # against the closed channel — the server's idle sweep is the + # backstop for those. + self._closing = True async def cypher( self, @@ -280,15 +961,17 @@ async def cypher( Consistency parameters (all optional; server defaults apply when omitted): - ``read_concern``: ``"local"`` (default), ``"majority"``, ``"linearizable"``, ``"snapshot"``. + Causal reads (``after_index`` > 0) require ``"majority"`` here. - ``write_concern``: ``"w0"``, ``"memory"``, ``"cache"``, ``"w1"`` (default, leader-ack), ``"majority"``, in rising order of durability. ``"memory"`` and ``"cache"`` acknowledge before the write reaches Raft, so a leader crash before the background drain loses them; - reach for those only where losing recent writes is acceptable. Causal reads - (``after_index`` > 0) require ``"majority"``. + reach for those only where losing recent writes is acceptable. - ``read_preference``: ``"primary"`` (default), ``"primary_preferred"``, ``"secondary"``, ``"secondary_preferred"``, ``"nearest"``. - ``after_index``: raft log index for causal reads, a fence. Returned rows reflect at - least the state at this index. + least the state at this index. Needs ``read_concern="majority"``: the fence is about + which replica may answer, so it is the read's concern that has to be raised, not the + write's. - ``at_timestamp``: timestamp to read at, a pin rather than a fence. Reads the database exactly as of that version without waiting, for time travel. Microseconds since the Unix epoch, so ``int(time.time() * 1_000_000)`` is now. Requires @@ -311,13 +994,23 @@ async def cypher( not isinstance(after_index, int) or isinstance(after_index, bool) or after_index < 0 ): raise ValueError(f"after_index must be a non-negative integer, got {after_index!r}") - # Causal reads (after_index > 0) are only satisfiable when writes were - # acknowledged by a majority; otherwise the referenced index may never - # replicate and the read would hang. Mirror the server's rejection. - if after_index is not None and after_index > 0 and (write_concern or "").strip().lower() != "majority": + # Causal reads (after_index > 0) need a majority READ concern: the + # server refuses the pair otherwise, with "readConcern=LOCAL is + # incompatible with afterClusterTime". The concern that matters is the + # read's, not the write's, because the fence is about which replicas + # may answer, not about how the referenced write was acknowledged. + # The isinstance guard keeps a non-string read_concern (e.g. an int) + # on the ValueError path here, instead of crashing on `.strip()` + # before the concern validators get their chance to reject it. + if ( + after_index is not None + and after_index > 0 + and (not isinstance(read_concern, str) or read_concern.strip().lower() != "majority") + ): raise ValueError( - "after_index > 0 requires write_concern='majority' — causal reads " - "depend on majority-committed writes. Pass write_concern='majority'." + "after_index > 0 requires read_concern='majority': a causal read has to be " + "answered by a majority-acknowledged replica, or the referenced index may " + "not be there yet. Pass read_concern='majority'." ) req = ExecuteCypherRequest( query=query, @@ -330,8 +1023,230 @@ async def cypher( if read_preference is not None: req.read_preference = _make_read_preference(read_preference) resp = await self._cypher_stub.ExecuteCypher(req, timeout=self._timeout) - columns = list(resp.columns) - return [{col: from_property_value(val) for col, val in zip(columns, row.values)} for row in resp.rows] + return _rows_to_dicts(resp) + + def _reclaim_cancelled_begin(self, begin: asyncio.Task[Any]) -> None: + """Collect the late reply of a cancelled begin and roll it back. + + Tracked in the same pending set the cancellation cleanups use, so + close() drains it before the channel goes away; every failure is + best-effort-suppressed, with the server's idle sweep as backstop. + """ + if self._closing: + return + + async def _reclaim() -> None: + with suppress(Exception): + # Bounded by the cleanup deadline, not the request timeout: + # close() drains this before releasing the transport, and a + # stalled begin must not hold shutdown for the whole 30 + # seconds. wait_for cancels the begin on expiry; whatever the + # server may have allocated then falls to its idle sweep. + resp = await asyncio.wait_for(begin, timeout=min(_CLEANUP_TIMEOUT_SECS, self._timeout)) + if resp.transaction_id != 0: + await AsyncTransaction(self, resp.transaction_id)._best_effort_rollback() + + pending = self._pending_cleanups + task = asyncio.get_running_loop().create_task(_reclaim()) + pending.add(task) + task.add_done_callback(pending.discard) + + async def begin_transaction(self) -> AsyncTransaction: + """Open an interactive transaction and return its handle. + + Prefer :meth:`transaction`, which cannot leave one open. Reach for this + when the commit point is decided somewhere the ``async with`` block + cannot follow. + + The caller owns the outcome: a transaction left neither committed nor + rolled back holds its buffered writes on the server until the idle + sweep collects it. + """ + from coordinode._proto.coordinode.v1.query.cypher_pb2 import ( # type: ignore[import] + BeginTransactionRequest, + ) + + # The begin RPC runs in its own task, shielded: cancellation landing + # after the server has ALLOCATED the transaction but before the reply + # reaches this frame would otherwise lose the only copy of the id — + # no handle, nothing for close() to drain, a pinned snapshot until + # the idle sweep. On cancellation the task keeps running detached and + # its late reply is handed straight to a rollback. + # Wrapped in a coroutine: a real grpc.aio stub returns a + # UnaryUnaryCall — an awaitable, NOT a coroutine object — and + # create_task() accepts only the latter. + async def _begin() -> Any: + return await self._cypher_stub.BeginTransaction(BeginTransactionRequest(), timeout=self._timeout) + + begin = asyncio.get_running_loop().create_task(_begin()) + try: + resp = await asyncio.shield(begin) + except asyncio.CancelledError: + if not begin.cancelled(): + self._reclaim_cancelled_begin(begin) + raise + if resp.transaction_id == 0: + # Zero is what ExecuteCypherRequest uses for "no transaction", so a + # zero handle would silently turn every statement of this + # transaction into its own auto-committed write. The protocol + # promises a non-zero id here; refuse a server that breaks it. + raise RuntimeError( + "server answered BeginTransaction with transaction_id=0, which the wire " + "reserves for auto-commit; refusing to run statements outside a transaction" + ) + return AsyncTransaction(self, resp.transaction_id) + + @asynccontextmanager + async def transaction(self) -> AsyncIterator[AsyncTransaction]: + """Run a block of statements as one transaction. + + Commits when the block finishes, rolls back when it raises:: + + async with client.transaction() as tx: + await tx.cypher("CREATE (:Person {name: $n})", {"n": "Alice"}) + await tx.cypher("CREATE (:Person {name: $n})", {"n": "Bob"}) + + Committing or rolling back inside the block is allowed; this then leaves + the finished transaction alone rather than committing it twice. + + An exception from the block propagates unchanged. A rollback that itself + fails on the way out is swallowed, since the failure being reported is + the one the caller needs and the server drops an unresolved transaction + on its own. + """ + tx = await self.begin_transaction() + try: + yield tx + except asyncio.CancelledError: + tx._settle_cancelled_scope() + raise + except BaseException as exc: + if tx.is_open and not isinstance(exc, Exception): + # KeyboardInterrupt / SystemExit: the exit must not hold for + # the full request deadline on a stalled server. The handle + # closes and the cleanup goes detached (bounded, drained at + # close) while the interrupt propagates — mirroring the sync + # context manager. + tx._state = "aborted" + tx._spawn_cleanup() + elif tx.is_open: + # An ordinary rollback failure must never replace the error + # that caused the rollback — but a CANCELLATION landing + # mid-rollback propagates (asyncio does not re-inject a + # swallowed one). Either way an interrupted or failed + # rollback leaves the cleanup unconfirmed: hand it to a + # detached retry before the scope is gone. + try: + await tx.rollback() + except asyncio.CancelledError: + if not tx._cleanup_confirmed: + tx._spawn_cleanup() + if _caller_is_being_cancelled(): + raise + except Exception: + if not tx._cleanup_confirmed: + tx._spawn_cleanup() + elif tx._state == "indeterminate" or (tx._state == "aborted" and not tx._cleanup_confirmed): + # Two ways to reach the same unfinished business. A lost + # commit reply may mean the commit never REACHED the server, + # leaving the transaction open there with the caller gone; a + # failed statement whose own cleanup also failed leaves it + # open for the same reason. Either way the bounded request + # frees it, and if there is nothing to free the server + # answers "unknown id" and nothing changes. The block's own + # exception, and an indeterminate verdict, stand untouched. + await tx._cleanup_preserving_outcome() + elif tx._state in _IN_FLIGHT_STATES: + # The block raised while an operation it started (in a + # background task) is still in flight. Raising over the + # block's own error would mask it, and the operation cannot + # be awaited or cancelled from here — so the straggler is + # marked to hand the transaction to cleanup when it + # completes, instead of returning the handle to "open" with + # nobody left to use it. An in-flight COMMIT is additionally + # contested with a detached rollback: a successful commit + # cannot be retracted afterwards, so the only honest shot at + # the rollback-on-exception contract is letting the server + # race decide which request wins. + tx._abandoned = True + if tx._state == "committing": + tx._spawn_cleanup() + raise + else: + # One loop turn before deciding anything: an operation the block + # QUEUED with create_task and never awaited has not run yet, so + # its state reservation has not happened and the handle still + # reads "open" — auto-committing here would report success while + # that write is silently dropped. The ready queue is FIFO, so a + # task created inside the block runs its first step (up to its + # own state reservation, which is synchronous) ahead of this + # continuation, and the guard below sees it. + # + # Guarded: this yield sits in the `else` suite, so a cancellation + # landing on it escapes the handlers above — an open transaction + # would walk away with neither rollback nor detached cleanup. + try: + await asyncio.sleep(0) + except asyncio.CancelledError: + tx._settle_cancelled_scope() + raise + if tx._state in _IN_FLIGHT_STATES: + # The block exited while an operation it started (in a + # background task) is still in flight: reporting a successful + # exit then would let that operation race an owner that has + # already returned — buffered writes and a pinned snapshot can + # outlive the block unnoticed. Surface the misuse instead. + # + # An operation that queued and then failed instantly is NOT + # caught here, and cannot be. By the time the exit looks, it + # has finished, and its handle reads exactly like one belonging + # to a block that awaited the failure and chose to ignore it: + # + # bg = asyncio.create_task(tx.cypher(q, bad_params)) # dropped + # try: await tx.cypher(q, bad_params) # handled + # except Exception: pass + # + # Both leave a failed statement and an open handle. Whether the + # result was ever retrieved lives inside the asyncio.Task and + # is not observable from the coroutine, and a task nobody + # awaits still runs to completion, so there is no signal to + # read. Treating a failed statement as misuse would reject the + # second case, which is legitimate. The guard covers what is + # unambiguous: work that has not finished. + in_flight = _IN_FLIGHT_STATES[tx._state] + # The raise below leaves the scope with the operation still + # running; the straggler hands the transaction to cleanup on + # completion, same as on an exceptional exit. + tx._abandoned = True + raise RuntimeError( + f"Transaction context exited while a {in_flight} on it is still " + "in flight. Await every operation started inside the block " + "before leaving it." + ) + if tx.is_open: + try: + await tx.commit() + except asyncio.CancelledError: + # Caught cancellation is not re-injected at the next + # await, so an inline bounded rollback here would delay + # its propagation by the whole cleanup deadline — the + # same reason cancellation of the block itself detaches + # its cleanup. Spawn, then let the cancellation go. + if tx._state == "indeterminate": + tx._spawn_cleanup() + raise + except BaseException: + if tx._state == "indeterminate": + # Same reasoning as above, for the automatic commit. + await tx._cleanup_preserving_outcome() + raise + elif tx._state == "indeterminate" or (tx._state == "aborted" and not tx._cleanup_confirmed): + # The exit is normal because the block CAUGHT the failure, but + # the transaction it left behind may still be held: a manual + # commit() whose reply was lost, or a failed statement whose + # own cleanup also failed. Same bounded request as the + # exception path, and the indeterminate verdict survives it. + await tx._cleanup_preserving_outcome() async def vector_search( self, @@ -875,6 +1790,77 @@ async def health(self) -> bool: # ── Sync client (wraps async) ───────────────────────────────────────────────── +class Transaction: + """Synchronous view of an :class:`AsyncTransaction`. + + Same semantics throughout, including the node affinity and the idle sweep + described there. Obtained from :meth:`CoordinodeClient.transaction` or + :meth:`CoordinodeClient.begin_transaction`. + """ + + def __init__(self, client: CoordinodeClient, inner: AsyncTransaction) -> None: + self._client = client + self._inner = inner + + def __repr__(self) -> str: + return f"Transaction(id={self._inner.transaction_id}, state={self._inner._state})" + + @property + def transaction_id(self) -> int: + """Server-side handle for this transaction. Non-zero while it exists.""" + return self._inner.transaction_id + + @property + def is_open(self) -> bool: + """True while the transaction can still take statements and be committed.""" + return self._inner.is_open + + def cypher( + self, + query: str, + params: dict[str, PyValue] | None = None, + ) -> list[dict[str, Any]]: + """Run one statement inside this transaction. See :meth:`AsyncTransaction.cypher`.""" + try: + return self._client._run(self._inner.cypher(query, params)) # type: ignore[no-any-return] + except BaseException as exc: + # An interruption raised INSIDE the stepping coroutine (Ctrl-C + # delivered mid-call) completes the task before _run() can + # cancel-and-drain it, so no async handler closes the handle. + # Mirror commit()'s conservative settlement: no commit was sent, + # so nothing can apply — the handle closes as aborted, with the + # cleanup unconfirmed so a later rollback() frees the server + # side. Only for REAL interruptions (non-Exception): a local + # failure such as an unsupported parameter type raises an + # ordinary Exception before any RPC and must leave the handle + # open, exactly as the async path does. An outcome the inner + # handler already decided is kept either way. + if not isinstance(exc, Exception) and self._inner._state in ("open", "executing"): + self._inner._state = "aborted" + self._inner._cleanup_confirmed = False + raise + + def commit(self) -> int: + """Apply every buffered write as one unit. See :meth:`AsyncTransaction.commit`.""" + try: + return self._client._run(self._inner.commit()) # type: ignore[no-any-return] + except BaseException: + # An interruption at the loop boundary (Ctrl-C, SystemExit) never + # reaches the async handlers, so without this the handle would + # read "open" (or stay parked mid-"committing") while the server + # may already have applied the writes — inviting the duplicate + # retry the indeterminate state exists to prevent. An outcome the + # inner handler already decided (aborted, indeterminate, + # committed) is kept. + if self._inner._state in ("open", "committing"): + self._inner._state = "indeterminate" + raise + + def rollback(self) -> None: + """Discard every buffered write. See :meth:`AsyncTransaction.rollback`.""" + self._client._run(self._inner.rollback()) + + class CoordinodeClient: """ Synchronous gRPC client for CoordiNode. @@ -921,7 +1907,26 @@ def _run(self, coro: Any) -> Any: if not self._connected: self._loop.run_until_complete(self._async.connect()) self._connected = True - return self._loop.run_until_complete(coro) + # Driven through an explicit task so an interruption at the loop + # boundary (Ctrl-C escapes run_until_complete while the task is still + # pending) can cancel and DRAIN it. Left pending, the task would + # silently resume inside the next _run() call, racing a statement the + # caller believes interrupted against whatever runs next; cancelling + # it also fires the async cancellation handlers, so a transaction + # statement closes its handle exactly as it does under async + # cancellation. + task = self._loop.create_task(coro) + try: + return self._loop.run_until_complete(task) + except BaseException: + if not task.done(): + task.cancel() + # Swallows the CancelledError (and a second interrupt in this + # short window): the exception the caller sees is the + # interruption that started this. + with suppress(BaseException): + self._loop.run_until_complete(task) + raise def cypher( self, @@ -947,6 +1952,82 @@ def cypher( ) ) + def begin_transaction(self) -> Transaction: + """Open an interactive transaction. See :meth:`AsyncCoordinodeClient.begin_transaction`.""" + return Transaction(self, self._run(self._async.begin_transaction())) + + @contextmanager + def transaction(self) -> Iterator[Transaction]: + """Run a block of statements as one transaction. + + Commits when the block finishes, rolls back when it raises:: + + with client.transaction() as tx: + tx.cypher("CREATE (:Person {name: $n})", {"n": "Alice"}) + tx.cypher("CREATE (:Person {name: $n})", {"n": "Bob"}) + + See :meth:`AsyncCoordinodeClient.transaction` for the details. + """ + tx = self.begin_transaction() + try: + yield tx + except BaseException as exc: + if tx.is_open: + if isinstance(exc, Exception): + # CancelledError included, mirroring the async context + # manager: the cleanup's failure must never replace the + # block's own exception. + with suppress(Exception, asyncio.CancelledError): + tx.rollback() + else: + # Ctrl-C / SystemExit must not hold this exit for the + # full request deadline on a stalled server. Close the + # handle; the trailing check below sends one BOUNDED + # best-effort request instead of the ordinary rollback. + tx._inner._state = "aborted" + tx._inner._cleanup_confirmed = False + elif tx._inner._state == "indeterminate": + # Mirrors the async context manager: the commit may never + # have reached the server, so the bounded best-effort request + # frees the transaction in that case without touching the + # indeterminate verdict. Ordinary cleanup failures stay + # suppressed; a FRESH Ctrl-C or SystemExit arriving + # mid-cleanup is the user's word and propagates. + with suppress(Exception, asyncio.CancelledError): + self._run(tx._inner._best_effort_rollback()) + # A rollback that itself failed, or an in-task interruption's + # conservative settlement, leaves the handle aborted with the + # cleanup unconfirmed: retry the bounded best-effort request + # before the exception propagates, or the server holds the + # transaction until its idle sweep. + if tx._inner._state == "aborted" and not tx._inner._cleanup_confirmed: + with suppress(Exception, asyncio.CancelledError): + self._run(tx._inner._best_effort_rollback()) + raise + else: + if tx.is_open: + try: + tx.commit() + except BaseException: + if tx._inner._state == "indeterminate": + # Ordinary cleanup failures stay suppressed (the + # commit's own error is the story); a FRESH Ctrl-C + # or SystemExit arriving mid-cleanup is the user's + # word and propagates instead. + with suppress(Exception, asyncio.CancelledError): + self._run(tx._inner._best_effort_rollback()) + raise + elif tx._inner._state == "indeterminate" or ( + tx._inner._state == "aborted" and not tx._inner._cleanup_confirmed + ): + # Mirrors the async context manager's normal-exit rule: a + # manual commit() that failed ambiguously, or a failed + # statement whose own cleanup also failed, still gets the + # bounded request when the block caught the error, with the + # indeterminate verdict preserved. + with suppress(Exception, asyncio.CancelledError): + self._run(tx._inner._best_effort_rollback()) + def vector_search( self, label: str, @@ -1088,6 +2169,22 @@ def health(self) -> bool: } +def _rows_to_dicts(resp: Any) -> list[dict[str, Any]]: + """Decode an ExecuteCypher response into one dict per row. + + Shared by the auto-commit path and the in-transaction one: both answer with + the same message, and a second copy of this loop is a second place for a + decoding fix to be forgotten. + + Pairs strictly. A row carrying more or fewer values than there are columns + is a wire-shape mismatch, and pairing loosely would hand the caller a dict + with a key quietly missing, indistinguishable from a property the node does + not have. Raising here names the real problem at the point it is visible. + """ + columns = list(resp.columns) + return [{col: from_property_value(val) for col, val in zip(columns, row.values, strict=True)} for row in resp.rows] + + def _normalize_consistency_key(value: Any, field: str, mapping: dict[str, str]) -> str: if not isinstance(value, str) or not value.strip(): raise ValueError(f"{field} must be a non-empty string; got {value!r}") diff --git a/demo/README.md b/demo/README.md index 2d5fd2d..a769ab4 100644 --- a/demo/README.md +++ b/demo/README.md @@ -10,7 +10,7 @@ Interactive notebooks for LlamaIndex, LangChain, and LangGraph integrations. | [![Open in Colab](https://colab.research.google.com/assets/colab-badge.svg)](https://colab.research.google.com/github/structured-world/coordinode-python/blob/main/demo/notebooks/01_llama_index_property_graph.ipynb) **LlamaIndex** | `CoordinodePropertyGraphStore`: upsert, triplets, structured query | nothing | | [![Open in Colab](https://colab.research.google.com/assets/colab-badge.svg)](https://colab.research.google.com/github/structured-world/coordinode-python/blob/main/demo/notebooks/02_langchain_graph_chain.ipynb) **LangChain** | `CoordinodeGraph`: add_graph_documents, schema, GraphCypherQAChain | nothing | | [![Open in Colab](https://colab.research.google.com/assets/colab-badge.svg)](https://colab.research.google.com/github/structured-world/coordinode-python/blob/main/demo/notebooks/03_langgraph_agent.ipynb) **LangGraph** | Agent with CoordiNode as graph memory (save/query/traverse) | nothing | -| [![Open in Colab](https://colab.research.google.com/assets/colab-badge.svg)](https://colab.research.google.com/github/structured-world/coordinode-python/blob/main/demo/notebooks/04_whats_new_in_0_5.ipynb) **What 0.5 Added** | Batch insert, `element_id`, schema revision, write/read concerns, time travel | a server (`COORDINODE_ADDR`) | +| [![Open in Colab](https://colab.research.google.com/assets/colab-badge.svg)](https://colab.research.google.com/github/structured-world/coordinode-python/blob/main/demo/notebooks/04_whats_new_in_0_5.ipynb) **What 0.5 Added** | Batch insert, `element_id`, schema revision, write/read concerns, time travel, transactions | a server (`COORDINODE_ADDR`) | > **Note:** **LangGraph** runs the engine in-process rather than against a > server, in a database file of its own that the graph outlives a kernel @@ -19,7 +19,7 @@ Interactive notebooks for LlamaIndex, LangChain, and LangGraph integrations. > alternative was filtering model-written Cypher by a session tag, which leaks > the first time the filter misses a case. `COORDINODE_AGENT_DB` moves that > file; deleting it starts the agent with no memory. -> The Docker Compose stack below pins the CoordiNode **server** image v0.5.5 by +> The Docker Compose stack below pins the CoordiNode **server** image v0.5.7 by > digest. Do not move it below that: 0.5.1 crashes its Raft core when the oplog > rolls a segment that already exists, and a single-node stack never regains > leadership afterwards. diff --git a/demo/docker-compose.yml b/demo/docker-compose.yml index 31763d5..93b59a8 100644 --- a/demo/docker-compose.yml +++ b/demo/docker-compose.yml @@ -4,8 +4,8 @@ services: # COORDINODE_IMAGE at a locally built tag to try a server build before it # is published. # Digest-pinned so a re-pushed tag cannot change the demo underneath - # you; this one is 0.5.5. Override COORDINODE_IMAGE to try another. - image: ${COORDINODE_IMAGE:-ghcr.io/structured-world/coordinode@sha256:8d3554be7680aa2cea7b6e773037aee97513865f4698b6df0ffc2daf845c42b8} + # you; this one is 0.5.7. Override COORDINODE_IMAGE to try another. + image: ${COORDINODE_IMAGE:-ghcr.io/structured-world/coordinode@sha256:75a6242beb4cea8ab6726c842898fafc03ca9f59c6f7e36e462934a4ca64874c} container_name: demo-coordinode ports: - "127.0.0.1:37080:7080" # gRPC (native API) — localhost-only diff --git a/demo/notebooks/04_whats_new_in_0_5.ipynb b/demo/notebooks/04_whats_new_in_0_5.ipynb index 90ce081..6a29d73 100644 --- a/demo/notebooks/04_whats_new_in_0_5.ipynb +++ b/demo/notebooks/04_whats_new_in_0_5.ipynb @@ -3,27 +3,7 @@ { "cell_type": "markdown", "metadata": {}, - "source": [ - "# What 0.5 Added\n", - "\n", - "[![Open in Colab](https://colab.research.google.com/assets/colab-badge.svg)](https://colab.research.google.com/github/structured-world/coordinode-python/blob/main/demo/notebooks/04_whats_new_in_0_5.ipynb)\n", - "\n", - "The client surface that arrived with CoordiNode 0.5, exercised end to end:\n", - "\n", - "| Feature | What it is for |\n", - "|---------|----------------|\n", - "| `create_nodes_batch` | Create many nodes in one atomic write instead of a loop of round trips |\n", - "| `element_id` | A stable identifier that survives restarts and replication |\n", - "| `schema_revision` | Tells you when a label's shape last changed, so caches know to refresh |\n", - "| `write_concern` | Chooses how durable an acknowledgement is, from fire-and-forget to majority |\n", - "| `read_concern` / `read_preference` | Chooses how fresh a read is, and which replica answers it |\n", - "| `at_timestamp` | Reads the database as it was at a point in time |\n", - "\n", - "> **Needs a server.** These are distribution and durability features, so they\n", - "> exist on the client that talks to a CoordiNode server. Set `COORDINODE_ADDR`\n", - "> before running. The embedded engine has no Raft and no replicas, so the\n", - "> cells below stop with an explanation rather than pretending.\n" - ], + "source": "# What 0.5 Added\n\n[![Open in Colab](https://colab.research.google.com/assets/colab-badge.svg)](https://colab.research.google.com/github/structured-world/coordinode-python/blob/main/demo/notebooks/04_whats_new_in_0_5.ipynb)\n\nThe client surface that arrived with CoordiNode 0.5, exercised end to end:\n\n| Feature | What it is for |\n|---------|----------------|\n| `create_nodes_batch` | Create many nodes in one atomic write instead of a loop of round trips |\n| `element_id` | A stable identifier that survives restarts and replication |\n| `schema_revision` | Tells you when a label's shape last changed, so caches know to refresh |\n| `write_concern` | Chooses how durable an acknowledgement is, from fire-and-forget to majority |\n| `read_concern` / `read_preference` | Chooses how fresh a read is, and which replica answers it |\n| `at_timestamp` | Reads the database as it was at a point in time |\n| `transaction()` | Commits several statements as one unit, or rolls back all of them |\n\n> **Needs a server, v0.5.5 or newer** (the release this client is tested\n> against). These are distribution and durability features, so they exist on\n> the client that talks to a CoordiNode server. Set `COORDINODE_ADDR` before\n> running. The embedded engine has no Raft and no replicas, so the cells below\n> stop with an explanation rather than pretending.\n>\n> The version floor is not covered by the connect cell's health check, which\n> exercises a different service: a server without the transaction RPCs passes\n> it and then refuses them.\n", "id": "cell-00-ccfb99" }, { @@ -39,7 +19,7 @@ "execution_count": null, "metadata": {}, "outputs": [], - "source": "import importlib.util, inspect, os, shutil, subprocess, sys\n\n# (distribution on PyPI, module to import it by).\npkgs = [\n (\"coordinode\", \"coordinode\"),\n (\"nest_asyncio\", \"nest_asyncio\"),\n]\n\n# Install only what is missing. A checkout mounted into the image, or an\n# editable install, already provides these; pulling them from PyPI there\n# would shadow the very code the notebook is meant to exercise.\n#\n# find_spec raises rather than returning None when a dotted name's parent\n# package is absent, which is exactly the fresh environment this cell exists\n# for: unguarded, it would abort before installing anything.\ndef _missing(module: str) -> bool:\n try:\n return importlib.util.find_spec(module) is None\n except ModuleNotFoundError:\n return True\n\n\ndef _install(dists: list[str], upgrade: bool = False) -> None:\n \"\"\"Install into this interpreter with whatever installer it has.\n\n Colab ships pip. A uv-managed venv deliberately does not, and there\n `python -m pip` dies with \"No module named pip\" before the notebook\n reaches its first query, so fall back to uv targeting this same\n interpreter rather than uv's own default environment.\n\n The finder caches a directory listing taken before the install, so a\n package that has just appeared on disk is invisible until the caches are\n dropped. Doing it here means every caller sees what it installed.\n \"\"\"\n flags = [\"-q\", \"-U\"] if upgrade else [\"-q\"]\n if not _missing(\"pip\"):\n cmd = [sys.executable, \"-m\", \"pip\", \"install\", *flags, *dists]\n elif shutil.which(\"uv\"):\n cmd = [\"uv\", \"pip\", \"install\", *flags, \"--python\", sys.executable, *dists]\n else:\n raise RuntimeError(\n f\"Neither pip nor uv is available to install: {', '.join(dists)}\"\n )\n subprocess.run(cmd, check=True, timeout=300)\n importlib.invalidate_caches()\n\n\ndef _has_0_5_surface() -> bool:\n \"\"\"Whether the installed `coordinode` has the API this notebook calls.\n\n Every cell below is about something 0.5 added, so \"the module imports\" is\n the wrong question: a 0.4 install answers it happily and then fails four\n cells later with an AttributeError about `create_nodes_batch`, which reads\n like a broken notebook rather than an old package. Two probes cover the\n whole set, since these all shipped together: the batch method, and the\n consistency arguments on `cypher`.\n\n Probing the surface rather than comparing `__version__` is deliberate. An\n editable install without hatch-vcs reports 0.0.0, so a version floor would\n reject exactly the checkout this notebook is meant to exercise.\n \"\"\"\n try:\n from coordinode import CoordinodeClient\n except ImportError:\n return False\n if not hasattr(CoordinodeClient, \"create_nodes_batch\"):\n return False\n params = inspect.signature(CoordinodeClient.cypher).parameters\n return {\"read_concern\", \"write_concern\", \"at_timestamp\"}.issubset(params)\n\n\nmissing = [dist for dist, mod in pkgs if _missing(mod)]\nif missing:\n _install(missing)\n\nif not _has_0_5_surface():\n print(\"Installed `coordinode` predates 0.5, upgrading it.\")\n _install([\"coordinode\"], upgrade=True)\n # The old module object stays in sys.modules and would keep answering\n # imports for the rest of the session, so drop it and re-probe.\n for name in [m for m in sys.modules if m == \"coordinode\" or m.startswith(\"coordinode.\")]:\n del sys.modules[name]\n if not _has_0_5_surface():\n raise RuntimeError(\n \"`coordinode` still lacks the 0.5 client surface after upgrading. \"\n \"If this environment mounts a checkout or uses an editable install, \"\n \"that source is older than 0.5: update it, or install the released \"\n \"package instead.\"\n )\n\nimport nest_asyncio\n\nnest_asyncio.apply()\n\nprint(\"Ready\")\n", + "source": "import importlib.util, inspect, os, shutil, subprocess, sys\n\n# (distribution on PyPI, module to import it by).\npkgs = [\n (\"coordinode\", \"coordinode\"),\n (\"nest_asyncio\", \"nest_asyncio\"),\n]\n\n# Install only what is missing. A checkout mounted into the image, or an\n# editable install, already provides these; pulling them from PyPI there\n# would shadow the very code the notebook is meant to exercise.\n#\n# find_spec raises rather than returning None when a dotted name's parent\n# package is absent, which is exactly the fresh environment this cell exists\n# for: unguarded, it would abort before installing anything.\ndef _missing(module: str) -> bool:\n try:\n return importlib.util.find_spec(module) is None\n except ModuleNotFoundError:\n return True\n\n\ndef _install(dists: list[str], upgrade: bool = False) -> None:\n \"\"\"Install into this interpreter with whatever installer it has.\n\n Colab ships pip. A uv-managed venv deliberately does not, and there\n `python -m pip` dies with \"No module named pip\" before the notebook\n reaches its first query, so fall back to uv targeting this same\n interpreter rather than uv's own default environment.\n\n The finder caches a directory listing taken before the install, so a\n package that has just appeared on disk is invisible until the caches are\n dropped. Doing it here means every caller sees what it installed.\n \"\"\"\n flags = [\"-q\", \"-U\"] if upgrade else [\"-q\"]\n if not _missing(\"pip\"):\n cmd = [sys.executable, \"-m\", \"pip\", \"install\", *flags, *dists]\n elif shutil.which(\"uv\"):\n cmd = [\"uv\", \"pip\", \"install\", *flags, \"--python\", sys.executable, *dists]\n else:\n raise RuntimeError(\n f\"Neither pip nor uv is available to install: {', '.join(dists)}\"\n )\n subprocess.run(cmd, check=True, timeout=300)\n importlib.invalidate_caches()\n\n\ndef _has_0_5_surface() -> bool:\n \"\"\"Whether the installed `coordinode` has the API this notebook calls.\n\n Every cell below is about something 0.5 added, so \"the module imports\" is\n the wrong question: a 0.4 install answers it happily and then fails four\n cells later with an AttributeError about `create_nodes_batch`, which reads\n like a broken notebook rather than an old package. Three probes cover the\n set: the batch method and the consistency arguments on `cypher`, which\n shipped together, and `transaction`, which arrived after them and so has\n to be asked about separately.\n\n Probing the surface rather than comparing `__version__` is deliberate. An\n editable install without hatch-vcs reports 0.0.0, so a version floor would\n reject exactly the checkout this notebook is meant to exercise.\n \"\"\"\n try:\n from coordinode import CoordinodeClient\n except ImportError:\n return False\n if not hasattr(CoordinodeClient, \"create_nodes_batch\"):\n return False\n if not hasattr(CoordinodeClient, \"transaction\"):\n return False\n params = inspect.signature(CoordinodeClient.cypher).parameters\n return {\"read_concern\", \"write_concern\", \"at_timestamp\"}.issubset(params)\n\n\nmissing = [dist for dist, mod in pkgs if _missing(mod)]\nif missing:\n _install(missing)\n\nif not _has_0_5_surface():\n print(\"Installed `coordinode` predates 0.5, upgrading it.\")\n _install([\"coordinode\"], upgrade=True)\n # The old module object stays in sys.modules and would keep answering\n # imports for the rest of the session, so drop it and re-probe.\n for name in [m for m in sys.modules if m == \"coordinode\" or m.startswith(\"coordinode.\")]:\n del sys.modules[name]\n if not _has_0_5_surface():\n raise RuntimeError(\n \"`coordinode` still lacks the 0.5 client surface after upgrading. \"\n \"If this environment mounts a checkout or uses an editable install, \"\n \"that source is older than 0.5: update it, or install the released \"\n \"package instead.\"\n )\n\nimport nest_asyncio\n\nnest_asyncio.apply()\n\nprint(\"Ready\")\n", "id": "cell-02-54b3dc" }, { @@ -343,6 +323,20 @@ ], "id": "cell-16-e958bf" }, + { + "cell_type": "markdown", + "id": "aa9bdfb0", + "source": "## Transactions: all of it, or none of it\n\n`client.cypher(...)` commits each statement on its own, which is fine until two\nof them only make sense together. `client.transaction()` holds them open as one\nunit: it commits when the block finishes and rolls back when the block raises,\nso a failure halfway through leaves the database as it was rather than carrying\nhalf an operation.\n\nThree things the cell below shows, each checked rather than just printed:\n\n- Both writes appear after the commit.\n- A write made before an exception is **not** there afterwards. This is the\n whole point: without a transaction that first `CREATE` would already have been\n applied on its own.\n- A write inside an open transaction is visible to that transaction and to\n nobody else until it commits.\n\nThree constraints worth carrying into your own code:\n\n- **The transaction lives on the node that opened it,** so every request of it\n has to reach that same server. Connect to a node's own address, or through a\n proxy configured for backend affinity. A single client is not by itself a\n guarantee: a layer-7 or per-request balancer can spread the calls, and a\n reconnection can move to another backend mid-transaction.\n- **An idle transaction is collected** after 30 seconds by default, so a\n transaction is a short unit of work, not a place to park state while a user\n thinks.\n- **A lost reply is not an abort.** If the connection drops while committing,\n the server may have applied everything or nothing. The transaction is marked\n indeterminate and says so on every later call, because a blind retry there\n can duplicate the writes.\n", + "metadata": {} + }, + { + "cell_type": "code", + "id": "e1825630", + "source": "if client:\n ledger = \"MATCH (n:Ledger {tag: $tag}) RETURN n.name AS name ORDER BY name\"\n\n # Start from a clean slate. DEMO_TAG lives for the whole kernel, so\n # rerunning this cell without rerunning the connect cell would add a second\n # credit/debit pair and the exact check below would then report a failure\n # for a transaction that committed perfectly.\n client.cypher(\"MATCH (n:Ledger {tag: $tag}) DETACH DELETE n\", params={\"tag\": DEMO_TAG})\n\n # 1. A clean block commits every statement in it.\n with client.transaction() as tx:\n tx.cypher(\"CREATE (:Ledger {tag: $tag, name: 'debit'})\", params={\"tag\": DEMO_TAG})\n tx.cypher(\"CREATE (:Ledger {tag: $tag, name: 'credit'})\", params={\"tag\": DEMO_TAG})\n committed = [r[\"name\"] for r in client.cypher(ledger, params={\"tag\": DEMO_TAG})]\n print(f\" after commit : {committed}\")\n\n # 2. An exception discards the write that had already succeeded.\n try:\n with client.transaction() as tx:\n tx.cypher(\"CREATE (:Ledger {tag: $tag, name: 'orphan'})\", params={\"tag\": DEMO_TAG})\n raise RuntimeError(\"failed halfway through\")\n except RuntimeError as exc:\n print(f\" rolled back on : {exc}\")\n after_rollback = [r[\"name\"] for r in client.cypher(ledger, params={\"tag\": DEMO_TAG})]\n print(f\" after rollback : {after_rollback}\")\n\n # 3. An open transaction sees its own uncommitted write; nothing else does.\n tx = client.begin_transaction()\n tx.cypher(\"CREATE (:Ledger {tag: $tag, name: 'pending'})\", params={\"tag\": DEMO_TAG})\n inside = [r[\"name\"] for r in tx.cypher(ledger, params={\"tag\": DEMO_TAG})]\n outside = [r[\"name\"] for r in client.cypher(ledger, params={\"tag\": DEMO_TAG})]\n tx.rollback()\n print(f\" inside the txn : {inside}\")\n print(f\" outside it : {outside}\")\n\n # Check every claim above. Printing alone would let a rollback that\n # silently kept its write, or a snapshot that leaked one, read as success.\n problems = []\n if committed != [\"credit\", \"debit\"]:\n problems.append(f\"both writes should have committed, got {committed}\")\n if \"orphan\" in after_rollback:\n problems.append(f\"the rolled-back write survived: {after_rollback}\")\n if \"pending\" not in inside:\n problems.append(\"a transaction must see its own uncommitted write\")\n if \"pending\" in outside:\n problems.append(f\"an uncommitted write leaked outside: {outside}\")\n if problems:\n raise RuntimeError(\"transactions behaved wrongly: \" + \"; \".join(problems))\n print(\" all of it, or none of it\")\n", + "metadata": {}, + "execution_count": null, + "outputs": [] + }, { "cell_type": "markdown", "metadata": {}, @@ -356,16 +350,7 @@ "execution_count": null, "metadata": {}, "outputs": [], - "source": [ - "if client:\n", - " for label in (\"Engineer\", \"Sample\", \"Era\"):\n", - " client.cypher(\n", - " f\"MATCH (n:{label} {{tag: $tag}}) DETACH DELETE n\",\n", - " params={\"tag\": DEMO_TAG},\n", - " )\n", - " client.close()\n", - " print(f\"Removed everything tagged {DEMO_TAG}\")\n" - ], + "source": "if client:\n for label in (\"Engineer\", \"Sample\", \"Era\", \"Ledger\"):\n client.cypher(\n f\"MATCH (n:{label} {{tag: $tag}}) DETACH DELETE n\",\n params={\"tag\": DEMO_TAG},\n )\n client.close()\n print(f\"Removed everything tagged {DEMO_TAG}\")\n", "id": "cell-18-d9e318" } ], diff --git a/docker-compose.yml b/docker-compose.yml index cdc38d9..a4de6d4 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -7,10 +7,10 @@ version: "3.9" services: coordinode: - # The same 0.5.5 digest the demo stack and the integration job run, so the + # The same 0.5.7 digest the demo stack and the integration job run, so the # quick start does not hand a newcomer an older server than everything else # here is tested against. Pinned by digest because a tag can be re-pushed. - image: ghcr.io/structured-world/coordinode@sha256:8d3554be7680aa2cea7b6e773037aee97513865f4698b6df0ffc2daf845c42b8 + image: ghcr.io/structured-world/coordinode@sha256:75a6242beb4cea8ab6726c842898fafc03ca9f59c6f7e36e462934a4ca64874c # Named for this repository rather than plain `coordinode`: the server # repository runs its own compose stack that claims that name, and a bare # name is global to the daemon. diff --git a/proto b/proto index 2538765..b1bf1d0 160000 --- a/proto +++ b/proto @@ -1 +1 @@ -Subproject commit 2538765ca2fea7133693eb42528d884741d812c1 +Subproject commit b1bf1d0f1b4b811b4cfe1789a93ff8aa7a54c939 diff --git a/tests/integration/test_sdk.py b/tests/integration/test_sdk.py index 3cf00df..7760bcf 100644 --- a/tests/integration/test_sdk.py +++ b/tests/integration/test_sdk.py @@ -728,14 +728,138 @@ def test_cypher_rejects_invalid_consistency_values(client): client.cypher("RETURN 1", read_preference="leader") with pytest.raises(ValueError, match="after_index must be a non-negative integer"): client.cypher("RETURN 1", after_index=-1) - # Causal reads (after_index > 0) require write_concern='majority'. - with pytest.raises(ValueError, match="after_index > 0 requires write_concern='majority'"): + # Causal reads (after_index > 0) require read_concern='majority'. This + # asserted write_concern until the guard was corrected: the server's own + # refusal names the read concern, and a majority write concern alone was + # accepted here and then rejected by the server. + with pytest.raises(ValueError, match="after_index > 0 requires read_concern='majority'"): client.cypher("RETURN 1", after_index=42) - with pytest.raises(ValueError, match="after_index > 0 requires write_concern='majority'"): - client.cypher("RETURN 1", after_index=42, write_concern="w1") + with pytest.raises(ValueError, match="after_index > 0 requires read_concern='majority'"): + client.cypher("RETURN 1", after_index=42, read_concern="local") + with pytest.raises(ValueError, match="after_index > 0 requires read_concern='majority'"): + client.cypher("RETURN 1", after_index=42, write_concern="majority") # Type validation runs before the causal-read check so bools/strings # surface the non-negative-integer error rather than a misleading one. with pytest.raises(ValueError, match="after_index must be a non-negative integer"): client.cypher("RETURN 1", after_index=True) with pytest.raises(ValueError, match="after_index must be a non-negative integer"): client.cypher("RETURN 1", after_index="7") # type: ignore[arg-type] + + +# ── Interactive transactions ────────────────────────────────────────────────── + + +def test_transaction_commits_every_statement(client): + """Two writes in one transaction land together.""" + tag = uid() + try: + with client.transaction() as tx: + tx.cypher("CREATE (:TxDemo {tag: $tag, name: 'Alice'})", params={"tag": tag}) + tx.cypher("CREATE (:TxDemo {tag: $tag, name: 'Bob'})", params={"tag": tag}) + rows = client.cypher( + "MATCH (n:TxDemo {tag: $tag}) RETURN n.name AS name ORDER BY name", + params={"tag": tag}, + ) + assert [r["name"] for r in rows] == ["Alice", "Bob"] + finally: + client.cypher("MATCH (n:TxDemo {tag: $tag}) DELETE n", params={"tag": tag}) + + +def test_rollback_leaves_no_partial_write(client): + """The write made before the failure is not in the database afterwards. + + This is the whole point of the feature: without it the first CREATE would + have been applied on its own and the graph would carry half of an operation. + """ + tag = uid() + try: + with pytest.raises(ZeroDivisionError): + with client.transaction() as tx: + tx.cypher("CREATE (:TxDemo {tag: $tag, name: 'Alice'})", params={"tag": tag}) + 1 / 0 + rows = client.cypher("MATCH (n:TxDemo {tag: $tag}) RETURN count(n) AS c", params={"tag": tag}) + assert rows[0]["c"] == 0 + finally: + client.cypher("MATCH (n:TxDemo {tag: $tag}) DELETE n", params={"tag": tag}) + + +def test_explicit_rollback_discards_the_write(client): + tag = uid() + try: + tx = client.begin_transaction() + tx.cypher("CREATE (:TxDemo {tag: $tag, name: 'Alice'})", params={"tag": tag}) + tx.rollback() + assert tx.is_open is False + rows = client.cypher("MATCH (n:TxDemo {tag: $tag}) RETURN count(n) AS c", params={"tag": tag}) + assert rows[0]["c"] == 0 + finally: + client.cypher("MATCH (n:TxDemo {tag: $tag}) DELETE n", params={"tag": tag}) + + +def test_uncommitted_write_is_invisible_outside_the_transaction(client): + """A buffered write is readable by the transaction and by nobody else.""" + tag = uid() + tx = client.begin_transaction() + try: + tx.cypher("CREATE (:TxDemo {tag: $tag, name: 'Alice'})", params={"tag": tag}) + inside = tx.cypher("MATCH (n:TxDemo {tag: $tag}) RETURN count(n) AS c", params={"tag": tag}) + assert inside[0]["c"] == 1, "the transaction has to see its own write" + outside = client.cypher("MATCH (n:TxDemo {tag: $tag}) RETURN count(n) AS c", params={"tag": tag}) + assert outside[0]["c"] == 0, "an uncommitted write must not be visible elsewhere" + finally: + if tx.is_open: + tx.rollback() + client.cypher("MATCH (n:TxDemo {tag: $tag}) DELETE n", params={"tag": tag}) + + +def test_statement_error_reports_itself_and_leaves_nothing(client): + """A bad statement fails with its own error, not with a cleanup error. + + The server discards the transaction on the spot, so the earlier write is + gone and the handle is closed. + """ + tag = uid() + try: + with pytest.raises(grpc.RpcError): + with client.transaction() as tx: + tx.cypher("CREATE (:TxDemo {tag: $tag, name: 'Alice'})", params={"tag": tag}) + # A parse failure, deliberately: an unknown function or a + # division by zero answers NULL here rather than failing, so + # neither would exercise the abort path. + tx.cypher("RETURN (") + rows = client.cypher("MATCH (n:TxDemo {tag: $tag}) RETURN count(n) AS c", params={"tag": tag}) + assert rows[0]["c"] == 0 + finally: + client.cypher("MATCH (n:TxDemo {tag: $tag}) DELETE n", params={"tag": tag}) + + +def test_commit_returns_an_index_a_read_can_be_fenced_on(client): + """Use the index for what the docstring promises, rather than type-checking it. + + `isinstance(applied_index, int)` cannot fail, since commit() returns + `int(...)`, and `> 0` passes for any non-zero field. The claim worth + testing is that the value fences a causal read: passing it as `after_index` + must return the write the commit carried. + """ + tag = uid() + try: + tx = client.begin_transaction() + tx.cypher("CREATE (:TxDemo {tag: $tag, name: 'Alice'})", params={"tag": tag}) + applied_index = tx.commit() + + rows = client.cypher( + "MATCH (n:TxDemo {tag: $tag}) RETURN n.name AS name", + params={"tag": tag}, + after_index=applied_index, + read_concern="majority", + ) + assert [r["name"] for r in rows] == ["Alice"] + finally: + client.cypher("MATCH (n:TxDemo {tag: $tag}) DELETE n", params={"tag": tag}) + + +def test_reuse_after_commit_raises(client): + tx = client.begin_transaction() + tx.commit() + with pytest.raises(RuntimeError, match="already committed"): + tx.cypher("RETURN 1") diff --git a/tests/unit/test_transactions.py b/tests/unit/test_transactions.py new file mode 100644 index 0000000..ac610b8 --- /dev/null +++ b/tests/unit/test_transactions.py @@ -0,0 +1,2808 @@ +"""Unit tests for interactive transactions. + +These drive the real generated proto messages through fake stubs, so a field +rename in the proto submodule fails here rather than reaching users as an +AttributeError mid-transaction. + +What is worth testing without a server is the state machine around the three +RPCs, because that is where the SDK makes decisions of its own: which call it +sends, which it declines to send, and which error a caller ends up seeing. The +server's own guarantees (snapshot isolation, conflict detection at commit) are +tested against a live server in tests/integration/test_sdk.py. +""" + +import asyncio + +import grpc +import pytest + +from coordinode._proto.coordinode.v1.common import types_pb2 +from coordinode._proto.coordinode.v1.query import cypher_pb2 +from coordinode.client import AsyncCoordinodeClient, CoordinodeClient + +# ── Fixtures ───────────────────────────────────────────────────────────────── + + +class _ServerRejected(grpc.RpcError): + """Stand-in for an ANSWERED gRPC failure: the server processed the request + and refused it, so it carries a definitive status code the way a real + rejection does. Transit losses are modelled by `_TransportError` below.""" + + def code(self): + return grpc.StatusCode.INVALID_ARGUMENT + + +def _execute_response(columns=(), rows=()): + return cypher_pb2.ExecuteCypherResponse( + columns=list(columns), + rows=[cypher_pb2.Row(values=[types_pb2.PropertyValue(string_value=v) for v in row]) for row in rows], + ) + + +def _stub(**methods): + """Build a fake CypherService stub whose named methods are AsyncMocks.""" + from unittest.mock import AsyncMock + + defaults = { + "BeginTransaction": AsyncMock(return_value=cypher_pb2.BeginTransactionResponse(transaction_id=42)), + "ExecuteCypher": AsyncMock(return_value=_execute_response()), + "CommitTransaction": AsyncMock(return_value=cypher_pb2.CommitTransactionResponse(applied_index=7)), + "RollbackTransaction": AsyncMock(return_value=cypher_pb2.RollbackTransactionResponse()), + } + defaults.update(methods) + return type("FakeCypherStub", (), defaults)() + + +def _async_client(**methods): + client = AsyncCoordinodeClient("localhost:0") + client._cypher_stub = _stub(**methods) + return client + + +def _sync_client(**methods): + """A sync client wired to a fake stub, with connect() skipped. + + connect() would replace the fake stubs with real ones built from a channel, + so the flag is set directly instead. + """ + client = CoordinodeClient("localhost:0") + client._async._cypher_stub = _stub(**methods) + client._connected = True + return client + + +# ── Context manager ────────────────────────────────────────────────────────── + + +class TestContextManager: + def test_commits_on_clean_exit(self): + async def _inner() -> None: + client = _async_client() + async with client.transaction() as tx: + await tx.cypher("CREATE (:Person {name: $n})", {"n": "Alice"}) + assert client._cypher_stub.CommitTransaction.await_count == 1 + assert client._cypher_stub.CommitTransaction.call_args.args[0].transaction_id == 42 + assert client._cypher_stub.RollbackTransaction.await_count == 0 + + asyncio.run(_inner()) + + def test_rolls_back_on_exception_and_reraises_it(self): + """The caller's exception is the one that propagates, not one from cleanup.""" + + async def _inner() -> None: + client = _async_client() + with pytest.raises(ZeroDivisionError): + async with client.transaction() as tx: + await tx.cypher("CREATE (:Person)") + 1 / 0 + assert client._cypher_stub.RollbackTransaction.await_count == 1 + assert client._cypher_stub.CommitTransaction.await_count == 0 + + asyncio.run(_inner()) + + def test_failed_rollback_does_not_mask_the_original_error(self): + """A rollback that fails on the way out must not replace the real failure. + + The server drops an unresolved transaction on its own, so the caller + loses nothing by not hearing about this. + """ + from unittest.mock import AsyncMock + + async def _inner() -> None: + client = _async_client(RollbackTransaction=AsyncMock(side_effect=_ServerRejected("gone"))) + with pytest.raises(ValueError, match="the real problem"): + async with client.transaction(): + raise ValueError("the real problem") + + asyncio.run(_inner()) + + def test_manual_commit_inside_the_block_is_not_repeated(self): + async def _inner() -> None: + client = _async_client() + async with client.transaction() as tx: + await tx.commit() + assert client._cypher_stub.CommitTransaction.await_count == 1 + + asyncio.run(_inner()) + + def test_manual_rollback_inside_the_block_is_not_followed_by_a_commit(self): + async def _inner() -> None: + client = _async_client() + async with client.transaction() as tx: + await tx.rollback() + assert client._cypher_stub.CommitTransaction.await_count == 0 + assert client._cypher_stub.RollbackTransaction.await_count == 1 + + asyncio.run(_inner()) + + +# ── Statements ─────────────────────────────────────────────────────────────── + + +class TestStatements: + def test_statement_carries_the_transaction_handle(self): + """Without the handle the server would auto-commit the statement.""" + + async def _inner() -> None: + client = _async_client() + async with client.transaction() as tx: + await tx.cypher("CREATE (:Person {name: $n})", {"n": "Alice"}) + sent = client._cypher_stub.ExecuteCypher.call_args.args[0] + assert sent.transaction_id == 42 + assert sent.query == "CREATE (:Person {name: $n})" + assert sent.parameters["n"].string_value == "Alice" + + asyncio.run(_inner()) + + def test_rows_are_decoded(self): + async def _inner() -> None: + from unittest.mock import AsyncMock + + client = _async_client( + ExecuteCypher=AsyncMock(return_value=_execute_response(["name"], [["Alice"], ["Bob"]])) + ) + async with client.transaction() as tx: + rows = await tx.cypher("MATCH (n:Person) RETURN n.name AS name") + assert rows == [{"name": "Alice"}, {"name": "Bob"}] + + asyncio.run(_inner()) + + def test_consistency_arguments_are_refused(self): + """The in-transaction path ignores them server-side, so accepting them would mislead. + + The snapshot is fixed at the begin and durability is decided once at the + commit, which leaves nothing for a per-statement read or write concern + to mean. + """ + + async def _inner() -> None: + client = _async_client() + async with client.transaction() as tx: + with pytest.raises(TypeError): + await tx.cypher("MATCH (n) RETURN n", read_concern="majority") + + asyncio.run(_inner()) + + +# ── A failed statement ends the transaction ────────────────────────────────── + + +class TestAbort: + """The server discards the buffered state on any statement error and consumes + the handle, so the SDK has to stop treating the transaction as usable.""" + + @staticmethod + def _client_whose_statement_fails(): + from unittest.mock import AsyncMock + + return _async_client(ExecuteCypher=AsyncMock(side_effect=_ServerRejected("bad query"))) + + def test_statement_error_propagates(self): + async def _inner() -> None: + client = self._client_whose_statement_fails() + with pytest.raises(_ServerRejected): + async with client.transaction() as tx: + await tx.cypher("RETURN nonsense(") + + asyncio.run(_inner()) + + def test_exactly_one_cleanup_is_sent_and_never_a_commit(self): + """This asserted that no rollback was sent, back when the statement path + classified failures. It does send one now, unconditionally, because + classifying was how an unprocessed statement leaked a transaction for + the idle timeout. What still must not happen is a second cleanup from + the context manager on the way out, or a commit.""" + + async def _inner() -> None: + client = self._client_whose_statement_fails() + with pytest.raises(_ServerRejected): + async with client.transaction() as tx: + await tx.cypher("RETURN nonsense(") + assert client._cypher_stub.RollbackTransaction.await_count == 1 + assert client._cypher_stub.CommitTransaction.await_count == 0 + + asyncio.run(_inner()) + + def test_commit_after_an_aborted_statement_explains_itself(self): + async def _inner() -> None: + client = self._client_whose_statement_fails() + tx = await client.begin_transaction() + with pytest.raises(_ServerRejected): + await tx.cypher("RETURN nonsense(") + with pytest.raises(RuntimeError, match="an earlier failure closed it"): + await tx.commit() + assert client._cypher_stub.CommitTransaction.await_count == 0 + + asyncio.run(_inner()) + + def test_rollback_after_an_aborted_statement_adds_no_second_call(self): + """The failing statement already sent the cleanup, so an explicit + rollback has nothing left to do and must not repeat the request.""" + + async def _inner() -> None: + client = self._client_whose_statement_fails() + tx = await client.begin_transaction() + with pytest.raises(_ServerRejected): + await tx.cypher("RETURN nonsense(") + assert client._cypher_stub.RollbackTransaction.await_count == 1 + await tx.rollback() + assert client._cypher_stub.RollbackTransaction.await_count == 1 + assert tx.is_open is False + + asyncio.run(_inner()) + + def test_a_rejected_commit_also_ends_the_transaction(self): + """A conflict is reported at the commit, which consumes the handle too.""" + from unittest.mock import AsyncMock + + async def _inner() -> None: + client = _async_client(CommitTransaction=AsyncMock(side_effect=_ServerRejected("conflict"))) + tx = await client.begin_transaction() + with pytest.raises(_ServerRejected): + await tx.commit() + assert tx.is_open is False + await tx.rollback() + assert client._cypher_stub.RollbackTransaction.await_count == 0 + + asyncio.run(_inner()) + + +# ── Reuse after the transaction is over ────────────────────────────────────── + + +class TestReuse: + def test_statement_after_commit_raises(self): + async def _inner() -> None: + client = _async_client() + tx = await client.begin_transaction() + await tx.commit() + with pytest.raises(RuntimeError, match="already committed"): + await tx.cypher("MATCH (n) RETURN n") + + asyncio.run(_inner()) + + def test_commit_twice_raises(self): + async def _inner() -> None: + client = _async_client() + tx = await client.begin_transaction() + await tx.commit() + with pytest.raises(RuntimeError, match="already committed"): + await tx.commit() + assert client._cypher_stub.CommitTransaction.await_count == 1 + + asyncio.run(_inner()) + + def test_statement_after_rollback_raises(self): + async def _inner() -> None: + client = _async_client() + tx = await client.begin_transaction() + await tx.rollback() + with pytest.raises(RuntimeError, match="already rolled back"): + await tx.cypher("MATCH (n) RETURN n") + + asyncio.run(_inner()) + + +# ── Explicit API ───────────────────────────────────────────────────────────── + + +class TestExplicitApi: + def test_begin_returns_the_server_handle(self): + async def _inner() -> None: + client = _async_client() + tx = await client.begin_transaction() + assert tx.transaction_id == 42 + assert tx.is_open is True + + asyncio.run(_inner()) + + def test_commit_returns_the_applied_index(self): + """Which a later causal read can pass as after_index.""" + + async def _inner() -> None: + client = _async_client() + tx = await client.begin_transaction() + assert await tx.commit() == 7 + + asyncio.run(_inner()) + + +# ── Sync wrapper ───────────────────────────────────────────────────────────── + + +class TestSyncClient: + def test_context_manager_commits(self): + client = _sync_client() + with client.transaction() as tx: + tx.cypher("CREATE (:Person {name: $n})", {"n": "Alice"}) + assert client._async._cypher_stub.CommitTransaction.await_count == 1 + assert client._async._cypher_stub.ExecuteCypher.call_args.args[0].transaction_id == 42 + + def test_context_manager_rolls_back_on_exception(self): + client = _sync_client() + with pytest.raises(ZeroDivisionError): + with client.transaction() as tx: + tx.cypher("CREATE (:Person)") + 1 / 0 + assert client._async._cypher_stub.RollbackTransaction.await_count == 1 + assert client._async._cypher_stub.CommitTransaction.await_count == 0 + + def test_explicit_commit_returns_the_applied_index(self): + client = _sync_client() + tx = client.begin_transaction() + assert tx.transaction_id == 42 + assert tx.commit() == 7 + assert tx.is_open is False + + def test_reuse_after_commit_raises(self): + client = _sync_client() + tx = client.begin_transaction() + tx.commit() + with pytest.raises(RuntimeError, match="already committed"): + tx.cypher("MATCH (n) RETURN n") + + +# -- Transport failures that prove nothing ------------------------------------ +# +# A gRPC error is only sometimes an answer. DEADLINE_EXCEEDED, UNAVAILABLE, +# CANCELLED and UNKNOWN mean the request or its reply was lost somewhere on the +# way, so the server may have processed the call or may never have seen it. +# Treating those like a server rejection produced two bugs: a lost statement +# left the server holding the transaction until the idle sweep, and a lost +# commit reply told the caller nothing was applied when it may all have been. + + +class _TransportError(grpc.RpcError): + """A gRPC failure with a status code, like the real client raises. + + `trailing` models the trailing metadata a SERVER-answered failure + carries; a failure generated inside the client (a receive limit, a lost + connection) has none. + """ + + def __init__(self, code, trailing=None): + self._code = code + self._trailing = trailing + + def code(self): + return self._code + + def trailing_metadata(self): + return self._trailing + + +class TestAmbiguousStatementFailure: + @staticmethod + def _client_with_lost_statement(): + from unittest.mock import AsyncMock + + return _async_client(ExecuteCypher=AsyncMock(side_effect=_TransportError(grpc.StatusCode.DEADLINE_EXCEEDED))) + + def test_sends_a_best_effort_rollback(self): + """The statement may never have arrived, leaving the transaction open on + the server with its buffered writes until the idle sweep. A rollback + frees it now; if the statement did arrive and abort it, the server + answers "unknown transaction id" and there was nothing to free.""" + + async def _inner() -> None: + client = self._client_with_lost_statement() + tx = await client.begin_transaction() + with pytest.raises(_TransportError): + await tx.cypher("CREATE (:Person)") + assert client._cypher_stub.RollbackTransaction.await_count == 1 + assert tx.is_open is False + + asyncio.run(_inner()) + + def test_manual_rollback_after_it_is_a_no_op(self): + """The cleanup already happened; a second RollbackTransaction would only + collect an "unknown transaction id" answer.""" + + async def _inner() -> None: + client = self._client_with_lost_statement() + tx = await client.begin_transaction() + with pytest.raises(_TransportError): + await tx.cypher("CREATE (:Person)") + await tx.rollback() + assert client._cypher_stub.RollbackTransaction.await_count == 1 + + asyncio.run(_inner()) + + def test_a_failed_cleanup_does_not_mask_the_statement_error(self): + from unittest.mock import AsyncMock + + async def _inner() -> None: + client = _async_client( + ExecuteCypher=AsyncMock(side_effect=_TransportError(grpc.StatusCode.UNAVAILABLE)), + RollbackTransaction=AsyncMock(side_effect=_TransportError(grpc.StatusCode.UNAVAILABLE)), + ) + tx = await client.begin_transaction() + with pytest.raises(_TransportError): + await tx.cypher("CREATE (:Person)") + + asyncio.run(_inner()) + + def test_an_answered_rejection_still_reaches_the_caller_unchanged(self): + """INVALID_ARGUMENT is the server speaking: it processed the statement, + discarded the transaction and consumed the handle, so the cleanup this + path now sends is answered "unknown transaction id" and swallowed. + + This asserted no rollback was sent, back when statement failures were + classified by code. The classification is gone because it decided, + wrongly, for codes the server never acted on. What matters here is what + the caller sees: their own error, and a closed handle.""" + from unittest.mock import AsyncMock + + async def _inner() -> None: + client = _async_client( + ExecuteCypher=AsyncMock(side_effect=_TransportError(grpc.StatusCode.INVALID_ARGUMENT)), + RollbackTransaction=AsyncMock(side_effect=_TransportError(grpc.StatusCode.NOT_FOUND)), + ) + tx = await client.begin_transaction() + with pytest.raises(_TransportError) as caught: + await tx.cypher("RETURN (") + assert caught.value.code() == grpc.StatusCode.INVALID_ARGUMENT + assert tx.is_open is False + + asyncio.run(_inner()) + + +class TestIndeterminateCommit: + @staticmethod + def _client_with_lost_commit_reply(): + from unittest.mock import AsyncMock + + return _async_client( + CommitTransaction=AsyncMock(side_effect=_TransportError(grpc.StatusCode.DEADLINE_EXCEEDED)) + ) + + def test_outcome_is_recorded_as_unknown_not_aborted(self): + """The server may have applied every buffered write before the reply + was lost, or never seen the commit. Claiming an abort would invite a + retry that duplicates the writes.""" + + async def _inner() -> None: + client = self._client_with_lost_commit_reply() + tx = await client.begin_transaction() + with pytest.raises(_TransportError): + await tx.commit() + assert tx.is_open is False + with pytest.raises(RuntimeError, match="outcome is unknown"): + await tx.cypher("RETURN 1") + with pytest.raises(RuntimeError, match="outcome is unknown"): + await tx.commit() + + asyncio.run(_inner()) + + def test_rollback_attempts_cleanup_but_refuses_to_promise_a_discard(self): + """If the commit never arrived, the rollback frees the transaction; if + it was applied, nothing can un-apply it. So the request is sent and the + call still raises, because "nothing reached the database" cannot be + promised either way.""" + + async def _inner() -> None: + client = self._client_with_lost_commit_reply() + tx = await client.begin_transaction() + with pytest.raises(_TransportError): + await tx.commit() + with pytest.raises(RuntimeError, match="may already be applied"): + await tx.rollback() + assert client._cypher_stub.RollbackTransaction.await_count == 1 + + asyncio.run(_inner()) + + def test_an_answered_commit_rejection_is_still_a_plain_abort(self): + """A conflict is a real answer: the server consumed the handle and + applied nothing, so no indeterminacy is involved.""" + from unittest.mock import AsyncMock + + async def _inner() -> None: + client = _async_client(CommitTransaction=AsyncMock(side_effect=_TransportError(grpc.StatusCode.ABORTED))) + tx = await client.begin_transaction() + with pytest.raises(_TransportError): + await tx.commit() + with pytest.raises(RuntimeError, match="an earlier failure closed it"): + await tx.cypher("RETURN 1") + await tx.rollback() + assert client._cypher_stub.RollbackTransaction.await_count == 0 + + asyncio.run(_inner()) + + def test_an_error_that_cannot_report_a_code_is_read_as_ambiguous(self): + """The two misreadings are not symmetric: a needless cleanup rollback + costs one RPC answered "unknown transaction id", while a wrongly + claimed abort invites a retry that duplicates every write. So an error + proving nothing gets the careful reading, not the convenient one.""" + from unittest.mock import AsyncMock + + class _Codeless(grpc.RpcError): + pass + + async def _inner() -> None: + client = _async_client(CommitTransaction=AsyncMock(side_effect=_Codeless())) + tx = await client.begin_transaction() + with pytest.raises(_Codeless): + await tx.commit() + with pytest.raises(RuntimeError, match="outcome is unknown"): + await tx.commit() + + asyncio.run(_inner()) + + +class TestBeginValidation: + def test_a_zero_handle_is_refused(self): + """The wire defines zero on ExecuteCypherRequest as auto-commit, so a + zero handle would silently turn every statement of this transaction + into its own committed write. The protocol promises begin answers a + non-zero id; a server that breaks that promise gets refused, not + obeyed.""" + from unittest.mock import AsyncMock + + async def _inner() -> None: + client = _async_client( + BeginTransaction=AsyncMock(return_value=cypher_pb2.BeginTransactionResponse(transaction_id=0)) + ) + with pytest.raises(RuntimeError, match="transaction_id=0"): + await client.begin_transaction() + + asyncio.run(_inner()) + + +class TestRowDecoding: + """The shared decoder for both the auto-commit and the in-transaction path.""" + + def test_a_short_row_is_an_error_not_a_missing_key(self): + """Silently dropping a column hands the caller a dict whose missing key + looks like an absent property. A wire-shape mismatch is a decoding + failure and should say so at the decode point.""" + from unittest.mock import AsyncMock + + short_row = cypher_pb2.ExecuteCypherResponse( + columns=["a", "b"], + rows=[cypher_pb2.Row(values=[types_pb2.PropertyValue(string_value="only-one")])], + ) + + async def _inner() -> None: + client = _async_client(ExecuteCypher=AsyncMock(return_value=short_row)) + with pytest.raises(ValueError): + await client.cypher("MATCH (n) RETURN n.a AS a, n.b AS b") + + asyncio.run(_inner()) + + def test_a_long_row_is_an_error_too(self): + from unittest.mock import AsyncMock + + long_row = cypher_pb2.ExecuteCypherResponse( + columns=["a"], + rows=[ + cypher_pb2.Row( + values=[ + types_pb2.PropertyValue(string_value="one"), + types_pb2.PropertyValue(string_value="unexpected"), + ] + ) + ], + ) + + async def _inner() -> None: + client = _async_client(ExecuteCypher=AsyncMock(return_value=long_row)) + with pytest.raises(ValueError): + await client.cypher("MATCH (n) RETURN n.a AS a") + + asyncio.run(_inner()) + + +class TestCausalReadValidation: + """The client guard for `after_index` checked the wrong field entirely. + + The server refuses a causal read unless the READ concern is majority (its + message: "readConcern=LOCAL is incompatible with afterClusterTime"). The + guard demanded a majority WRITE concern instead, so it rejected valid calls + and waved through invalid ones. + """ + + def test_a_majority_read_concern_is_accepted(self): + """This is the call the server actually wants; the guard used to refuse it.""" + from unittest.mock import AsyncMock + + async def _inner() -> None: + client = _async_client(ExecuteCypher=AsyncMock(return_value=_execute_response())) + await client.cypher("MATCH (n) RETURN n", after_index=7, read_concern="majority") + sent = client._cypher_stub.ExecuteCypher.call_args.args[0] + assert sent.read_concern.after_index == 7 + + asyncio.run(_inner()) + + def test_a_majority_write_concern_alone_is_refused(self): + """The guard used to accept this and let the server reject it instead.""" + + async def _inner() -> None: + client = _async_client() + with pytest.raises(ValueError, match="read_concern='majority'"): + await client.cypher("MATCH (n) RETURN n", after_index=7, write_concern="majority") + + asyncio.run(_inner()) + + def test_no_concern_at_all_is_refused(self): + async def _inner() -> None: + client = _async_client() + with pytest.raises(ValueError, match="read_concern='majority'"): + await client.cypher("MATCH (n) RETURN n", after_index=7) + + asyncio.run(_inner()) + + def test_a_non_string_read_concern_is_a_value_error_not_a_crash(self): + """The guard runs before the concern validators, so a non-string used + to crash it with AttributeError from `.strip()` instead of the clear + rejection every other invalid consistency value gets.""" + + async def _inner() -> None: + client = _async_client() + with pytest.raises(ValueError): + await client.cypher("MATCH (n) RETURN n", after_index=7, read_concern=1) + + asyncio.run(_inner()) + + +# -- Failures that are not gRPC errors, and cleanup that must not be skipped --- + + +class TestCommitCancellation: + def test_a_cancelled_commit_is_indeterminate_not_open(self): + """`asyncio.CancelledError` is a BaseException, so `except grpc.RpcError` + never sees it. The RPC may still have reached the server and applied + everything, so leaving the transaction open invites exactly the retry + the indeterminate state exists to prevent.""" + from unittest.mock import AsyncMock + + async def _inner() -> None: + client = _async_client(CommitTransaction=AsyncMock(side_effect=asyncio.CancelledError())) + tx = await client.begin_transaction() + with pytest.raises(asyncio.CancelledError): + await tx.commit() + assert tx.is_open is False + with pytest.raises(RuntimeError, match="outcome is unknown"): + await tx.commit() + + asyncio.run(_inner()) + + def test_a_cancelled_statement_closes_the_transaction(self): + """No commit was sent, so nothing of this transaction can ever apply; + the handle is closed rather than left open for reuse.""" + from unittest.mock import AsyncMock + + async def _inner() -> None: + client = _async_client(ExecuteCypher=AsyncMock(side_effect=asyncio.CancelledError())) + tx = await client.begin_transaction() + with pytest.raises(asyncio.CancelledError): + await tx.cypher("CREATE (:Person)") + assert tx.is_open is False + + asyncio.run(_inner()) + + +class TestRollbackTransportFailure: + def test_a_lost_rollback_still_closes_the_transaction(self): + """The request may not have landed, so the server may still hold the + transaction until the idle sweep. What is certain is that no commit was + ever sent, so nothing can apply: the discard promise holds and the + handle must not stay usable — though it stays RETRIABLE for rollback, + so the closed state reads as aborted, not rolled back.""" + from unittest.mock import AsyncMock + + async def _inner() -> None: + client = _async_client( + RollbackTransaction=AsyncMock(side_effect=_TransportError(grpc.StatusCode.UNAVAILABLE)) + ) + tx = await client.begin_transaction() + with pytest.raises(_TransportError): + await tx.rollback() + assert tx.is_open is False + with pytest.raises(RuntimeError, match="earlier failure closed it"): + await tx.cypher("CREATE (:Person)") + + asyncio.run(_inner()) + + +class TestStatementCleanupIsUnconditional: + def test_a_client_side_size_failure_still_cleans_up(self): + """RESOURCE_EXHAUSTED is raised by the client when the response exceeds + its receive limit, after the server has executed the statement and kept + the transaction open. Classifying codes missed this one; cleanup for + statements is now unconditional, so the next unclassified code cannot + leak a transaction either.""" + from unittest.mock import AsyncMock + + async def _inner() -> None: + client = _async_client( + ExecuteCypher=AsyncMock(side_effect=_TransportError(grpc.StatusCode.RESOURCE_EXHAUSTED)) + ) + tx = await client.begin_transaction() + with pytest.raises(_TransportError): + await tx.cypher("MATCH (n) RETURN n") + assert client._cypher_stub.RollbackTransaction.await_count == 1 + + asyncio.run(_inner()) + + +class TestCancellationCleanup: + """Cancellation can arrive after the server has the statement, so the + transaction may still be alive there with its buffered writes and a pinned + snapshot. Nothing can free it afterwards: the handle is closed, so both + `rollback()` and the context manager decline to act.""" + + def test_a_really_cancelled_statement_still_sends_the_cleanup(self): + """Cancels a task mid-flight rather than raising CancelledError from a + mock, so the shielding is exercised the way the event loop does it.""" + + async def _inner() -> None: + from unittest.mock import AsyncMock + + in_flight = asyncio.Event() + + async def never_answers(req, timeout=None): + in_flight.set() + await asyncio.sleep(10) + + # Wrapped in AsyncMock: a bare function stored on the fake stub + # class would bind as a method and receive the stub as `req`. + client = _async_client(ExecuteCypher=AsyncMock(side_effect=never_answers)) + tx = await client.begin_transaction() + task = asyncio.create_task(tx.cypher("CREATE (:Person)")) + await in_flight.wait() + task.cancel() + with pytest.raises(asyncio.CancelledError): + await task + # The cleanup outlives the cancellation, so give the loop a turn. + await asyncio.sleep(0.05) + assert client._cypher_stub.RollbackTransaction.await_count == 1 + assert tx.is_open is False + + asyncio.run(_inner()) + + def test_a_cancelled_commit_sends_no_cleanup(self): + """The opposite case, and the reason this is not symmetric: after a + commit the writes may be applied, and a rollback cannot un-apply them. + Sending one could only discard a transaction the server still holds, + turning an unknown outcome into a silently discarded one.""" + from unittest.mock import AsyncMock + + async def _inner() -> None: + client = _async_client(CommitTransaction=AsyncMock(side_effect=asyncio.CancelledError())) + tx = await client.begin_transaction() + with pytest.raises(asyncio.CancelledError): + await tx.commit() + await asyncio.sleep(0.05) + assert client._cypher_stub.RollbackTransaction.await_count == 0 + + asyncio.run(_inner()) + + +class TestCommitFailureClassification: + """Which commit failures are definitive rejections and which leave the + outcome unknown. The dangerous misreading is one-directional: telling the + caller "nothing was applied" when the server may have applied everything + invites a retry that duplicates the writes.""" + + @staticmethod + def _commit_failing_with(err): + from unittest.mock import AsyncMock + + return _async_client(CommitTransaction=AsyncMock(side_effect=err)) + + def test_a_local_resource_exhausted_reply_failure_is_indeterminate(self): + """RESOURCE_EXHAUSTED can be generated INSIDE the client while + receiving an oversized reply, after the server already applied the + commit. Without the server's structured details in the trailing + metadata, the code alone proves nothing.""" + + async def _inner() -> None: + client = self._commit_failing_with(_TransportError(grpc.StatusCode.RESOURCE_EXHAUSTED)) + tx = await client.begin_transaction() + with pytest.raises(_TransportError): + await tx.commit() + with pytest.raises(RuntimeError, match="outcome is unknown"): + await tx.cypher("RETURN 1") + + asyncio.run(_inner()) + + def test_a_server_answered_resource_exhausted_is_a_plain_abort(self): + """The same code WITH the server's structured error details is an + answer (a transaction-too-large rejection): nothing was applied.""" + + async def _inner() -> None: + client = self._commit_failing_with( + _TransportError( + grpc.StatusCode.RESOURCE_EXHAUSTED, + trailing=[("grpc-status-details-bin", b"\x08\x08")], + ) + ) + tx = await client.begin_transaction() + with pytest.raises(_TransportError): + await tx.commit() + with pytest.raises(RuntimeError, match="an earlier failure closed it"): + await tx.cypher("RETURN 1") + + asyncio.run(_inner()) + + def test_a_bare_internal_error_is_indeterminate(self): + """INTERNAL without details can come from either side of the wire and + says nothing about whether the proposal was applied.""" + + async def _inner() -> None: + client = self._commit_failing_with(_TransportError(grpc.StatusCode.INTERNAL)) + tx = await client.begin_transaction() + with pytest.raises(_TransportError): + await tx.commit() + with pytest.raises(RuntimeError, match="outcome is unknown"): + await tx.cypher("RETURN 1") + + asyncio.run(_inner()) + + +class TestCleanupDrainOnClose: + """The detached cancellation cleanup must not race client shutdown: the + channel closing first would strand the transaction on the server.""" + + def test_close_awaits_spawned_cleanup_before_returning(self): + async def _inner() -> None: + from unittest.mock import AsyncMock + + in_flight = asyncio.Event() + rollback_done = asyncio.Event() + + async def never_answers(req, timeout=None): + in_flight.set() + await asyncio.sleep(10) + + async def slow_rollback(req, timeout=None): + # Slower than a single event-loop turn: merely yielding once + # is not enough for this to finish, so the assertion below + # holds only if close() genuinely awaits the cleanup task. + await asyncio.sleep(0.05) + rollback_done.set() + + client = _async_client( + ExecuteCypher=AsyncMock(side_effect=never_answers), + RollbackTransaction=AsyncMock(side_effect=slow_rollback), + ) + tx = await client.begin_transaction() + task = asyncio.create_task(tx.cypher("CREATE (:Person)")) + await in_flight.wait() + task.cancel() + with pytest.raises(asyncio.CancelledError): + await task + await client.close() + assert rollback_done.is_set(), "close() returned before the cleanup finished" + + asyncio.run(_inner()) + + +class TestCleanupDeadline: + """Best-effort cleanup must not double the caller's worst-case latency: + a statement that already burned the full RPC deadline would otherwise be + followed by a rollback burning another one, for a result nobody reads.""" + + def test_cleanup_rollback_uses_a_short_deadline_not_the_client_timeout(self): + async def _inner() -> None: + from unittest.mock import AsyncMock + + client = _async_client( + ExecuteCypher=AsyncMock(side_effect=_TransportError(grpc.StatusCode.DEADLINE_EXCEEDED)) + ) + client._timeout = 30.0 + tx = await client.begin_transaction() + with pytest.raises(_TransportError): + await tx.cypher("RETURN 1") + assert client._cypher_stub.RollbackTransaction.await_count == 1 + used = client._cypher_stub.RollbackTransaction.call_args.kwargs["timeout"] + assert used < 30.0, f"cleanup must use a short deadline, got {used}" + + asyncio.run(_inner()) + + def test_cleanup_deadline_is_capped_by_a_shorter_client_timeout(self): + """A caller who configured 100ms requests must not find a failed + statement holding the line for a multi-second cleanup.""" + + async def _inner() -> None: + from unittest.mock import AsyncMock + + client = _async_client( + ExecuteCypher=AsyncMock(side_effect=_TransportError(grpc.StatusCode.DEADLINE_EXCEEDED)) + ) + client._timeout = 0.1 + tx = await client.begin_transaction() + with pytest.raises(_TransportError): + await tx.cypher("RETURN 1") + used = client._cypher_stub.RollbackTransaction.call_args.kwargs["timeout"] + assert used == 0.1, f"cleanup must not exceed the client timeout, got {used}" + + asyncio.run(_inner()) + + +class TestRollbackCancellationDoesNotMaskTheBlockError: + """A rollback cancelled on the way out of the context manager must not + replace the exception that caused the rollback: CancelledError is a + BaseException, so a plain `suppress(Exception)` let it through.""" + + def test_async_context_preserves_the_block_exception(self): + from unittest.mock import AsyncMock + + async def _inner() -> None: + client = _async_client(RollbackTransaction=AsyncMock(side_effect=asyncio.CancelledError())) + with pytest.raises(ValueError, match="the real problem"): + async with client.transaction(): + raise ValueError("the real problem") + + asyncio.run(_inner()) + + def test_sync_context_preserves_the_block_exception(self): + from unittest.mock import AsyncMock + + client = _sync_client(RollbackTransaction=AsyncMock(side_effect=asyncio.CancelledError())) + with pytest.raises(ValueError, match="the real problem"): + with client.transaction(): + raise ValueError("the real problem") + + +class TestSyncCommitInterruption: + """Ctrl-C during a synchronous commit crosses the loop boundary as + KeyboardInterrupt, which the async handlers never see. The server may + already have applied the writes, so the handle must come back + indeterminate, not open: an "open" handle invites the duplicate retry.""" + + def test_an_interrupted_sync_commit_is_indeterminate(self): + from unittest.mock import AsyncMock + + client = _sync_client(CommitTransaction=AsyncMock(side_effect=KeyboardInterrupt())) + tx = client.begin_transaction() + with pytest.raises(KeyboardInterrupt): + tx.commit() + assert tx.is_open is False + with pytest.raises(RuntimeError, match="outcome is unknown"): + tx.cypher("RETURN 1") + + +class TestCancellationDuringInlineCleanup: + """A statement failure runs its cleanup inline; a cancellation arriving + DURING that cleanup must not lose it (the handle is already closed, so + nothing later would retry), it must detach it.""" + + def test_cleanup_cancelled_mid_flight_is_respawned_detached(self): + from unittest.mock import AsyncMock + + async def _inner() -> None: + cleanup_in_flight = asyncio.Event() + cleanup_done = asyncio.Event() + calls = {"n": 0} + + async def rollback(req, timeout=None): + calls["n"] += 1 + if calls["n"] == 1: + cleanup_in_flight.set() + await asyncio.sleep(10) + cleanup_done.set() + + client = _async_client( + ExecuteCypher=AsyncMock(side_effect=_TransportError(grpc.StatusCode.INVALID_ARGUMENT)), + RollbackTransaction=AsyncMock(side_effect=rollback), + ) + tx = await client.begin_transaction() + task = asyncio.create_task(tx.cypher("RETURN (")) + await cleanup_in_flight.wait() + task.cancel() + with pytest.raises(asyncio.CancelledError): + await task + # The detached retry must be drained by close(), like every + # cancellation-spawned cleanup. + await client.close() + assert cleanup_done.is_set(), "the cleanup was lost to the cancellation" + + asyncio.run(_inner()) + + +class TestLateCleanupDrain: + """close() must not settle for a one-time snapshot of the cleanup set: a + statement cancelled WHILE the drain awaits an earlier cleanup adds its + task after the snapshot, and a single gather would strand it against a + closed channel.""" + + def test_close_drains_cleanups_spawned_during_the_drain(self): + from unittest.mock import AsyncMock + + async def _inner() -> None: + stmt2_in_flight = asyncio.Event() + late_done = asyncio.Event() + calls = {"n": 0} + + async def rollback(req, timeout=None): + # Every cleanup is slower than one loop turn, so the late one + # can only complete if close() genuinely waits for it too (a + # one-time snapshot would return while it is still running). + calls["n"] += 1 + mine = calls["n"] + await asyncio.sleep(0.1) + if mine == 2: + late_done.set() + + async def hang(req, timeout=None): + stmt2_in_flight.set() + await asyncio.sleep(10) + + client = _async_client( + ExecuteCypher=AsyncMock(side_effect=hang), + RollbackTransaction=AsyncMock(side_effect=rollback), + ) + tx1 = await client.begin_transaction() + tx2 = await client.begin_transaction() + # First cancellation: its cleanup is the slow one close() drains. + t1 = asyncio.create_task(tx1.cypher("CREATE (:A)")) + await stmt2_in_flight.wait() + stmt2_in_flight.clear() + t1.cancel() + with pytest.raises(asyncio.CancelledError): + await t1 + # Second statement still in flight when close() starts. + t2 = asyncio.create_task(tx2.cypher("CREATE (:B)")) + await stmt2_in_flight.wait() + closer = asyncio.create_task(client.close()) + await asyncio.sleep(0.02) # close() is now inside the drain + t2.cancel() + with pytest.raises(asyncio.CancelledError): + await t2 + await closer + assert late_done.is_set(), "a cleanup spawned during the drain was stranded" + + asyncio.run(_inner()) + + +class TestIndeterminateCommitCleanupInContext: + """An automatic commit whose reply is lost may never have REACHED the + server, leaving the transaction open there; the context-managed caller has + already left the owning scope, so the exit sends the same best-effort + rollback the explicit rollback() path uses, without touching the original + error or the indeterminate verdict.""" + + def test_async_context_sends_best_effort_cleanup_and_keeps_the_error(self): + from unittest.mock import AsyncMock + + async def _inner() -> None: + client = _async_client( + CommitTransaction=AsyncMock(side_effect=_TransportError(grpc.StatusCode.DEADLINE_EXCEEDED)) + ) + with pytest.raises(_TransportError): + async with client.transaction() as tx: + await tx.cypher("CREATE (:A)") + assert client._cypher_stub.RollbackTransaction.await_count == 1 + with pytest.raises(RuntimeError, match="outcome is unknown"): + await tx.cypher("RETURN 1") + + asyncio.run(_inner()) + + +class TestSyncInterruptionDrainsThePendingStatement: + """Ctrl-C reaches the private loop as KeyboardInterrupt from a callback, + leaving the statement's task pending. A later call on the same loop would + RESUME that statement and race it against the caller's cleanup, so the + sync boundary must cancel and drain it before propagating.""" + + def test_interrupted_statement_is_cancelled_not_left_pending(self): + from unittest.mock import AsyncMock + + in_flight = {"seen": False} + + async def hang(req, timeout=None): + in_flight["seen"] = True + await asyncio.sleep(10) + + client = _sync_client(ExecuteCypher=AsyncMock(side_effect=hang)) + tx = client.begin_transaction() + loop = client._loop + + def interrupt(): + raise KeyboardInterrupt + + loop.call_later(0.02, interrupt) + with pytest.raises(KeyboardInterrupt): + tx.cypher("CREATE (:A)") + assert in_flight["seen"] is True + # The statement's task was cancelled and drained: its cancellation + # handler closed the handle, and nothing is left to resume. + assert tx.is_open is False + assert not asyncio.all_tasks(loop), "a pending task survived the interruption" + + +class TestNoCleanupSpawnsAfterClose: + """Once close() has drained and released the channel, a late cancellation + must not spawn a cleanup that would only fail against the dead transport: + the server's idle sweep is the documented backstop then.""" + + def test_a_cancellation_after_close_spawns_nothing(self): + from unittest.mock import AsyncMock + + async def _inner() -> None: + in_flight = asyncio.Event() + + async def hang(req, timeout=None): + in_flight.set() + await asyncio.sleep(10) + + client = _async_client(ExecuteCypher=AsyncMock(side_effect=hang)) + tx = await client.begin_transaction() + task = asyncio.create_task(tx.cypher("CREATE (:A)")) + await in_flight.wait() + await client.close() + task.cancel() + with pytest.raises(asyncio.CancelledError): + await task + await asyncio.sleep(0.05) + assert client._cypher_stub.RollbackTransaction.await_count == 0, ( + "a cleanup spawned after close can only fail against the closed channel" + ) + + asyncio.run(_inner()) + + +class TestCancelledCloseDoesNotAbortCleanup: + """Cancelling the task that runs close() must not take the in-flight + cleanup down with it: the drain is shielded, so the rollback completes + detached while the cancellation propagates.""" + + def test_cleanup_survives_a_cancelled_close(self): + from unittest.mock import AsyncMock + + async def _inner() -> None: + stmt_in_flight = asyncio.Event() + cleanup_started = asyncio.Event() + cleanup_done = asyncio.Event() + + async def hang(req, timeout=None): + stmt_in_flight.set() + await asyncio.sleep(10) + + async def slow_rollback(req, timeout=None): + cleanup_started.set() + await asyncio.sleep(0.1) + cleanup_done.set() + + client = _async_client( + ExecuteCypher=AsyncMock(side_effect=hang), + RollbackTransaction=AsyncMock(side_effect=slow_rollback), + ) + tx = await client.begin_transaction() + stmt = asyncio.create_task(tx.cypher("CREATE (:A)")) + await stmt_in_flight.wait() + stmt.cancel() + with pytest.raises(asyncio.CancelledError): + await stmt + closer = asyncio.create_task(client.close()) + await cleanup_started.wait() + # Let the closer actually enter its drain (a cancel delivered + # before it starts would never reach the gather). + await asyncio.sleep(0.03) + closer.cancel() + with pytest.raises(asyncio.CancelledError): + await closer + await asyncio.sleep(0.15) + assert cleanup_done.is_set(), "cancelling close() killed the cleanup" + + asyncio.run(_inner()) + + +class TestConcurrentCommitSerialization: + """Two tasks committing the same handle must not race the state machine: + the second must be refused BEFORE it sends anything, or the loser's + "unknown transaction" rejection would overwrite `committed` with + `aborted` and invite a duplicate retry.""" + + def test_a_second_concurrent_commit_is_refused_not_raced(self): + from unittest.mock import AsyncMock + + async def _inner() -> None: + async def slow_commit(req, timeout=None): + await asyncio.sleep(0.05) + return cypher_pb2.CommitTransactionResponse(applied_index=7) + + client = _async_client(CommitTransaction=AsyncMock(side_effect=slow_commit)) + tx = await client.begin_transaction() + first = asyncio.create_task(tx.commit()) + await asyncio.sleep(0.01) # first commit is now awaiting the RPC + with pytest.raises(RuntimeError, match="in flight"): + await tx.commit() + assert await first == 7 + assert client._cypher_stub.CommitTransaction.await_count == 1 + # The handle records the real outcome, untouched by the refusal. + with pytest.raises(RuntimeError, match="already committed"): + await tx.cypher("RETURN 1") + + asyncio.run(_inner()) + + def test_a_statement_during_a_commit_is_refused(self): + from unittest.mock import AsyncMock + + async def _inner() -> None: + async def slow_commit(req, timeout=None): + await asyncio.sleep(0.05) + return cypher_pb2.CommitTransactionResponse(applied_index=7) + + client = _async_client(CommitTransaction=AsyncMock(side_effect=slow_commit)) + tx = await client.begin_transaction() + first = asyncio.create_task(tx.commit()) + await asyncio.sleep(0.01) + with pytest.raises(RuntimeError, match="in flight"): + await tx.cypher("CREATE (:Late)") + assert await first == 7 + assert client._cypher_stub.ExecuteCypher.await_count == 0 + + asyncio.run(_inner()) + + +class TestConcurrentStatementSerialization: + """The mirror image of the commit race: while a statement awaits its RPC + the handle must not accept a concurrent commit (the commit could land + without the statement's write, then the statement's failure would + overwrite `committed` with `aborted`) nor a second statement.""" + + def test_a_commit_during_a_statement_is_refused(self): + from unittest.mock import AsyncMock + + async def _inner() -> None: + started = asyncio.Event() + + async def slow_execute(req, timeout=None): + started.set() + await asyncio.sleep(0.05) + return _execute_response() + + client = _async_client(ExecuteCypher=AsyncMock(side_effect=slow_execute)) + tx = await client.begin_transaction() + stmt = asyncio.create_task(tx.cypher("CREATE (:A)")) + await started.wait() + with pytest.raises(RuntimeError, match="in flight"): + await tx.commit() + assert client._cypher_stub.CommitTransaction.await_count == 0 + await stmt + # The handle is usable again once the statement resolved. + assert await tx.commit() == 7 + + asyncio.run(_inner()) + + def test_a_second_concurrent_statement_is_refused(self): + from unittest.mock import AsyncMock + + async def _inner() -> None: + started = asyncio.Event() + + async def slow_execute(req, timeout=None): + started.set() + await asyncio.sleep(0.05) + return _execute_response() + + client = _async_client(ExecuteCypher=AsyncMock(side_effect=slow_execute)) + tx = await client.begin_transaction() + stmt = asyncio.create_task(tx.cypher("CREATE (:A)")) + await started.wait() + with pytest.raises(RuntimeError, match="in flight"): + await tx.cypher("CREATE (:B)") + await stmt + assert client._cypher_stub.ExecuteCypher.await_count == 1 + + asyncio.run(_inner()) + + +class TestLocalEncodingFailureKeepsTheHandleUsable: + """A parameter the encoder rejects fails LOCALLY, before any RPC: the + handle must stay open (nothing changed server-side), not be marooned in + an in-flight state that rejects even rollback.""" + + def test_a_bad_parameter_leaves_the_transaction_open(self): + async def _inner() -> None: + client = _async_client() + tx = await client.begin_transaction() + with pytest.raises(Exception): + await tx.cypher("CREATE (:A {v: $v})", {"v": object()}) + assert client._cypher_stub.ExecuteCypher.await_count == 0 + assert tx.is_open is True + await tx.rollback() + assert client._cypher_stub.RollbackTransaction.await_count == 1 + + asyncio.run(_inner()) + + +class TestUnconfirmedCleanupIsRetriedByExplicitRollback: + """When a statement failed ambiguously AND its best-effort cleanup also + failed, the server may still hold the transaction. An explicit rollback() + afterwards must retry the cleanup instead of declaring the transaction + already gone.""" + + def test_rollback_after_failed_cleanup_sends_again(self): + from unittest.mock import AsyncMock + + async def _inner() -> None: + calls = {"n": 0} + + async def flaky_rollback(req, timeout=None): + calls["n"] += 1 + if calls["n"] == 1: + raise _TransportError(grpc.StatusCode.UNAVAILABLE) + return cypher_pb2.RollbackTransactionResponse() + + client = _async_client( + ExecuteCypher=AsyncMock(side_effect=_TransportError(grpc.StatusCode.UNAVAILABLE)), + RollbackTransaction=AsyncMock(side_effect=flaky_rollback), + ) + tx = await client.begin_transaction() + with pytest.raises(_TransportError): + await tx.cypher("CREATE (:A)") + # The inline cleanup failed (suppressed); connectivity recovers. + await tx.rollback() + assert client._cypher_stub.RollbackTransaction.await_count == 2 + + asyncio.run(_inner()) + + +class TestCancelledBlockDetachesTheRollback: + """A block unwound by cancellation must not hold __aexit__ for a full + rollback round trip: the cleanup goes detached (drained at close), so + the cancellation propagates immediately.""" + + def test_context_exit_does_not_await_the_rollback_inline(self): + from unittest.mock import AsyncMock + + async def _inner() -> None: + rollback_done = asyncio.Event() + + async def slow_rollback(req, timeout=None): + await asyncio.sleep(0.2) + rollback_done.set() + + client = _async_client(RollbackTransaction=AsyncMock(side_effect=slow_rollback)) + with pytest.raises(asyncio.CancelledError): + async with client.transaction(): + raise asyncio.CancelledError() + assert not rollback_done.is_set(), "the context exit awaited the rollback inline instead of detaching it" + await client.close() + assert rollback_done.is_set(), "the detached rollback was not drained" + + asyncio.run(_inner()) + + +class TestCancelledChannelCloseKeepsCleanupUsable: + """If channel.close() itself is cancelled, the transport may still be + usable: the closing flag must be restored so later cancellations still + spawn their cleanup instead of forfeiting it.""" + + def test_cleanups_still_spawn_after_a_cancelled_close(self): + from unittest.mock import AsyncMock + + async def _inner() -> None: + in_flight = asyncio.Event() + + async def hang(req, timeout=None): + in_flight.set() + await asyncio.sleep(10) + + class _HangingChannel: + async def close(self): + await asyncio.sleep(10) + + client = _async_client(ExecuteCypher=AsyncMock(side_effect=hang)) + client._channel = _HangingChannel() + tx = await client.begin_transaction() + stmt = asyncio.create_task(tx.cypher("CREATE (:A)")) + await in_flight.wait() + + closer = asyncio.create_task(client.close()) + await asyncio.sleep(0.02) # closer is awaiting channel.close() + closer.cancel() + with pytest.raises(asyncio.CancelledError): + await closer + + stmt.cancel() + with pytest.raises(asyncio.CancelledError): + await stmt + await asyncio.sleep(0.05) + assert client._cypher_stub.RollbackTransaction.await_count == 1, ( + "a cancelled channel close must not permanently disable cleanup" + ) + + asyncio.run(_inner()) + + +class TestInFlightOperationAtContextExit: + """A block that starts a statement or commit in a background task and + exits without awaiting it leaves the transaction in a transient state; + reporting a successful context exit then would let buffered writes and a + pinned snapshot outlive the owning scope unnoticed.""" + + def test_exiting_with_a_statement_in_flight_raises(self): + from contextlib import suppress + from unittest.mock import AsyncMock + + async def _inner() -> None: + in_flight = asyncio.Event() + + async def hang(req, timeout=None): + in_flight.set() + await asyncio.sleep(10) + + client = _async_client(ExecuteCypher=AsyncMock(side_effect=hang)) + bg = None + with pytest.raises(RuntimeError, match="in flight"): + async with client.transaction() as tx: + bg = asyncio.create_task(tx.cypher("CREATE (:A)")) + await in_flight.wait() + bg.cancel() + with suppress(asyncio.CancelledError): + await bg + + asyncio.run(_inner()) + + +class TestCancelledManualCommitCleansUpOnContextExit: + """A manual commit() inside the block, cancelled mid-flight, marks the + transaction indeterminate before the cancellation reaches the context + manager; the exit must still send the detached best-effort rollback in + case the commit never reached the server.""" + + def test_cancelled_manual_commit_spawns_cleanup(self): + from unittest.mock import AsyncMock + + async def _inner() -> None: + commit_in_flight = asyncio.Event() + + async def hang_commit(req, timeout=None): + commit_in_flight.set() + await asyncio.sleep(10) + + client = _async_client(CommitTransaction=AsyncMock(side_effect=hang_commit)) + + async def run_block() -> None: + async with client.transaction() as tx: + await tx.cypher("CREATE (:A)") + await tx.commit() + + t = asyncio.create_task(run_block()) + await commit_in_flight.wait() + t.cancel() + with pytest.raises(asyncio.CancelledError): + await t + # close() drains the detached cleanup before the assertion. + await client.close() + assert client._cypher_stub.RollbackTransaction.await_count == 1, ( + "a cancelled manual commit left the server transaction to the idle sweep" + ) + + asyncio.run(_inner()) + + +class TestCancelledAutomaticCommitDetachesTheCleanup: + """Cancellation during the context manager's automatic commit must not + hold the exit for an inline rollback round trip: caught cancellation is + not re-injected at the next await, so the exit could otherwise overrun a + surrounding asyncio.timeout by the whole cleanup deadline.""" + + def test_exit_does_not_wait_for_the_rollback(self): + from unittest.mock import AsyncMock + + async def _inner() -> None: + commit_in_flight = asyncio.Event() + rollback_done = asyncio.Event() + + async def hang_commit(req, timeout=None): + commit_in_flight.set() + await asyncio.sleep(10) + + async def slow_rollback(req, timeout=None): + await asyncio.sleep(0.2) + rollback_done.set() + + client = _async_client( + CommitTransaction=AsyncMock(side_effect=hang_commit), + RollbackTransaction=AsyncMock(side_effect=slow_rollback), + ) + + async def run_block() -> None: + async with client.transaction() as tx: + await tx.cypher("CREATE (:A)") + + t = asyncio.create_task(run_block()) + await commit_in_flight.wait() + t.cancel() + with pytest.raises(asyncio.CancelledError): + await t + assert not rollback_done.is_set(), "context exit held for the inline rollback" + await client.close() + assert rollback_done.is_set(), "the detached cleanup never ran" + assert client._cypher_stub.RollbackTransaction.await_count == 1 + + asyncio.run(_inner()) + + +class TestCancelledCloseStillClosesTheChannel: + """Cancelling the task awaiting close() must not abandon shutdown midway: + the finalization continues detached, so the drain finishes AND the channel + is released, while the cancellation propagates to the caller.""" + + def test_channel_is_closed_after_a_cancelled_close(self): + from unittest.mock import AsyncMock + + async def _inner() -> None: + stmt_in_flight = asyncio.Event() + + async def hang(req, timeout=None): + stmt_in_flight.set() + await asyncio.sleep(10) + + async def slow_rollback(req, timeout=None): + await asyncio.sleep(0.15) + + class _CountingChannel: + def __init__(self) -> None: + self.close_calls = 0 + + async def close(self) -> None: + self.close_calls += 1 + + client = _async_client( + ExecuteCypher=AsyncMock(side_effect=hang), + RollbackTransaction=AsyncMock(side_effect=slow_rollback), + ) + channel = _CountingChannel() + client._channel = channel + tx = await client.begin_transaction() + stmt = asyncio.create_task(tx.cypher("CREATE (:A)")) + await stmt_in_flight.wait() + stmt.cancel() + with pytest.raises(asyncio.CancelledError): + await stmt + + closer = asyncio.create_task(client.close()) + await asyncio.sleep(0.03) # closer is inside the drain, rollback still running + closer.cancel() + with pytest.raises(asyncio.CancelledError): + await closer + await asyncio.sleep(0.3) # finalization continues detached + assert channel.close_calls == 1, "cancelling close() abandoned the channel" + assert client._closing is True + + asyncio.run(_inner()) + + +class TestLateCancellationDuringChannelCloseStillRollsBack: + """A statement cancelled while channel.close() is in progress must still + get its cleanup: the transport is not conclusively gone yet, so forfeiting + the rollback to the idle sweep at that point strands an accepted + server-side transaction for no reason.""" + + def test_cleanup_spawned_during_channel_close_runs(self): + from unittest.mock import AsyncMock + + async def _inner() -> None: + stmt_in_flight = asyncio.Event() + release_close = asyncio.Event() + + async def hang(req, timeout=None): + stmt_in_flight.set() + await asyncio.sleep(10) + + class _BlockedChannel: + async def close(self) -> None: + await release_close.wait() + + client = _async_client(ExecuteCypher=AsyncMock(side_effect=hang)) + client._channel = _BlockedChannel() + tx = await client.begin_transaction() + stmt = asyncio.create_task(tx.cypher("CREATE (:A)")) + await stmt_in_flight.wait() + + closer = asyncio.create_task(client.close()) + await asyncio.sleep(0.02) # closer is awaiting channel.close() + stmt.cancel() + with pytest.raises(asyncio.CancelledError): + await stmt + await asyncio.sleep(0.05) # the detached cleanup runs while close is blocked + assert client._cypher_stub.RollbackTransaction.await_count == 1, ( + "a cancellation during channel close forfeited a reachable rollback" + ) + release_close.set() + await closer + + asyncio.run(_inner()) + + +class TestReconnectWaitsForActiveShutdown: + """connect() during a detached, still-running shutdown must serialize + with it: the finalizer clears the channel and raises the closing gate + when it resumes, and doing that to a freshly installed transport would + disable cleanup on the new connection and leak its channel.""" + + def test_connect_serializes_with_an_inflight_close(self): + async def _inner() -> None: + release_close = asyncio.Event() + + class _BlockedChannel: + async def close(self) -> None: + await release_close.wait() + + client = _async_client() + client._channel = _BlockedChannel() + closer = asyncio.create_task(client.close()) + await asyncio.sleep(0.02) # finalizer is awaiting channel.close() + closer.cancel() + with pytest.raises(asyncio.CancelledError): + await closer + + reconnect = asyncio.create_task(client.connect()) + await asyncio.sleep(0.05) + assert not reconnect.done(), "connect() replaced the transport under an active shutdown" + release_close.set() + await reconnect + await asyncio.sleep(0.05) # give a stale finalizer time to clobber, if any + assert client._channel is not None, "the old finalizer cleared the new channel" + assert client._closing is False, "the old finalizer disabled cleanup on the new connection" + await client.close() + + asyncio.run(_inner()) + + +class TestAbandonedInFlightStatementDoesNotReopen: + """A block that starts a background statement and then raises leaves the + scope while the operation is still in flight; when the straggler later + completes, it must not return the handle to `open` — nobody is left to + commit or roll it back, so its buffered writes and pinned snapshot would + leak until the idle sweep.""" + + def test_straggler_hands_the_transaction_to_cleanup(self): + from unittest.mock import AsyncMock + + async def _inner() -> None: + in_flight = asyncio.Event() + release = asyncio.Event() + + async def gated(req, timeout=None): + in_flight.set() + await release.wait() + return _execute_response() + + client = _async_client(ExecuteCypher=AsyncMock(side_effect=gated)) + bg = None + with pytest.raises(ValueError): + async with client.transaction() as tx: + bg = asyncio.create_task(tx.cypher("CREATE (:A)")) + await in_flight.wait() + raise ValueError("boom") + release.set() + await bg # the statement completes after the owner is gone + assert tx.is_open is False, "the straggler returned the abandoned handle to open" + await client.close() + assert client._cypher_stub.RollbackTransaction.await_count == 1, ( + "an abandoned transaction was left to the idle sweep" + ) + + asyncio.run(_inner()) + + +class TestRollbackBeforeTheDetachedCleanupFirstTurn: + """The cleanup-confirmed flag must fall when the detached task is + SCHEDULED, not on its first step: an explicit rollback() racing in before + that first turn otherwise sees the flag still up, sends nothing, and a + loop shutdown can then cancel the pending task with nothing ever sent.""" + + def test_explicit_rollback_in_the_prestart_window_sends(self): + from unittest.mock import AsyncMock + + async def _inner() -> None: + in_flight = asyncio.Event() + + async def hang(req, timeout=None): + in_flight.set() + await asyncio.sleep(10) + + client = _async_client(ExecuteCypher=AsyncMock(side_effect=hang)) + tx = await client.begin_transaction() + stmt = asyncio.create_task(tx.cypher("CREATE (:A)")) + await in_flight.wait() + stmt.cancel() + # One yield: the cancellation handler runs (spawning the detached + # cleanup), but the detached task itself is still behind us in + # the ready queue — the pre-start window. + await asyncio.sleep(0) + await tx.rollback() + assert client._cypher_stub.RollbackTransaction.await_count >= 1, ( + "rollback() trusted a cleanup that had not started yet" + ) + with pytest.raises(asyncio.CancelledError): + await stmt + await client.close() + + asyncio.run(_inner()) + + +class TestCaughtIndeterminateCommitCleansUpOnNormalExit: + """A manual commit() inside the block that fails ambiguously and is + CAUGHT there routes the exit through the normal path; the indeterminate + transaction must still get the bounded best-effort cleanup in case the + commit never reached the server, with the verdict preserved.""" + + def test_normal_exit_sends_cleanup_and_keeps_the_verdict(self): + from contextlib import suppress as ctx_suppress + from unittest.mock import AsyncMock + + async def _inner() -> None: + client = _async_client( + CommitTransaction=AsyncMock(side_effect=_TransportError(grpc.StatusCode.DEADLINE_EXCEEDED)) + ) + async with client.transaction() as tx: + await tx.cypher("CREATE (:A)") + with ctx_suppress(grpc.RpcError): + await tx.commit() + assert client._cypher_stub.RollbackTransaction.await_count == 1, ( + "a caught ambiguous commit left the server transaction to the idle sweep" + ) + with pytest.raises(RuntimeError, match="outcome is unknown"): + await tx.cypher("RETURN 1") + + asyncio.run(_inner()) + + +class TestSyncCaughtIndeterminateCommitCleansUpOnNormalExit: + """Sync mirror of the async normal-exit rule: a manual commit() that + fails ambiguously and is caught inside the block leaves the transaction + indeterminate; the context exit must still send the bounded best-effort + cleanup in case the commit never reached the server.""" + + def test_normal_exit_sends_cleanup_and_keeps_the_verdict(self): + from contextlib import suppress as ctx_suppress + from unittest.mock import AsyncMock + + client = _sync_client( + CommitTransaction=AsyncMock(side_effect=_TransportError(grpc.StatusCode.DEADLINE_EXCEEDED)) + ) + with client.transaction() as tx: + tx.cypher("CREATE (:A)") + with ctx_suppress(grpc.RpcError): + tx.commit() + assert client._async._cypher_stub.RollbackTransaction.await_count == 1, ( + "a caught ambiguous commit left the server transaction to the idle sweep" + ) + with pytest.raises(RuntimeError, match="outcome is unknown"): + tx.cypher("RETURN 1") + + +class TestRepeatedRollbackRetriesUnconfirmedCleanup: + """rollback() on an aborted handle must not settle into rolled_back while + the cleanup remains unconfirmed: a retry that itself failed would + otherwise report success and lock out every later retry, leaving the + server transaction to the idle sweep even after connectivity recovers.""" + + def test_state_settles_only_after_a_confirmed_cleanup(self): + from unittest.mock import AsyncMock + + async def _inner() -> None: + in_flight = asyncio.Event() + + async def hang(req, timeout=None): + in_flight.set() + await asyncio.sleep(10) + + rollback = AsyncMock( + side_effect=[ + _TransportError(grpc.StatusCode.UNAVAILABLE), + _TransportError(grpc.StatusCode.UNAVAILABLE), + cypher_pb2.RollbackTransactionResponse(), + ] + ) + client = _async_client(ExecuteCypher=AsyncMock(side_effect=hang), RollbackTransaction=rollback) + tx = await client.begin_transaction() + stmt = asyncio.create_task(tx.cypher("CREATE (:A)")) + await in_flight.wait() + stmt.cancel() + with pytest.raises(asyncio.CancelledError): + await stmt + await asyncio.sleep(0.01) # the detached cleanup runs and fails (1st call) + await tx.rollback() # the retry fails too (2nd call); must stay retriable + await tx.rollback() # this retry succeeds (3rd call) + assert client._cypher_stub.RollbackTransaction.await_count == 3 + await client.close() + + asyncio.run(_inner()) + + +class TestCancelledNormalExitCleanupPropagates: + """Cancellation landing while the normal-exit indeterminate cleanup is + awaiting the server must propagate, not be swallowed into a + successful-looking exit — and the interrupted cleanup must be retried + detached so the server transaction is still freed.""" + + def test_cancellation_mid_cleanup_is_not_swallowed(self): + from contextlib import suppress as ctx_suppress + from unittest.mock import AsyncMock + + async def _inner() -> None: + rollback_started = asyncio.Event() + calls = {"n": 0} + + async def rollback(req, timeout=None): + calls["n"] += 1 + if calls["n"] == 1: + rollback_started.set() + await asyncio.sleep(10) + return cypher_pb2.RollbackTransactionResponse() + + client = _async_client( + CommitTransaction=AsyncMock(side_effect=_TransportError(grpc.StatusCode.DEADLINE_EXCEEDED)), + RollbackTransaction=AsyncMock(side_effect=rollback), + ) + + async def run_block() -> None: + async with client.transaction() as tx: + await tx.cypher("CREATE (:A)") + with ctx_suppress(grpc.RpcError): + await tx.commit() + + t = asyncio.create_task(run_block()) + await rollback_started.wait() + t.cancel() + with pytest.raises(asyncio.CancelledError): + await t + await client.close() # drains the detached retry + assert calls["n"] == 2, "the interrupted cleanup was never retried" + + asyncio.run(_inner()) + + +class TestAbandonedInFlightCommitIsContested: + """A block that starts commit() in a background task and then raises has + asked for a rollback it can no longer perform itself: the exit must send + the best-effort rollback to CONTEST the in-flight commit, so the server + race decides the outcome instead of the commit silently applying despite + the exception.""" + + def test_exceptional_exit_sends_a_contesting_rollback(self): + from unittest.mock import AsyncMock + + async def _inner() -> None: + commit_in_flight = asyncio.Event() + release_commit = asyncio.Event() + + async def gated_commit(req, timeout=None): + commit_in_flight.set() + await release_commit.wait() + return cypher_pb2.CommitTransactionResponse(applied_index=7) + + client = _async_client(CommitTransaction=AsyncMock(side_effect=gated_commit)) + bg = None + with pytest.raises(ValueError): + async with client.transaction() as tx: + await tx.cypher("CREATE (:A)") + bg = asyncio.create_task(tx.commit()) + await commit_in_flight.wait() + raise ValueError("boom") + await asyncio.sleep(0.02) # the detached contesting rollback runs + assert client._cypher_stub.RollbackTransaction.await_count == 1, ( + "an abandoned in-flight commit was left uncontested" + ) + release_commit.set() + await bg # the fake server lets the commit win; that is its call + await client.close() + + asyncio.run(_inner()) + + +class TestFailedExplicitRollbackStaysRetriable: + """An explicit rollback() whose request is lost in transit must not leave + the handle permanently rolled_back: the server may still hold the + transaction, so once connectivity recovers a later rollback() has to be + able to retry instead of being rejected as already finished.""" + + def test_rollback_can_be_retried_after_a_transport_failure(self): + from unittest.mock import AsyncMock + + async def _inner() -> None: + rollback = AsyncMock( + side_effect=[ + _TransportError(grpc.StatusCode.UNAVAILABLE), + cypher_pb2.RollbackTransactionResponse(), + ] + ) + client = _async_client(RollbackTransaction=rollback) + tx = await client.begin_transaction() + with pytest.raises(_TransportError): + await tx.rollback() + assert tx.is_open is False, "a failed rollback left the handle usable" + await tx.rollback() # connectivity recovered; the retry must go through + assert client._cypher_stub.RollbackTransaction.await_count == 2 + await client.close() + + asyncio.run(_inner()) + + +class TestSyncStatementInterruptedInsideTheTask: + """KeyboardInterrupt raised while the statement coroutine itself is + stepping completes the task before _run() can cancel-and-drain it, so no + async handler closes the handle; the sync wrapper must then settle the + outcome conservatively instead of leaving the handle parked in + "executing" forever.""" + + def test_interrupted_statement_closes_the_handle(self): + from unittest.mock import AsyncMock + + async def ki(req, timeout=None): + raise KeyboardInterrupt + + client = _sync_client(ExecuteCypher=AsyncMock(side_effect=ki)) + tx = client.begin_transaction() + with pytest.raises(KeyboardInterrupt): + tx.cypher("CREATE (:A)") + assert tx._inner._state == "aborted", "the interruption left the handle parked in-flight" + # The server may still hold the transaction; an explicit rollback + # must send the cleanup rather than trusting one that never ran. + tx.rollback() + assert client._async._cypher_stub.RollbackTransaction.await_count == 1 + + +class TestCancelledFailedCommitCleanupPropagates: + """When the automatic commit fails ambiguously and cancellation then + lands during the inline cleanup, the cancellation must propagate (a + swallowed one is simply lost — asyncio does not re-inject it) and the + interrupted cleanup must be retried detached.""" + + def test_cancellation_mid_failed_commit_cleanup_is_not_swallowed(self): + from unittest.mock import AsyncMock + + async def _inner() -> None: + rollback_started = asyncio.Event() + calls = {"n": 0} + + async def rollback(req, timeout=None): + calls["n"] += 1 + if calls["n"] == 1: + rollback_started.set() + await asyncio.sleep(10) + return cypher_pb2.RollbackTransactionResponse() + + client = _async_client( + CommitTransaction=AsyncMock(side_effect=_TransportError(grpc.StatusCode.DEADLINE_EXCEEDED)), + RollbackTransaction=AsyncMock(side_effect=rollback), + ) + + async def run_block() -> None: + async with client.transaction() as tx: + await tx.cypher("CREATE (:A)") + + t = asyncio.create_task(run_block()) + await rollback_started.wait() + t.cancel() + with pytest.raises(asyncio.CancelledError): + await t + await client.close() # drains the detached retry + assert calls["n"] == 2, "the interrupted cleanup was never retried" + + asyncio.run(_inner()) + + +class TestCancelledManualRollbackGetsDetachedRetry: + """A manual rollback() cancelled mid-RPC leaves the handle aborted with + the cleanup unconfirmed; the context exit must register the detached + retry, or closing the client strands the server-side transaction until + the idle sweep.""" + + def test_cancelled_rollback_in_context_spawns_cleanup(self): + from unittest.mock import AsyncMock + + async def _inner() -> None: + rollback_started = asyncio.Event() + calls = {"n": 0} + + async def rollback(req, timeout=None): + calls["n"] += 1 + if calls["n"] == 1: + rollback_started.set() + await asyncio.sleep(10) + return cypher_pb2.RollbackTransactionResponse() + + client = _async_client(RollbackTransaction=AsyncMock(side_effect=rollback)) + + async def run_block() -> None: + async with client.transaction() as tx: + await tx.cypher("CREATE (:A)") + await tx.rollback() + + t = asyncio.create_task(run_block()) + await rollback_started.wait() + t.cancel() + with pytest.raises(asyncio.CancelledError): + await t + await client.close() # drains the detached retry + assert calls["n"] == 2, "a cancelled manual rollback got no detached retry" + + asyncio.run(_inner()) + + +class TestSyncLocalEncodingFailureKeepsTheHandleUsable: + """Sync mirror of the async encoding-failure contract: a parameter the + encoder rejects fails locally before any RPC, so the handle must stay + open — the interrupt settlement is for real interruptions only, not for + ordinary local exceptions.""" + + def test_a_bad_parameter_leaves_the_sync_transaction_open(self): + client = _sync_client() + tx = client.begin_transaction() + tx.cypher("CREATE (:A)") + with pytest.raises(Exception): + tx.cypher("CREATE (:B {v: $v})", {"v": object()}) + assert client._async._cypher_stub.ExecuteCypher.await_count == 1, ( + "the failing statement must not have reached the wire" + ) + assert tx.is_open is True, "a local encoding failure aborted a usable transaction" + tx.rollback() + assert client._async._cypher_stub.RollbackTransaction.await_count == 1 + + +class TestCancelledExceptionalRollbackPropagates: + """Cancellation landing while the exceptional-exit rollback awaits the + server must propagate (not be traded for the block's earlier error) and + the interrupted rollback must get its detached retry.""" + + def test_cancellation_mid_exceptional_rollback_is_not_swallowed(self): + from unittest.mock import AsyncMock + + async def _inner() -> None: + rollback_started = asyncio.Event() + calls = {"n": 0} + + async def rollback(req, timeout=None): + calls["n"] += 1 + if calls["n"] == 1: + rollback_started.set() + await asyncio.sleep(10) + return cypher_pb2.RollbackTransactionResponse() + + client = _async_client(RollbackTransaction=AsyncMock(side_effect=rollback)) + + async def run_block() -> None: + async with client.transaction() as tx: + await tx.cypher("CREATE (:A)") + raise ValueError("boom") + + t = asyncio.create_task(run_block()) + await rollback_started.wait() + t.cancel() + with pytest.raises(asyncio.CancelledError): + await t + await client.close() # drains the detached retry + assert calls["n"] == 2, "the interrupted exceptional rollback got no retry" + + asyncio.run(_inner()) + + +class TestSyncContextRetriesSettledInterruptCleanup: + """An in-task interruption settles the sync handle as aborted with the + cleanup unconfirmed; the surrounding sync transaction context must then + send the bounded best-effort rollback on exit instead of leaving the + server transaction to the idle sweep.""" + + def test_interrupted_statement_in_context_still_rolls_back(self): + from unittest.mock import AsyncMock + + async def ki(req, timeout=None): + raise KeyboardInterrupt + + client = _sync_client(ExecuteCypher=AsyncMock(side_effect=ki)) + # The scope stays inside pytest.raises because its exit is what is + # under test: it must send the cleanup AND still let the interrupt + # through. The await count below pins where the interrupt came from, + # so a passing test cannot mean one raised by any other phase. + with pytest.raises(KeyboardInterrupt): + with client.transaction() as tx: + tx.cypher("CREATE (:A)") + assert client._async._cypher_stub.ExecuteCypher.await_count == 1 + assert tx._inner._state == "aborted" + assert client._async._cypher_stub.RollbackTransaction.await_count >= 1, ( + "the interrupted sync transaction was left to the idle sweep" + ) + + +class TestNormalExitRetriesUnconfirmedCleanup: + """A failed statement whose own cleanup also failed, with the error + caught inside the block, reaches the normal exit as aborted with the + cleanup unconfirmed; the exit must retry the bounded request instead of + leaving the server transaction to the idle sweep.""" + + def test_async_normal_exit_retries(self): + from contextlib import suppress as ctx_suppress + from unittest.mock import AsyncMock + + async def _inner() -> None: + rollback = AsyncMock( + side_effect=[ + _TransportError(grpc.StatusCode.UNAVAILABLE), + cypher_pb2.RollbackTransactionResponse(), + ] + ) + client = _async_client( + ExecuteCypher=AsyncMock(side_effect=_ServerRejected()), + RollbackTransaction=rollback, + ) + async with client.transaction() as tx: + with ctx_suppress(grpc.RpcError): + await tx.cypher("CREATE (:A)") + assert client._cypher_stub.RollbackTransaction.await_count == 2, ( + "the failed cleanup was never retried on the normal exit" + ) + assert tx._cleanup_confirmed is True + + asyncio.run(_inner()) + + def test_sync_normal_exit_retries(self): + from contextlib import suppress as ctx_suppress + from unittest.mock import AsyncMock + + rollback = AsyncMock( + side_effect=[ + _TransportError(grpc.StatusCode.UNAVAILABLE), + cypher_pb2.RollbackTransactionResponse(), + ] + ) + client = _sync_client( + ExecuteCypher=AsyncMock(side_effect=_ServerRejected()), + RollbackTransaction=rollback, + ) + with client.transaction() as tx: + with ctx_suppress(grpc.RpcError): + tx.cypher("CREATE (:A)") + assert client._async._cypher_stub.RollbackTransaction.await_count == 2, ( + "the failed cleanup was never retried on the sync normal exit" + ) + + +class TestSkippedShutdownCleanupIsUnconfirmed: + """The closing gate may skip the cleanup spawn, but the transaction can + still have reached the server: the skipped cleanup must read as + unconfirmed, so an explicit rollback() after a reconnect retries it + instead of trusting a cleanup that never ran.""" + + def test_gated_spawn_leaves_the_cleanup_unconfirmed(self): + from unittest.mock import AsyncMock + + async def _inner() -> None: + in_flight = asyncio.Event() + + async def hang(req, timeout=None): + in_flight.set() + await asyncio.sleep(10) + + client = _async_client(ExecuteCypher=AsyncMock(side_effect=hang)) + tx = await client.begin_transaction() + task = asyncio.create_task(tx.cypher("CREATE (:A)")) + await in_flight.wait() + await client.close() + task.cancel() + with pytest.raises(asyncio.CancelledError): + await task + assert tx._cleanup_confirmed is False, "a cleanup the closing gate skipped must not read as done" + # After a reconnect, the explicit rollback must send the retry. + await tx.rollback() + assert client._cypher_stub.RollbackTransaction.await_count == 1 + + asyncio.run(_inner()) + + +class TestSyncInterruptUnwindUsesBoundedCleanup: + """A sync transaction block unwound by Ctrl-C or SystemExit must not + hold the exit for the full request deadline on a stalled server: the + cleanup goes out with the bounded deadline, not the ordinary rollback + timeout.""" + + def test_interrupt_exit_sends_the_bounded_request(self): + from unittest.mock import AsyncMock + + recorded = {} + + async def rollback(req, timeout=None): + recorded["timeout"] = timeout + return cypher_pb2.RollbackTransactionResponse() + + client = _sync_client(RollbackTransaction=AsyncMock(side_effect=rollback)) + with pytest.raises(KeyboardInterrupt): + with client.transaction() as tx: + tx.cypher("CREATE (:A)") + raise KeyboardInterrupt + assert tx.is_open is False + assert recorded["timeout"] == 5.0, ( + "an interrupt unwind must use the bounded cleanup deadline, not the full rollback timeout" + ) + + +class TestDirectRollbackCancellationSpawnsCleanup: + """A transaction rolled back directly (no context manager) and cancelled + mid-RPC has no exit handler to retry for it, and the cancelled owner may + never call again: rollback() itself must hand the cleanup to a detached + task before propagating the cancellation.""" + + def test_cancelled_direct_rollback_gets_detached_retry(self): + from unittest.mock import AsyncMock + + async def _inner() -> None: + rollback_started = asyncio.Event() + calls = {"n": 0} + + async def rollback(req, timeout=None): + calls["n"] += 1 + if calls["n"] == 1: + rollback_started.set() + await asyncio.sleep(10) + return cypher_pb2.RollbackTransactionResponse() + + client = _async_client(RollbackTransaction=AsyncMock(side_effect=rollback)) + tx = await client.begin_transaction() + t = asyncio.create_task(tx.rollback()) + await rollback_started.wait() + t.cancel() + with pytest.raises(asyncio.CancelledError): + await t + await client.close() # drains the detached retry + assert calls["n"] == 2, "a cancelled direct rollback got no detached retry" + + asyncio.run(_inner()) + + +class TestExceptionalExitRetriesUnconfirmedCleanup: + """A block that catches a failed statement (whose cleanup also failed) + and raises a DIFFERENT error reaches the exceptional exit as aborted + with the cleanup unconfirmed; the exit must retry the bounded request + while preserving the block's own exception.""" + + def test_exceptional_exit_retries_and_keeps_the_error(self): + from unittest.mock import AsyncMock + + async def _inner() -> None: + rollback = AsyncMock( + side_effect=[ + _TransportError(grpc.StatusCode.UNAVAILABLE), + cypher_pb2.RollbackTransactionResponse(), + ] + ) + client = _async_client( + ExecuteCypher=AsyncMock(side_effect=_ServerRejected()), + RollbackTransaction=rollback, + ) + with pytest.raises(RuntimeError, match="different"): + async with client.transaction() as tx: + try: + await tx.cypher("CREATE (:A)") + except grpc.RpcError: + raise RuntimeError("different") from None + assert client._cypher_stub.RollbackTransaction.await_count == 2, ( + "the failed cleanup was never retried on the exceptional exit" + ) + + asyncio.run(_inner()) + + +class TestSyncCommitCleanupInterruptPropagates: + """Ctrl-C arriving while the sync automatic-commit cleanup is running is + the user's word: it must propagate instead of being swallowed into + reporting the earlier commit error.""" + + def test_interrupt_during_commit_cleanup_wins(self): + from unittest.mock import AsyncMock + + client = _sync_client( + CommitTransaction=AsyncMock(side_effect=_TransportError(grpc.StatusCode.DEADLINE_EXCEEDED)), + RollbackTransaction=AsyncMock(side_effect=KeyboardInterrupt), + ) + with pytest.raises(KeyboardInterrupt): + with client.transaction() as tx: + tx.cypher("CREATE (:A)") + + +class TestCancelledIndeterminateRollbackSpawnsRetry: + """rollback() on an indeterminate handle whose best-effort request is + cancelled mid-RPC must hand the cleanup to a detached retry before + propagating: a direct caller has no context manager to do it, and the + commit may never have reached the server.""" + + def test_cancelled_indeterminate_rollback_gets_detached_retry(self): + from contextlib import suppress as ctx_suppress + from unittest.mock import AsyncMock + + async def _inner() -> None: + rollback_started = asyncio.Event() + calls = {"n": 0} + + async def rollback(req, timeout=None): + calls["n"] += 1 + if calls["n"] == 1: + rollback_started.set() + await asyncio.sleep(10) + return cypher_pb2.RollbackTransactionResponse() + + client = _async_client( + CommitTransaction=AsyncMock(side_effect=_TransportError(grpc.StatusCode.DEADLINE_EXCEEDED)), + RollbackTransaction=AsyncMock(side_effect=rollback), + ) + tx = await client.begin_transaction() + with ctx_suppress(grpc.RpcError): + await tx.commit() # lands indeterminate + t = asyncio.create_task(tx.rollback()) + await rollback_started.wait() + t.cancel() + with pytest.raises(asyncio.CancelledError): + await t + await client.close() # drains the detached retry + assert calls["n"] == 2, "a cancelled indeterminate rollback got no retry" + + asyncio.run(_inner()) + + +class TestAsyncInterruptUnwindDetachesTheRollback: + """An async block unwound by KeyboardInterrupt or SystemExit must not + hold the exit for the full request deadline: the cleanup goes detached + (bounded, drained at close) while the interrupt propagates.""" + + def test_interrupt_exit_does_not_wait_for_the_rollback(self): + from unittest.mock import AsyncMock + + async def _inner() -> None: + rollback_done = asyncio.Event() + + async def slow_rollback(req, timeout=None): + await asyncio.sleep(0.2) + rollback_done.set() + return cypher_pb2.RollbackTransactionResponse() + + client = _async_client(RollbackTransaction=AsyncMock(side_effect=slow_rollback)) + + async def run_block() -> None: + async with client.transaction() as tx: + await tx.cypher("CREATE (:A)") + raise KeyboardInterrupt + + with pytest.raises(KeyboardInterrupt): + await run_block() + assert not rollback_done.is_set(), "the interrupt exit held for the inline rollback" + await client.close() + assert rollback_done.is_set(), "the detached cleanup never ran" + + asyncio.run(_inner()) + + +class TestCancelledBeginReclaimsTheAllocation: + """Cancellation landing after the server allocated the transaction but + before the begin reply reaches the caller loses the handle entirely; the + late reply must be collected detached and handed straight to a rollback, + or the pinned snapshot survives until the idle sweep.""" + + def test_cancelled_begin_rolls_back_the_late_allocation(self): + from unittest.mock import AsyncMock + + async def _inner() -> None: + begin_started = asyncio.Event() + release_begin = asyncio.Event() + + async def gated_begin(req, timeout=None): + begin_started.set() + await release_begin.wait() + return cypher_pb2.BeginTransactionResponse(transaction_id=42) + + client = _async_client(BeginTransaction=AsyncMock(side_effect=gated_begin)) + t = asyncio.create_task(client.begin_transaction()) + await begin_started.wait() + t.cancel() + with pytest.raises(asyncio.CancelledError): + await t + release_begin.set() + await client.close() # drains the reclaim task + rollback = client._cypher_stub.RollbackTransaction + assert rollback.await_count == 1, "the late begin reply was never reclaimed" + assert rollback.await_args.args[0].transaction_id == 42 + + asyncio.run(_inner()) + + +class TestCancelledAbortedRetrySpawnsCleanup: + """The aborted-branch retry in rollback(), cancelled mid-RPC, must hand + the cleanup to a detached task like the indeterminate and open branches + already do: a direct caller has no exit handler to retry for it.""" + + def test_cancelled_aborted_retry_gets_detached_cleanup(self): + from contextlib import suppress as ctx_suppress + from unittest.mock import AsyncMock + + async def _inner() -> None: + retry_started = asyncio.Event() + calls = {"n": 0} + + async def rollback(req, timeout=None): + calls["n"] += 1 + if calls["n"] == 1: + raise _TransportError(grpc.StatusCode.UNAVAILABLE) + if calls["n"] == 2: + retry_started.set() + await asyncio.sleep(10) + return cypher_pb2.RollbackTransactionResponse() + + client = _async_client( + ExecuteCypher=AsyncMock(side_effect=_ServerRejected()), + RollbackTransaction=AsyncMock(side_effect=rollback), + ) + tx = await client.begin_transaction() + with ctx_suppress(grpc.RpcError): + await tx.cypher("CREATE (:A)") # aborts; its cleanup fails (1st call) + t = asyncio.create_task(tx.rollback()) # aborted branch retries (2nd call) + await retry_started.wait() + t.cancel() + with pytest.raises(asyncio.CancelledError): + await t + await client.close() # drains the detached retry (3rd call) + assert calls["n"] == 3, "a cancelled aborted-branch retry got no detached cleanup" + + asyncio.run(_inner()) + + +class TestBeginWorksWithGrpcStyleAwaitables: + """A real grpc.aio unary stub returns a UnaryUnaryCall — an AWAITABLE, + not a coroutine object — and loop.create_task() accepts only coroutines. + AsyncMock hides this by returning coroutines, so this test wires a + call-like awaitable the way the real transport does.""" + + def test_begin_accepts_a_non_coroutine_awaitable(self): + from unittest.mock import MagicMock + + class _CallLike: + def __init__(self, resp): + self._resp = resp + + def __await__(self): + async def _deliver(): + return self._resp + + return _deliver().__await__() + + async def _inner() -> None: + client = _async_client( + BeginTransaction=MagicMock( + side_effect=lambda req, timeout=None: _CallLike( + cypher_pb2.BeginTransactionResponse(transaction_id=42) + ) + ) + ) + tx = await client.begin_transaction() + assert tx.transaction_id == 42 + + asyncio.run(_inner()) + + +class TestAsyncStatementSettlesAfterProcessControlException: + """KeyboardInterrupt or SystemExit raised from inside the awaited + statement matches neither the gRPC nor the cancellation handler; the + handle must still close conservatively (aborted, cleanup unconfirmed) + instead of staying "executing" forever.""" + + def test_interrupted_statement_closes_the_handle(self): + from unittest.mock import AsyncMock + + async def _inner() -> None: + client = _async_client(ExecuteCypher=AsyncMock(side_effect=KeyboardInterrupt)) + tx = await client.begin_transaction() + with pytest.raises(KeyboardInterrupt): + await tx.cypher("CREATE (:A)") + assert tx._state == "aborted", "the interruption left the handle parked in-flight" + await tx.rollback() # must be able to send the cleanup + await client.close() + assert client._cypher_stub.RollbackTransaction.await_count >= 1 + + asyncio.run(_inner()) + + +class TestAsyncCommitSettlesAfterProcessControlException: + """KeyboardInterrupt or SystemExit raised from inside the awaited commit + leaves the outcome unknowable — the request may already have applied — + so the handle must settle as indeterminate, not stay "committing".""" + + def test_interrupted_commit_is_indeterminate(self): + from unittest.mock import AsyncMock + + async def _inner() -> None: + client = _async_client(CommitTransaction=AsyncMock(side_effect=KeyboardInterrupt)) + tx = await client.begin_transaction() + with pytest.raises(KeyboardInterrupt): + await tx.commit() + assert tx._state == "indeterminate" + with pytest.raises(RuntimeError, match="outcome is unknown"): + await tx.cypher("RETURN 1") + + asyncio.run(_inner()) + + +class TestUnknownTransactionAnswerConfirmsCleanup: + """The server answering a cleanup rollback with NOT_FOUND ("unknown + transaction id") is a definitive statement that nothing is held: the + cleanup must read as confirmed, so no redundant retries follow and an + explicit rollback() can settle the handle.""" + + def test_not_found_confirms_the_cleanup(self): + from contextlib import suppress as ctx_suppress + from unittest.mock import AsyncMock + + async def _inner() -> None: + client = _async_client( + ExecuteCypher=AsyncMock(side_effect=_ServerRejected()), + RollbackTransaction=AsyncMock(side_effect=_TransportError(grpc.StatusCode.NOT_FOUND)), + ) + tx = await client.begin_transaction() + with ctx_suppress(grpc.RpcError): + await tx.cypher("CREATE (:A)") # aborts; cleanup answered NOT_FOUND + assert tx._cleanup_confirmed is True, "a definitive unknown-id answer read as a lost request" + await tx.rollback() + assert tx._state == "rolled_back" + assert client._cypher_stub.RollbackTransaction.await_count == 1, "redundant cleanup retry" + + asyncio.run(_inner()) + + def test_not_found_settles_a_direct_rollback(self): + """A transaction the idle sweep already reclaimed answers the + caller's own rollback() with NOT_FOUND. Nothing is held and no + commit was ever sent, so the discard the method promises is a fact, + not a failure: it must return and settle the handle terminally, + rather than leaving an "aborted" one a second call would retry.""" + from unittest.mock import AsyncMock + + async def _inner() -> None: + client = _async_client( + RollbackTransaction=AsyncMock(side_effect=_TransportError(grpc.StatusCode.NOT_FOUND)), + ) + tx = await client.begin_transaction() + await tx.rollback() + assert tx._state == "rolled_back", "a settled discard reported as an unfinished one" + assert tx._cleanup_confirmed is True + assert client._cypher_stub.RollbackTransaction.await_count == 1 + # Terminal, so nothing is left for a later call to re-send. + with pytest.raises(RuntimeError, match="already rolled back"): + await tx.rollback() + + asyncio.run(_inner()) + + +class TestCancelledBeginReclaimerIsBounded: + """The reclaimer that collects a cancelled begin's late reply must be + bounded by the cleanup deadline: close() drains it before releasing the + transport, so an unbounded wait would stall shutdown for the whole + request timeout on a stalled server.""" + + def test_close_does_not_stall_on_a_hanging_begin(self): + from unittest.mock import AsyncMock, patch + + async def _inner() -> None: + started = asyncio.Event() + + async def hanging_begin(req, timeout=None): + started.set() + await asyncio.sleep(30) + return cypher_pb2.BeginTransactionResponse(transaction_id=42) + + # Patched for the whole scope: the reclaimer reads the deadline + # when it first runs, which is already during the cancellation. + with patch("coordinode.client._CLEANUP_TIMEOUT_SECS", 0.1): + client = _async_client(BeginTransaction=AsyncMock(side_effect=hanging_begin)) + t = asyncio.create_task(client.begin_transaction()) + await started.wait() + t.cancel() + with pytest.raises(asyncio.CancelledError): + await t + await asyncio.wait_for(client.close(), timeout=2) + + asyncio.run(_inner()) + + +class TestInFlightRollbackBlocksContextExit: + """A rollback started in a background task and still awaiting the server + when the block exits must be caught by the in-flight guard: reporting a + successful exit would let the RPC settle after the owning scope is gone, + with a close or a late failure stranding the server transaction.""" + + def test_exiting_with_a_rollback_in_flight_raises(self): + from contextlib import suppress as ctx_suppress + from unittest.mock import AsyncMock + + async def _inner() -> None: + in_flight = asyncio.Event() + + async def hang(req, timeout=None): + in_flight.set() + await asyncio.sleep(10) + + client = _async_client(RollbackTransaction=AsyncMock(side_effect=hang)) + bg = None + with pytest.raises(RuntimeError, match="in flight"): + async with client.transaction() as tx: + bg = asyncio.create_task(tx.rollback()) + await in_flight.wait() + bg.cancel() + with ctx_suppress(asyncio.CancelledError): + await bg + + asyncio.run(_inner()) + + +class TestTransportCancelledCleanupKeepsTheStatementError: + """A CancelledError raised by the TRANSPORT during the inline cleanup, + with no cancellation pending on the task, must not replace the + statement's own gRPC failure: only a real caller cancellation + supersedes it.""" + + def test_statement_error_survives_a_spurious_cleanup_cancellation(self): + from unittest.mock import AsyncMock + + async def _inner() -> None: + client = _async_client( + ExecuteCypher=AsyncMock(side_effect=_ServerRejected()), + RollbackTransaction=AsyncMock(side_effect=asyncio.CancelledError()), + ) + tx = await client.begin_transaction() + with pytest.raises(_ServerRejected): + await tx.cypher("CREATE (:A)") + assert tx._state == "aborted" + + asyncio.run(_inner()) + + +class TestTransportCancelledCommitCleanupKeepsTheError: + """A CancelledError raised by the TRANSPORT while the automatic commit's + cleanup runs, with no cancellation pending on the task, must not replace + the commit's own gRPC failure — the same rule the statement path + follows.""" + + def test_commit_error_survives_a_spurious_cleanup_cancellation(self): + from unittest.mock import AsyncMock + + async def _inner() -> None: + client = _async_client( + CommitTransaction=AsyncMock(side_effect=_TransportError(grpc.StatusCode.DEADLINE_EXCEEDED)), + RollbackTransaction=AsyncMock(side_effect=asyncio.CancelledError()), + ) + with pytest.raises(_TransportError): + async with client.transaction() as tx: + await tx.cypher("CREATE (:A)") + assert tx._state == "indeterminate" + + asyncio.run(_inner()) + + +class TestTransportCancelledCleanupKeepsANormalExit: + """The same rule on the NORMAL exit: a manual commit caught inside the + block leaves the handle indeterminate, and a transport-raised + CancelledError during the exit's cleanup must not turn a successful + block into a cancelled one.""" + + def test_normal_exit_survives_a_spurious_cleanup_cancellation(self): + from contextlib import suppress as ctx_suppress + from unittest.mock import AsyncMock + + async def _inner() -> None: + client = _async_client( + CommitTransaction=AsyncMock(side_effect=_TransportError(grpc.StatusCode.DEADLINE_EXCEEDED)), + RollbackTransaction=AsyncMock(side_effect=asyncio.CancelledError()), + ) + async with client.transaction() as tx: + await tx.cypher("CREATE (:A)") + with ctx_suppress(grpc.RpcError): + await tx.commit() + assert tx._state == "indeterminate" + + asyncio.run(_inner()) + + +class TestQueuedStatementIsSeenBeforeAutoCommit: + """A statement queued with create_task and never awaited has not reached + its state reservation when the block exits; auto-committing then would + report success while that write is silently dropped. The exit must give + queued work its first step and see the in-flight operation.""" + + def test_queued_statement_blocks_the_auto_commit(self): + from contextlib import suppress as ctx_suppress + from unittest.mock import AsyncMock + + async def _inner() -> None: + # The statement must actually reach the wire and wait there, the + # way a real RPC does: a mock that returns without suspending + # would finish the whole statement within that first step, and + # there would be nothing in flight to catch. + async def hang(req, timeout=None): + await asyncio.sleep(10) + + client = _async_client(ExecuteCypher=AsyncMock(side_effect=hang)) + bg = None + with pytest.raises(RuntimeError, match="in flight"): + async with client.transaction() as tx: + bg = asyncio.create_task(tx.cypher("CREATE (:A)")) + assert client._cypher_stub.CommitTransaction.await_count == 0, ( + "the block auto-committed while a queued statement was pending" + ) + bg.cancel() + with ctx_suppress(asyncio.CancelledError): + await bg + + asyncio.run(_inner()) + + +class TestCancellationAtTheSchedulingYieldCleansUp: + """Cancellation can land while the exit's scheduling yield is suspended + — after the block returned, before anything was decided. That path + leaves the try suite, so it must settle the handle itself instead of + walking away from an open transaction.""" + + def test_cancellation_during_the_yield_still_cleans_up(self): + async def _inner() -> None: + body_done = asyncio.Event() + holder: dict[str, object] = {} + + client = _async_client() + + async def run_block() -> None: + async with client.transaction() as tx: + holder["tx"] = tx + # Event.set() schedules this waiter via call_soon, so the + # test resumes BEFORE the exit's own sleep(0) + # continuation: the cancel below lands exactly on that + # suspended yield. + body_done.set() + + t = asyncio.create_task(run_block()) + await body_done.wait() + t.cancel() + with pytest.raises(asyncio.CancelledError): + await t + await client.close() # drains the detached cleanup + assert client._cypher_stub.RollbackTransaction.await_count == 1, ( + "a transaction cancelled at the scheduling yield was left open" + ) + + asyncio.run(_inner()) + + +class TestTransportCancelledCleanupKeepsTheBlockError: + """The exceptional exit's indeterminate cleanup follows the same rule as + every other cleanup: a CancelledError the transport raised, with no + cancellation pending, must not replace the block's own exception.""" + + def test_block_error_survives_a_spurious_cleanup_cancellation(self): + from contextlib import suppress as ctx_suppress + from unittest.mock import AsyncMock + + async def _inner() -> None: + client = _async_client( + CommitTransaction=AsyncMock(side_effect=_TransportError(grpc.StatusCode.DEADLINE_EXCEEDED)), + RollbackTransaction=AsyncMock(side_effect=asyncio.CancelledError()), + ) + with pytest.raises(RuntimeError, match="boom"): + async with client.transaction() as tx: + await tx.cypher("CREATE (:A)") + with ctx_suppress(grpc.RpcError): + await tx.commit() + raise RuntimeError("boom") + assert tx._state == "indeterminate" + + asyncio.run(_inner()) diff --git a/uv.lock b/uv.lock index 98fb435..58c89d2 100644 --- a/uv.lock +++ b/uv.lock @@ -586,7 +586,7 @@ provides-extras = ["dev"] [[package]] name = "coordinode-workspace" -version = "1.0.6" +version = "2.0.0" source = { virtual = "." } dependencies = [ { name = "googleapis-common-protos" },