-
-
Notifications
You must be signed in to change notification settings - Fork 225
Conversation
realfishsam
commented
Jun 16, 2026
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.pygit 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.mdMarket.questiondocs. - Python client-method generator wants to change
fetch_order_book(... params: Optional[FetchOrderBookParams])toOptional[dict]. - TypeScript client-method generator wants to change
fetchMatchedMarkets(params?: FetchMatchedMarketClustersParams)toparams?: 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
commented
Jun 16, 2026
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 -qcould not run because this runner lackspytest) - 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.
Summary
pmxt_internalartifacts in a focused checkoutFixes #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.pygit diff --check