Drop outer _truncate_error wrap on handshake FailureResponse rendering

The handshake FailureResponse arm in protocol.py composed:

    raise ProtocolError(
        f"Handshake failed: [{response.code}] "
        f"{_truncate_error(self._failure_text(response))}"
    )

The outer _truncate_error wrapped a string that _failure_text had
already pre-truncated (body capped at 200, then addr suffix appended).
With a long FailureResponse.message, the outer call saw the result as
oversized again and re-truncated to 200 chars — silently stripping the
" to host:port" addr suffix the inner pre-truncation just preserved,
and reporting a confused "[truncated, N chars]" marker against the
post-inner-truncation length rather than the original server-message
length.

Drop the outer wrap. _failure_text already bounds the result at
~240 chars, and the handshake ProtocolError is a per-peer diagnostic
(not an aggregate) where peer attribution is precisely what the
operator paged on needs. Pin the composition shape and the addr-suffix
preservation against regression — including a source-level guard that
prevents re-introduction of the outer wrap.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: antoineleclair

src/dqliteclient/protocol.py

@@ -233,17 +233,14 @@ async def handshake(self, client_id: int | None = None) -> int:
             # server-reported code and peer address so log aggregators
             # can group on the numeric code (DQLITE_PARSE,
             # DQLITE_NOTLEADER, etc.) rather than on text alone.
-            # Truncate the failure body so a hostile-or-buggy peer
-            # cannot inflate the handshake error into a multi-KiB
-            # log line — the rest of the protocol module's
-            # OperationalError raise sites get this for free via
-            # ``OperationalError._MAX_DISPLAY_MESSAGE``; the raw
-            # ProtocolError shape doesn't.
-            from dqliteclient.cluster import _truncate_error
-
+            # ``_failure_text`` already pre-truncates the body before
+            # appending the addr suffix, bounding the result at
+            # ~240 chars; do NOT wrap it in an outer ``_truncate_error``,
+            # which would re-truncate over the addr suffix and silently
+            # strip the peer attribution exactly when the operator
+            # needs it most (a long handshake failure).
             raise ProtocolError(
-                f"Handshake failed: [{response.code}] "
-                f"{_truncate_error(self._failure_text(response))}"
+                f"Handshake failed: [{response.code}] {self._failure_text(response)}"
             )
 
         if not isinstance(response, WelcomeResponse):

tests/test_handshake_failure_addr_suffix_preserved.py

@@ -0,0 +1,82 @@
+"""Pin: handshake-time ``ProtocolError`` raised on a ``FailureResponse``
+preserves the peer-address attribution suffix even when the server
+message is long.
+
+``DqliteProtocol._failure_text`` pre-truncates the body before
+appending the addr suffix; the handshake raise site MUST NOT wrap the
+result in an outer truncator that would re-truncate over the suffix
+and silently strip the peer attribution. The most-likely-paged class
+of handshake error (a flaky / mis-configured peer) is exactly the one
+that produces a long FailureResponse body — losing the suffix exactly
+when an operator needs to know "which node?" defeats the purpose of
+the suffix.
+"""
+
+from __future__ import annotations
+
+from dqliteclient.exceptions import ProtocolError
+from dqliteclient.protocol import DqliteProtocol
+from dqlitewire.messages import FailureResponse
+
+
+def _make_proto_with_address(address: str) -> DqliteProtocol:
+    proto = DqliteProtocol.__new__(DqliteProtocol)
+    proto._address = address
+    return proto
+
+
+def test_handshake_protocolerror_composition_preserves_addr_suffix() -> None:
+    """The production handshake raise site composes:
+
+        ProtocolError(f"Handshake failed: [{code}] {self._failure_text(response)}")
+
+    With a long FailureResponse.message, the result must end with the
+    addr suffix that ``_failure_text`` appends — i.e. NO outer
+    truncator can wrap _failure_text and strip the suffix.
+    """
+    proto = _make_proto_with_address("host-a:9001")
+    body = "X" * 1024
+    response = FailureResponse(code=1001, message=body)
+
+    rendered = f"Handshake failed: [{response.code}] {proto._failure_text(response)}"
+
+    # The ProtocolError text the operator sees must end with the
+    # addr suffix — that is the load-bearing peer attribution.
+    err = ProtocolError(rendered)
+    text = str(err)
+    assert text.endswith(" to host-a:9001"), f"Addr suffix dropped from handshake error: {text!r}"
+    # Defensive cap on the body still applies.
+    assert "[truncated," in text
+
+
+def test_handshake_protocolerror_composition_short_message_no_truncation() -> None:
+    """Short messages do not need truncation; the suffix is still
+    appended verbatim. Pin that the no-truncation path also keeps
+    the suffix (defensive against an over-eager outer truncator
+    being re-introduced)."""
+    proto = _make_proto_with_address("host-b:19002")
+    response = FailureResponse(code=1001, message="boom")
+    rendered = f"Handshake failed: [{response.code}] {proto._failure_text(response)}"
+    text = str(ProtocolError(rendered))
+    assert text.endswith(" to host-b:19002")
+    assert "[truncated," not in text
+
+
+def test_handshake_protocolerror_does_not_use_outer_truncate_error() -> None:
+    """Source-level pin: the handshake raise site at protocol.py
+    must NOT wrap _failure_text in _truncate_error — that wrapping
+    silently strips the addr suffix the inner pre-truncation just
+    preserved. This guard catches a regression that would re-introduce
+    the outer wrap.
+    """
+    import inspect
+
+    from dqliteclient import protocol
+
+    # Locate the _handshake method and read its source.
+    src = inspect.getsource(protocol.DqliteProtocol.handshake)
+    # The handshake's FailureResponse arm must not call
+    # _truncate_error on top of self._failure_text.
+    assert "_truncate_error(self._failure_text" not in src, (
+        "Handshake raise site must not wrap _failure_text in _truncate_error"
+    )