fix: treat WS connect transients (ConnectionRefusedError) as WARNING not ERROR (OPS-4738)

[palantir-valiot]

Summary

_new_conn in pygqlc/GraphQLClient.py:729 now logs WARNING (no traceback) for transient connect errors (OSError/TimeoutError, including ConnectionRefusedError) when websocket.create_connection fails to reach a GraphQL WS endpoint. Configuration problems (no environment, missing wss) remain ERROR. A regression test was added. This eliminates the recurring ERROR spam observed in lamosa-gto-services-workflows when the WS target is temporarily unreachable.

Why

The stack trace showed ConnectionRefusedError originating inside first-party pygqlc _new_conn (GraphQLClient.py:703) with no special handling, producing ERROR + full traceback on every reconnect attempt even though the client already returned False and retried. Triage marked this NOTIFY+FIX (high, code_bug). See Linear OPS-4738.

Base main advanced after the original PR was opened. Redundancy check (git diff origin/main...HEAD) confirmed the fix was not present; the original bug still reproduced on the advanced base. Performed rebase onto origin/main, resolved conflicts in CHANGELOG and __version__.py, bumped to 3.8.4, and force-with-lease pushed to the existing PR branch.

Test plan

• uv run pytest tests/pygqlc/gql_client/test_subscribe.py::test_new_conn_connection_refused_logs_warning_not_error (green)
• uvx ruff format --check (clean)
• uvx bandit -r . -s B101 -ll --exclude $(find . -type d -name '.venv' | paste -sd, -) (clean)
• uv lock --upgrade run; lock committed
• Version bumped to 3.8.4; CHANGELOG entry added at top
• Manual review of git diff origin/main...HEAD (no debug prints, no scope creep, no secrets)
• Branch name follows contract: palantir/OPS-4738-downgrade-ws-connect-error

Closes OPS-4738

[linear-code]

OPS-4738

[palantir-valiot]

Overall Assessment

PR downgrades transient WS connect errors (OSError/TimeoutError) from ERROR to WARNING in _new_conn. Preserves ERROR for config issues. Follows existing transient-error pattern. No blocking bugs.

Findings

No actionable findings.

Notes

• Change is consistent with TRANSIENT_WS_ERRORS precedent in _sub_routing_loop.
• New regression test isolates create_connection via patch and asserts WARNING (not ERROR).
• CHANGELOG, version, and lock updates follow AGENTS.md.

[palantir-valiot]

Review notes "No actionable findings" and the change aligns with the existing TRANSIENT_WS_ERRORS pattern. The only remaining items are pending "Analyze (python)" (CodeQL) jobs, which are not caused by this diff.

No further code changes needed on this PR.

[palantir-valiot]

Overall Assessment

The PR narrows the log level for transient WS connect failures (OSError/TimeoutError, including ConnectionRefusedError) inside _new_conn from ERROR to WARNING while leaving configuration errors (no environment, missing wss) at ERROR. The implementation is sound, the regression test follows existing patterns, and no blocking issues were identified.

Findings

No actionable findings.

Notes

• The change is isolated to the exception path in _new_conn (lines 729-737) and preserves the existing return False retry contract.
• The new test test_new_conn_connection_refused_logs_warning_not_error mirrors the style of the other _new_conn regression tests already present (e.g., test_new_conn_returns_false_when_wss_missing).
• The isinstance(exc, (OSError, TimeoutError)) guard correctly covers ConnectionRefusedError (a subclass of OSError) without needing an explicit case.
• No other call sites of _new_conn needed adjustment; callers already treat a False return as a recoverable failure.

[palantir-valiot]

Thanks for the review. No actionable findings to address. The notes align with the intent:

• change isolated to the transient exception path in _new_conn
• return False retry contract preserved
• test mirrors the other _new_conn regression tests
• isinstance(..., (OSError, TimeoutError)) covers ConnectionRefusedError as a subclass

Let me know if anything else is needed.

[palantir-valiot]

Overall Assessment

This PR downgrades transient WebSocket connection failures (ConnectionRefusedError, other OSError subclasses, TimeoutError) in _new_conn from ERROR to WARNING. Configuration errors (missing environment, missing wss) remain ERROR. The change is isolated, the test is hermetic, and follows the existing TRANSIENT_WS_ERRORS pattern. No blocking bugs found.

Findings

No actionable findings.

Notes

• Test coverage: the new regression test test_new_conn_connection_refused_logs_warning_not_error directly exercises the _new_conn path with a mocked ConnectionRefusedError and verifies both the log level and return value; existing tests for missing env/wss continue to assert ERROR.
• The implementation reuses the established isinstance(exc, (OSError, TimeoutError)) guard rather than introducing a new constant, which is consistent with the "simplest thing that works" guidance in AGENTS.md.
• Considered but not flagged: TimeoutError is a builtin (not socket.timeout), which is the correct type for websocket.create_connection timeouts; no version/syntax verification needed.

