diff --git a/CHANGELOG.md b/CHANGELOG.md index 0065b77..bf0ab9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # CHANGELOG +## [3.8.4] - 2026-06-24 + +- [Fixed] `_ping_pong` send now classifies `ConnectionResetError` / `BrokenPipeError` / `ConnectionAbortedError` / `WebSocketConnectionClosedException` (the `TRANSIENT_WS_ERRORS`) at WARNING instead of ERROR+traceback. The background thread no longer emits noisy ERROR logs with full stacktraces on peer resets (the root cause of the reported "unhandled ConnectionResetError" during `_ping_pong`). Reconnect via `wss_conn_halted` is unchanged; only log level and noise. (OPS-3616) +- [Changed] Added deterministic regression tests for transient vs. fatal send errors inside `_ping_pong` (modeled exactly on the OPS-3485 recv tests; <100 ms, no sockets, no sleeps). (OPS-3616) + ## [3.8.3] - 2026-06-24 - [Fixed] `query`, `mutate`, `async_query`, and `async_mutate` no longer collapse a raised transport exception into `[{"message": ""}]`. httpx timeout and connection-reset exceptions carry an empty message (`str(httpx.ReadTimeout(""))` is `""`), so the previous `errors = [{"message": str(e)}]` surfaced a blank, undiagnosable error — most visibly on `async_mutate`, where async httpx timeouts always stringify to "". A new `exception_errors` helper falls back to `repr(error)` when the message is blank, preserving the exception class name (e.g. `ReadTimeout('')`) so the caller can tell what actually failed. Added an integration test that drives a real `GraphQLClient` against a local HTTP server stalled past `post_timeout`. @@ -14,6 +19,7 @@ - [Fixed] `async_cleanup` now actually closes a live async client. Its previous usability probe called `httpx.AsyncClient.get_timeout()`, a method that doesn't exist, so every client went down the "let GC handle it" branch and leaked. (Same broken probe in `_get_async_client` meant the cached client was recreated on every call; recreation now closes the old client first, so nothing leaks.) - [Changed] Removed the unused `_close_async_client` private method, superseded by `_drop_async_client`. + ## [3.8.0] - 2026-05-28 - [Fixed] `addEnvironment` no longer wipes an existing environment's `wss`/`url`/`headers`/`post_timeout`/`ipv4_only` when re-registering the same environment name without those arguments. Re-registration now MERGES — omitted arguments keep their previous values. Previously a second `addEnvironment("prod", url=..., headers=...)` on the shared (Singleton) client — e.g. a library configuring its own environment — silently reset `wss` to `None`, which crashed the WSS reconnect loop in `_new_conn` with `TypeError: argument of type 'NoneType' is not a container` and produced endless "Failed connecting to None" errors. This was the root cause of the production incident. (OPS-3496) diff --git a/pygqlc/GraphQLClient.py b/pygqlc/GraphQLClient.py index 801af56..4ea2e5c 100644 --- a/pygqlc/GraphQLClient.py +++ b/pygqlc/GraphQLClient.py @@ -812,10 +812,16 @@ def _ping_pong(self): # No need to log normal ping operations except Exception as e: if not self.closing: - log( - LogLevel.ERROR, - "error trying to send ping, WSS Pipe is broken", - ) + if isinstance(e, TRANSIENT_WS_ERRORS): + log( + LogLevel.WARNING, + "WSS connection reset or closed by peer", + ) + else: + log( + LogLevel.ERROR, + "error trying to send ping, WSS Pipe is broken", + ) self.wss_conn_halted = True def _registerSub(self, _id=None): diff --git a/pygqlc/__version__.py b/pygqlc/__version__.py index 429dcfe..9fd9cda 100644 --- a/pygqlc/__version__.py +++ b/pygqlc/__version__.py @@ -1 +1 @@ -__version__ = "3.8.3" +__version__ = "3.8.4" diff --git a/tests/pygqlc/gql_client/test_subscribe.py b/tests/pygqlc/gql_client/test_subscribe.py index 2697592..638b22e 100644 --- a/tests/pygqlc/gql_client/test_subscribe.py +++ b/tests/pygqlc/gql_client/test_subscribe.py @@ -145,6 +145,14 @@ def _stop(): return _stop +def _run_ping_pong(gql, timeout=1.0): + """Run _ping_pong in a daemon thread; fail (not hang) if it doesn't exit.""" + thread = threading.Thread(target=gql._ping_pong, daemon=True) + thread.start() + thread.join(timeout) + assert not thread.is_alive(), "ping pong loop did not terminate" + + @pytest.fixture def routing_client(): """A fresh GraphQLClient (bypassing the process-wide singleton cache) wired to a mock @@ -248,6 +256,77 @@ def _recv_then_close(): assert not any("invalid WSS message" in msg for level, msg in records) +def test_ping_pong_connection_reset_logged_as_warning(routing_client): + """OPS-3616: ConnectionResetError from send in _ping_pong is transient — log at + WARNING (not ERROR+traceback) and set wss_conn_halted to trigger reconnection. + The send error must be classified like recv transient errors (OPS-3485).""" + gql = routing_client + gql.pingIntervalTime = 0 + gql.pingTimer = 0 + + send_called = [0] + + def send_effect(data): + send_called[0] += 1 + if send_called[0] == 1: + raise ConnectionResetError(104, "Connection reset by peer") + return None + + gql._conn.send.side_effect = send_effect + + sleep_calls = [0] + + def sleepy(*args): + sleep_calls[0] += 1 + if sleep_calls[0] > 2: + gql.closing = True + return None + + with patch("time.sleep", side_effect=sleepy), _capture_logs() as records: + _run_ping_pong(gql) + + assert gql.wss_conn_halted is True + assert send_called[0] == 1 + assert any( + level == LogLevel.WARNING and "reset or closed by peer" in msg + for level, msg in records + ) + assert not any( + level == LogLevel.ERROR and "error trying to send ping" in msg + for level, msg in records + ) + + +def test_ping_pong_unexpected_error_logged_as_error(routing_client): + """A non-transient error from _ping_pong send must still surface at ERROR + level (and halt).""" + gql = routing_client + gql.pingIntervalTime = 0 + gql.pingTimer = 0 + + def send_effect(data): + raise ValueError("unexpected ping send failure") + + gql._conn.send.side_effect = send_effect + + sleep_calls = [0] + + def sleepy(*args): + sleep_calls[0] += 1 + if sleep_calls[0] > 2: + gql.closing = True + return None + + with patch("time.sleep", side_effect=sleepy), _capture_logs() as records: + _run_ping_pong(gql) + + assert gql.wss_conn_halted is True + assert any( + level == LogLevel.ERROR and "error trying to send ping" in msg + for level, msg in records + ) + + def test_addenvironment_reregister_preserves_wss(): """OPS-3496: re-registering an existing environment without `wss` (as a library configuring the shared Singleton client does) must NOT wipe the previously-set