Skip to content

Commit 4c31cf0

Browse files
Narrow is_wire_coherent docstring to codec-layer poison only
The accessor reflects only MessageDecoder.is_poisoned, but the previous docstring's gloss ("wire desync from a prior parse failure") implied a broader contract that covers any wire-protocol violation. Client-layer ProtocolError raises (prepare db_id mismatch, _read_response extra-frame-after-FAILURE, _read_continuation unexpected EmptyResponse, interrupt drain caps, _drain_continuations caps) all leave the flag True at the moment of raise even though the wire IS desynchronised at the next-request boundary — the codec decoded correctly but the connection has unread frames buffered or a continuation expected. The in-tree path is masked: every ProtocolError routes through _run_protocol → _invalidate, which closes the writer and clears _protocol so the pool short-circuits on `protocol is None` before this accessor is ever read. Third-party harnesses on DqliteProtocol directly are NOT masked. Rewrite the docstring to scope the contract to codec-layer poison, enumerate the major client-layer raise sites that do NOT flip the flag, and direct third-party direct-DqliteProtocol harnesses to invalidate on any ProtocolError rather than reading this flag. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 94e1327 commit 4c31cf0

2 files changed

Lines changed: 84 additions & 18 deletions

File tree

src/dqliteclient/protocol.py

Lines changed: 41 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -178,29 +178,52 @@ def __reduce__(self) -> NoReturn:
178178

