Skip to content

fix(python-sdk): log websocket cleanup failures#1127

Open
realfishsam wants to merge 1 commit into
mainfrom
fix/ws-client-close-logs-error
Open

fix(python-sdk): log websocket cleanup failures#1127
realfishsam wants to merge 1 commit into
mainfrom
fix/ws-client-close-logs-error

Conversation

@realfishsam

Copy link
Copy Markdown
Contributor

Summary

  • Logs websocket cleanup failures during failed connection retries instead of silently swallowing them
  • Keeps the original handshake failure behavior unchanged
  • Adjusts the focused Python websocket tests so they can run without generated pmxt_internal artifacts in a focused checkout

Fixes #1106

Test Plan

  • cd sdks/python && /opt/hermes/.venv/bin/python -m pytest tests/test_ws_client.py -q
  • /opt/hermes/.venv/bin/python -m py_compile sdks/python/pmxt/ws_client.py sdks/python/tests/test_ws_client.py
  • git diff --check

@realfishsam

Copy link
Copy Markdown
Contributor Author

CI note from autonomous fixer:

Targeted local checks for the scoped change are green:

  • cd sdks/python && /opt/hermes/.venv/bin/python -m pytest tests/test_ws_client.py -q → 7 passed
  • /opt/hermes/.venv/bin/python -m py_compile sdks/python/pmxt/ws_client.py sdks/python/tests/test_ws_client.py
  • git diff --check

Required generated-sync checks are currently red from pre-existing/unrelated generator drift on this base, not from this websocket change:

  • API reference generator wants to remove sdks/typescript/API_REFERENCE.md Market.question docs.
  • Python client-method generator wants to change fetch_order_book(... params: Optional[FetchOrderBookParams]) to Optional[dict].
  • TypeScript client-method generator wants to change fetchMatchedMarkets(params?: FetchMatchedMarketClustersParams) to params?: any.

Per PMXT focused-PR generated-drift policy, I am keeping this PR scoped to #1106 and not folding broad SDK/API-reference regeneration into it.

@realfishsam

Copy link
Copy Markdown
Contributor Author

PR Review: PASS (NOT VERIFIED)

What This Does

Logs WebSocket cleanup failures during retry after a failed connection attempt instead of silently swallowing ws.close() errors. This improves SDK diagnostics while preserving the original retry/final-error behavior for consumers.

Blast Radius

Python SDK WebSocket client cleanup path and its unit tests only. No sidecar API contract, venue exchange logic, generated SDK files, or field propagation changes.

Consumer Verification

Before (base branch):
SidecarWsClient._ensure_connected() caught exceptions from ws.close() after a failed handshake and did pass, so a consumer only saw the eventual connection failure and had no visibility that cleanup also failed.

try:
    ws.close()
except Exception:
    pass

After (PR branch):
A mocked consumer-path check forced _connect_websocket() to raise OSError("handshake failed") and ws.close() to raise RuntimeError("close failed"). The final exception remained OSError: handshake failed, and the logger emitted the cleanup diagnostic with traceback:

OSError handshake failed
DEBUG:Failed to close unsuccessful WebSocket connection
... RuntimeError: close failed

This verifies the cleanup failure is now observable without changing the retry outcome.

Test Results

  • Build: PASS (npm run build --workspace=pmxt-core)
  • Unit tests: NOT VERIFIED (python3 -m pytest sdks/python/tests/test_ws_client.py -q could not run because this runner lacks pytest)
  • Server starts: N/A (SDK WebSocket client-only change)
  • E2E smoke: PASS for the mocked WebSocket cleanup path described above

Findings

No blocking findings.

PMXT Pipeline Check

  • Field propagation (3-layer): N/A
  • OpenAPI sync: N/A
  • Financial precision: N/A
  • Type safety: OK
  • Auth safety: N/A

Semver Impact

patch -- diagnostic-only bug fix with unchanged public API and unchanged final failure semantics.

Risk

I could not run the pytest suite in this environment because pytest is not installed, so this is marked PASS (NOT VERIFIED) rather than VERIFIED. The targeted mocked check covers the changed cleanup branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

empty-catch: sdks/python/pmxt/ws_client.py:132 — bare except Exception: pass during WebSocket close

1 participant