fix: downgrade BrokenPipeError ping log from ERROR to WARNING#54
Conversation
There was a problem hiding this comment.
Review
Functional Change
The downgrade of BrokenPipeError (and OSError) ping logs from ERROR to WARNING is correct and appropriate. It aligns with the existing _unsubscribe pattern and correctly reduces alert noise for an expected, self-healing failure path. Other unexpected exceptions continue to log at ERROR, which is the right behavior.
Issue Found
| Severity | Finding |
|---|---|
| Minor | Docstring bracket imbalance — the "fix" on line 7 adds an extra ], changing ]] to ]]]. The original was already balanced (3 [ and 3 ]), so the change introduces a syntax error in the docstring. |
Minor Note
Catching OSError in _ping_pong is slightly broader than the existing _unsubscribe method (which catches only BrokenPipeError). In practice this is unlikely to cause issues since self._conn.send() is not expected to raise non-connection OSError subclasses, but it is a minor inconsistency with the existing pattern cited in the PR description.
Recommendation
Revert the docstring bracket change (remove the extra ]) before merging.
|
|
||
| GQLResponse (type variable): [data[field(string)], errors[message(string), | ||
| field?(string)] | ||
| field?(string)]] |
There was a problem hiding this comment.
This docstring change introduces an extra ]. The original ]] was balanced — [data[field(string)], errors[message(string), field?(string)]] has 3 opening and 3 closing brackets. The new ]]] has 4 closing brackets, making it unbalanced.
Automated Fix Proposal
🟢 High confidence
Bug
Recurring
BrokenPipeErrorinpygqlcWebSocket ping thread (_ping_pong). When the WSS pipe breaks during a ping, the code catches the exception, setswss_conn_halted = True, and the connection self-heals via the reconnection loop in_sub_routing_loop. However, it logs at ERROR level, creating alert noise.Fix
Catch
BrokenPipeError/OSErrorexplicitly in_ping_pongand log at WARNING level, consistent with the existing_unsubscribemethod which already handles the same exception at WARNING:The
_ping_pongmethod now follows the same pattern:Other unexpected exceptions continue to log at ERROR.
Error context
lamosa-gto-services-workflows(lamosa-gto:prod)97878975pygqlc/GraphQLClient.py:723→self._conn.send(PING_JSON)→BrokenPipeError: [Errno 32] Broken pipeOriginal Error