179179
@property
180180
def is_wire_coherent(self) -> bool:
181-
"""True if the decoder buffer has not been poisoned.
181+
"""True if the codec-layer decoder buffer has not been poisoned.
182182
183-
A poisoned buffer (mid-stream desync, malformed frame, etc.)
184-
cannot recover without ``reset()`` + reconnect. Pool reset
185-
consults this before sending ROLLBACK so a wasted round-trip
183+
Reflects ONLY the wire-codec layer (``MessageDecoder.is_poisoned``).
184+
A poisoned buffer (mid-stream wire desync, malformed frame, etc.)
185+
cannot recover without ``reset()`` + reconnect; the pool-reset
186+
path consults this before sending ROLLBACK so a wasted round-trip
186187
on an already-doomed connection is short-circuited.
187188
188-
INTENTIONALLY consulted only by the pool reset path
189-
(``pool.py::_socket_looks_dead``). The dbapi, the SA dialect,
190-
and direct ``DqliteConnection`` callers route a wire desync
191-
through the exception-based classifier chain instead:
192-
wire-layer ``ProtocolError`` is wrapped to
193-
``OperationalError(code=None)`` by
189+
Does NOT reflect client-layer ``ProtocolError`` raises that
190+
detect higher-level invariant violations on top of coherent
191+
bytes: ``prepare`` db_id mismatch, ``_read_response``
192+
extra-frame-after-FAILURE, ``_read_continuation`` unexpected
193+
EmptyResponse, ``interrupt`` drain wrong-type / no-progress /
194+
frame-cap, ``_drain_continuations`` max_total_rows /
195+
max_continuation_frames / no-progress. In those cases the codec
196+
decoded correctly but the connection still has unread frames
197+
buffered or a continuation expected; the wire IS desynchronised
198+
at the next-request boundary, but ``is_wire_coherent`` returns
199+
``True``.
200+
201+
This narrowness is intentional. ``is_wire_coherent`` is the
202+
codec-poison hint only; client-layer protocol violations route
203+
through ``DqliteConnection._run_protocol``'s
204+
``ProtocolError → _invalidate`` chain, which closes the writer
205+
and clears ``self._protocol`` — the pool short-circuits on
206+
``protocol is None`` before consulting this accessor at all
207+
(see ``pool.py::_socket_looks_dead``).
208+
209+
INTENTIONALLY consulted only by the pool reset path. The
210+
dbapi, the SA dialect, and direct ``DqliteConnection``
211+
callers route a wire desync through the exception-based
212+
classifier chain instead: wire-layer ``ProtocolError`` is
213+
wrapped to ``OperationalError(code=None)`` by
194214
``dqlitedbapi.cursor._call_client``, and the SA dialect's
195215
``is_disconnect`` substring branch matches the
196-
``"wire decode failed"`` prefix the client emits. That
197-
layering is deliberate — wire coherence is a hint, not a
198-
liveness contract: a ``CancelledError`` mid-flight could
199-
poison the buffer between this read and the next operation,
200-
so a ``True`` return MUST NOT be treated as "the connection
201-
is healthy". Use as a short-circuit optimisation only; do
202-
not propagate this check to ``do_ping`` / pre-checkout
203-
paths or to other operational classifiers.
216+
``"wire decode failed"`` prefix the client emits.
217+
218+
Wire coherence is a hint, not a liveness contract: a
219+
``CancelledError`` mid-flight could poison the buffer between
220+
this read and the next operation, so a ``True`` return MUST
221+
NOT be treated as "the connection is healthy". Use as a
222+
short-circuit optimisation only; do not propagate this check
223+
to ``do_ping`` / pre-checkout paths or to other operational
224+
classifiers. Third-party harnesses that consume
225+
``DqliteProtocol`` directly should always invalidate on any
226+
``ProtocolError`` rather than reading this flag.
204227
"""
205228
return not self._decoder.is_poisoned
206229

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
"""Pin: ``DqliteProtocol.is_wire_coherent`` docstring scopes the
2+
flag to codec-layer poison only and explicitly disclaims client-
3+
layer ProtocolError raises.
4+
5+
The accessor reflects only ``MessageDecoder.is_poisoned``; client-
6+
layer ProtocolError raises (db_id mismatch, extra-frame-after-FAILURE,
7+
unexpected EmptyResponse mid-ROWS, interrupt drain caps,
8+
_drain_continuations caps) leave it True at the moment of raise even
9+
though the wire IS desynchronised at the next-request boundary. The
10+
in-tree path is masked by _run_protocol's ProtocolError → _invalidate
11+
chain; third-party harnesses on DqliteProtocol directly are not
12+
masked.
13+
"""
14+
15+
from __future__ import annotations
16+
17+
from dqliteclient.protocol import DqliteProtocol
18+
19+
20+
def test_is_wire_coherent_docstring_calls_out_codec_layer_only() -> None:
21+
doc = DqliteProtocol.is_wire_coherent.__doc__ or ""
22+
assert "codec-layer" in doc.lower() or "codec layer" in doc.lower()
23+
assert "is_poisoned" in doc, "Docstring should name the underlying MessageDecoder.is_poisoned"
24+
25+
26+
def test_is_wire_coherent_docstring_disclaims_client_layer_protocolerror() -> None:
27+
"""The docstring must explicitly call out that the flag does NOT
28+
reflect client-layer ProtocolError raises."""
29+
doc = DqliteProtocol.is_wire_coherent.__doc__ or ""
30+
assert "Does NOT reflect" in doc or "does not reflect" in doc.lower(), (
31+
"Docstring must disclaim coverage of client-layer ProtocolError raises"
32+
)
33+
# The major sites the audit identified should be referenced so a
34+
# reader knows where to look for the contract drift.
35+
assert "_drain_continuations" in doc or "drain_continuations" in doc
36+
37+
38+
def test_is_wire_coherent_docstring_directs_third_party_harnesses_to_invalidate() -> None:
39+
"""Third-party direct DqliteProtocol harnesses must be told to
40+
invalidate on any ProtocolError rather than reading this flag."""
41+
doc = DqliteProtocol.is_wire_coherent.__doc__ or ""
42+
assert "third-party" in doc.lower() or "third party" in doc.lower()
43+
assert "invalidate" in doc.lower()

0 commit comments

Comments
 (0)