[palantir-valiot]

Thanks for the review — all notes noted.

• All required checks that have completed are green: tests, check_outdated, Palantir PR Reviewer, security/snyk.
• Current mergeable=blocked is only due to in-progress CodeQL "Analyze (python)" runs (no conclusion yet). The prior CodeQL run on this branch was SUCCESS.
• No code changes or test updates needed; the diff is minimal and the regression test is hermetic.

Ready when the analyzers finish.

[palantir-valiot]

Acknowledged — review notes "No actionable findings". All checks (tests, CodeQL, check_outdated, Palantir PR Reviewer) now SUCCESS. No further changes required.

Ready for merge.

[palantir-valiot]

⏸️ Awaiting human input

How to push the rebased OPS-4738 fix when push-safe always exits 3?

Reason: push-safe recovery rebases onto old PR branch tip (e83e5cb, behind main bb9f48e) and aborts with conflict on CHANGELOG.md; exit 3 per wrapper rules; plain/force push forbidden; local diff vs main is the needed change and test is green

Reply on this PR (or the linked Linear thread) to unblock the agent.

[palantir-valiot]

❌ Agent run failed

The implementer agent encountered a failure and has been parked
in the failed state. The PR remains open for human follow-up.

Reason: "opencode APIError: Forbidden: The OAuth2 access token could not be validated."

If this is a transient provider error (e.g. xAI capacity), the
automatic retry sweeper will revive the run within ~15 min.
Otherwise, reply on this PR or the Linear thread with context
and the run will resume.

[Eduardotrom]

Automated review summary

Verdict: approve · Risk: low

Downgrades transient WS connect failures in _new_conn (GraphQLClient.py:738-748) from ERROR (with traceback) to WARNING when the caught exception is an OSError/TimeoutError (covering ConnectionRefusedError, host-unreachable, socket timeouts). True configuration errors (no environment, unregistered environment, missing wss URL) are checked earlier and still log ERROR, and non-transport exceptions in the connect path remain ERROR. Adds a focused regression test and bumps version to 3.8.4. The change accomplishes its stated goal of eliminating recurring ERROR spam when a GraphQL WS endpoint is temporarily unreachable.

Recommendation

Safe to approve on code merits — the logic is correct, narrowly scoped, and covered by a regression test that matches existing conventions. Before merging, resolve the reported merge conflict (re-rebase onto current origin/main; CHANGELOG top entry and version bump to 3.8.4 are the likely conflict points) and confirm the new test passes on the updated base.

Findings

🟡 [minor] PR is in CONFLICTING merge state

• 📍 CHANGELOG.md, pygqlc/__version__.py
• gh reports mergeable=CONFLICTING (CHANGELOG.md and version.py per the PR body). The bot stated it rebased onto origin/main and force-with-lease pushed, but GitHub still shows conflicts. The branch needs to be re-synced with main before merge. This is a process/merge issue, not a code defect.

🔵 [nit] _conn_init() inside the same try can be downgraded too

• 📍 pygqlc/GraphQLClient.py:736-748
• self._conn_init() runs inside the same try block. If the TCP socket connects but _conn_init later raises an OSError (e.g. a send failing mid-handshake), it would also be logged at WARNING rather than ERROR. This is arguably acceptable (still a transient transport-layer failure) and does not change return behavior (still returns False), but it is a slightly broader scope than 'connect transients' and worth being aware of. Not blocking.

✅ [praise] Exception classification is correct and well-targeted

• 📍 pygqlc/GraphQLClient.py:744
• ConnectionRefusedError and socket.timeout are subclasses of OSError (timeout also of TimeoutError), so they correctly route to WARNING. websocket-client's WebSocketException/WebSocketBadStatusException inherit from Exception, NOT OSError, so handshake/protocol/bad-status failures correctly stay at ERROR. Config errors are validated before the try block, so they are unaffected.

✅ [praise] Regression test follows existing project conventions

• 📍 tests/pygqlc/gql_client/test_subscribe.py:369-399
• test_new_conn_connection_refused_logs_warning_not_error reuses the established _capture_logs() helper and Singleton reset pattern from adjacent transient-handling tests (OPS-3485). It asserts both that WARNING is logged and that no ERROR is emitted, plus that _new_conn returns False so the router survives.

---

Generated by an automated multi-agent review swarm and posted by @Eduardotrom. Verify load-bearing claims before merging.

[palantir-valiot]

❌ Agent run failed

The implementer agent encountered a failure and has been parked
in the failed state. The PR remains open for human follow-up.

Reason: "opencode APIError: Forbidden: You have run out of credits or need a Grok subscription. Add credits at https://grok.com/?_s=usage or upgrade at https://grok.com/supergrok."

If this is a transient provider error (e.g. xAI capacity), the
automatic retry sweeper will revive the run within ~15 min.
Otherwise, reply on this PR or the Linear thread with context
and the run will resume.