Fix _ping_pong WS send for ConnectionResetError (OPS-4640)#73
Closed
palantir-valiot[bot] wants to merge 1 commit into
Closed
Fix _ping_pong WS send for ConnectionResetError (OPS-4640)#73palantir-valiot[bot] wants to merge 1 commit into
palantir-valiot[bot] wants to merge 1 commit into
Conversation
….) — log WARNING, set wss_conn_halted, trigger reconnect. TDD: two new hermetic tests (transient vs unexpected) written first, red for correct reason, now green. Bumped patch + CHANGELOG. (OPS-4640)
Contributor
Author
|
:robot_face: Closing as a duplicate — another Palantir PR is already fixing the same error: #68. This PR's Linear issue is being canceled. |
palantir-valiot
Bot
deleted the
palantir/OPS-4640-ping-pong-connection-reset
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Handle transient websocket send errors (ConnectionResetError, BrokenPipeError, etc.) in
_ping_pongthe same way_sub_routing_loopalready handles them on recv: log at WARNING (no traceback), setwss_conn_halted=True, and let the router trigger reconnection. Non-transient errors continue to surface at ERROR. This eliminates the noisy ERROR+traceback observed in production when the WSS pipe is reset by the peer during a ping.Files changed:
pygqlc/GraphQLClient.py:782— classify send exceptions using the existingTRANSIENT_WS_ERRORStuple; WARNING vs ERROR path.tests/pygqlc/gql_client/test_subscribe.py— two new hermetic tests (modeled on the OPS-3485/OPS-3496 routing tests):test_ping_pong_connection_reset_logged_as_warningandtest_ping_pong_unexpected_error_logged_as_error. Written first (TDD), initially red for the correct reason, now green.pygqlc/__version__.py— 3.8.1 → 3.8.2 (patch).CHANGELOG.md— top entry for 3.8.2.uv.lock—uv lock --upgradeper AGENTS.md (cryptography 48→49, pytest 9.0.3→9.1.0).Why
Linear: OPS-4640 (lamosa-gto-services-workflows). First occurrence but a clear code bug per the GraphQL WS protocol: the ping thread must tolerate a broken pipe and let the existing reconnect machinery recover instead of spamming ERROR logs and leaving the client in a half-dead state.
Test plan
uv run pytest -q(39 unit/hermetic tests green; the 25 live-gql errors are expected — they require real API/WSS/TOKEN env vars and are unrelated to this change; the two new ping_pong tests specifically exercise the fix paths).uvx ruff format --check— clean.uvx bandit -r . -s B101 -ll --exclude $(find . -type d -name '.venv' | paste -sd, -)— clean (the two B104 suppressions are pre-existing and justified in the IPv4 egress code).uv lock --upgradecommitted (per AGENTS.md +python-check-outdated.yaml).git diff HEAD~1: only the intended fix + tests + release metadata; no debug prints, no scope creep, no secrets.palantir/OPS-4640-ping-pong-connection-reset(exact convention).Closes OPS-4640