From 2b4ce21b79459ec9ae5912aac1ff7022bc766693 Mon Sep 17 00:00:00 2001 From: matthew-pilot Date: Thu, 28 May 2026 03:35:19 +0000 Subject: [PATCH] fix(client): use PilotError type check instead of fragile substring match in subscribe_event (PILOT-187) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The subscribe_event loop used str(e) substring matching to detect clean disconnects: "connection closed" or "EOF". The "EOF" branch is too broad — any error message containing "EOF" (e.g. RuntimeError "unexpected EOF on stream", or a hypothetical "EOFError raised mid-frame") was silently swallowed even though the underlying error was not a clean disconnect. Replace the substring check with isinstance(e, PilotError) so only the library's own PilotError instances are treated as recoverable disconnects. The PilotError("connection closed") path (raised by the C-bound Conn class when the underlying Go connection is closed) is the legitimate clean-disconnect signal. The related timeout semantics (loop timeout vs per-read deadline) are documented in the existing docstring; no behavioral change there. Closes PILOT-187 --- pilotprotocol/client.py | 2 +- tests/test_services.py | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/pilotprotocol/client.py b/pilotprotocol/client.py index 2a52cf0..bc0caa9 100644 --- a/pilotprotocol/client.py +++ b/pilotprotocol/client.py @@ -1058,7 +1058,7 @@ def read_event(conn): else: yield (event_topic, event_data) except Exception as e: - if "connection closed" in str(e).lower() or "EOF" in str(e): + if isinstance(e, PilotError) and "connection closed" in str(e).lower(): break raise finally: diff --git a/tests/test_services.py b/tests/test_services.py index 02457d7..3f67de6 100644 --- a/tests/test_services.py +++ b/tests/test_services.py @@ -406,15 +406,18 @@ def read(self, size: int = 4096) -> bytes: assert events == [] assert conn.closed is True - def test_eof_error_breaks_cleanly(self, monkeypatch): + def test_eof_error_propagates(self, monkeypatch): + # RuntimeError containing "EOF" is NOT a clean disconnect — + # only PilotError("connection closed") should be silenced. class EofConn(FakeConn): def read(self, size: int = 4096) -> bytes: raise RuntimeError("unexpected EOF on stream") conn = EofConn() d = _make_driver_with_dial(monkeypatch, conn) - events = list(d.subscribe_event("0:0001.0000.0002", "t", timeout=2)) - assert events == [] + with pytest.raises(RuntimeError, match="unexpected EOF on stream"): + list(d.subscribe_event("0:0001.0000.0002", "t", timeout=2)) + assert conn.closed is True def test_other_exception_propagates(self, monkeypatch): # Error string contains neither "connection closed" nor "EOF" —