Skip to content

Commit 2fe2365

Browse files
Warn on WelcomeResponse heartbeat_timeout=0 instead of silent accept
The WelcomeResponse docstring explicitly calls heartbeat_timeout=0 "semantically ambiguous" / "misconfigured peer or non-conforming server" — upstream config.c defaults to 15000ms and never emits 0, so a 0 from the wire indicates a peer bug. But the diagnostic content lived only in source comments; decode_body silently accepted the value, so operators running a dqlite cluster with a misconfigured peer got no signal to investigate. Emit a single logger.warning at decode time when heartbeat_timeout==0, naming the upstream default (15000ms) and the downstream consumer fallback ("falls back to static read_timeout floor"). Aligns with the in-tree ServersResponse.decode_body unknown_role_policy="warn" precedent. The permissive-accept contract is preserved — no DecodeError raised, the docstring's "is accepted (the wire layer does not enforce a minimum)" framing still holds. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent bf8f705 commit 2fe2365

2 files changed

Lines changed: 79 additions & 0 deletions

File tree

src/dqlitewire/messages/responses.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -609,6 +609,22 @@ def decode_body(cls, data: bytes, schema: int = 0) -> "WelcomeResponse":
609609
if len(data) != 8:
610610
raise DecodeError(f"WelcomeResponse body must be exactly 8 bytes, got {len(data)}")
611611
heartbeat_timeout = decode_uint64(data)
612+
if heartbeat_timeout == 0:
613+
# Surface the docstring's "misconfigured peer or non-
614+
# conforming server" diagnostic content into the log
615+
# stream. Upstream ``config.c`` defaults to 15000 and
616+
# never emits 0, so a 0 from the wire indicates a peer
617+
# bug; downstream consumers (``trust_server_heartbeat``)
618+
# fall back to the static read_timeout floor. Aligns
619+
# with the ``ServersResponse.decode_body`` warn-mode
620+
# precedent below. The permissive-accept contract is
621+
# preserved — no DecodeError raised.
622+
logger.warning(
623+
"WelcomeResponse: heartbeat_timeout=0 — peer is "
624+
"misconfigured or non-conforming (upstream config.c "
625+
"defaults to 15000ms and never emits 0); downstream "
626+
"consumers fall back to the static read_timeout floor"
627+
)
612628
return cls(heartbeat_timeout)
613629

614630

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
"""Pin: ``WelcomeResponse.decode_body`` emits a ``logger.warning``
2+
when ``heartbeat_timeout == 0``.
3+
4+
The docstring explicitly calls a zero heartbeat "semantically
5+
ambiguous" / "misconfigured peer or non-conforming server" — that
6+
diagnostic content used to live only in source comments, so operators
7+
running a dqlite cluster with a misconfigured peer got no log signal.
8+
9+
The wire layer keeps its permissive-accept contract (the decoder
10+
still returns the response; no DecodeError raised) but emits a
11+
single warning that surfaces the docstring's diagnostic content into
12+
the log stream. Aligns with the in-tree ``ServersResponse.decode_body``
13+
``unknown_role_policy="warn"`` precedent.
14+
"""
15+
16+
from __future__ import annotations
17+
18+
import logging
19+
20+
import pytest
21+
22+
from dqlitewire.messages.responses import WelcomeResponse
23+
from dqlitewire.types import encode_uint64
24+
25+
26+
def test_decode_body_zero_heartbeat_emits_warning(
27+
caplog: pytest.LogCaptureFixture,
28+
) -> None:
29+
"""A zero heartbeat is accepted but produces a single
30+
logger.warning at decode time."""
31+
body = encode_uint64(0)
32+
with caplog.at_level(logging.WARNING, logger="dqlitewire.messages.responses"):
33+
resp = WelcomeResponse.decode_body(body)
34+
assert resp.heartbeat_timeout == 0
35+
# Single warning emitted.
36+
warnings = [r for r in caplog.records if r.levelno == logging.WARNING]
37+
assert len(warnings) == 1
38+
msg = warnings[0].message
39+
# Diagnostic content from the docstring surfaces in the log line.
40+
assert "heartbeat_timeout=0" in msg or "heartbeat" in msg.lower()
41+
assert "15000" in msg or "non-conforming" in msg.lower() or "misconfig" in msg.lower()
42+
43+
44+
def test_decode_body_default_heartbeat_no_warning(
45+
caplog: pytest.LogCaptureFixture,
46+
) -> None:
47+
"""A legitimate heartbeat (e.g. 15000ms upstream default) does
48+
NOT trigger the warning."""
49+
body = encode_uint64(15000)
50+
with caplog.at_level(logging.WARNING, logger="dqlitewire.messages.responses"):
51+
resp = WelcomeResponse.decode_body(body)
52+
assert resp.heartbeat_timeout == 15000
53+
assert not [r for r in caplog.records if r.levelno == logging.WARNING]
54+
55+
56+
def test_decode_body_zero_heartbeat_is_still_accepted() -> None:
57+
"""The warning is observability-only — the decoder still returns
58+
a valid WelcomeResponse with heartbeat_timeout=0 (preserves the
59+
documented permissive-accept contract)."""
60+
body = encode_uint64(0)
61+
resp = WelcomeResponse.decode_body(body)
62+
assert resp.heartbeat_timeout == 0
63+
assert resp.heartbeat_timeout_seconds == 0.0

0 commit comments

Comments
 (0)