Repository navigation
feat(ws): reliability bundle (#254, #257, #255, #256) - #285
Conversation
…tured frames resubscribe_one toggled `_stashing` around its unsubscribe+subscribe pair so data frames the server emitted during the sid handoff would be captured in `SubscriptionManager._stash` instead of being silently dropped. The drain path, however, was only wired into the full-reconnect path (`_handle_reconnect` -> `_drain_resubscribe_stash`). Single-sub gap recovery (`_handle_seq_gap` -> `resubscribe_one`) populated the stash but never drained it. Captured frames sat there until the next full reconnect: post-resubscribe deltas with seq>1 could hit the seq tracker before the snapshot they describe, mis-triggering another gap. Call the existing `_drain_resubscribe_stash` after a successful `resubscribe_one` so the new sid's stashed frames replay through `_process_frame` (preserving seq tracking + orderbook-apply order) and frames for other still-mapped sids that happened to land during the window also replay. Frames whose sid has no current mapping (the just- replaced old sid, or a sub that failed mid-cycle) are dropped with a debug log via the helper's existing branch. Failed resubscribe still early-returns after broadcast_error so we don't re-drain into a torn-down sub. Closes #254
…pty Orderbook After a gap-triggered ``OrderbookManager.remove_by_sid`` (or the stale-sid race in #255), ``OrderbookManager.get(ticker)`` returns None between snapshot teardown and the resync snapshot landing. The underlying delta stream can yield a frame during that window; ``_OrderbookIterator.__anext__`` previously masked this by returning a fresh ``Orderbook(ticker=ticker)``. A consumer reading bid/ask off that empty book sees zero-depth, indistinguishable from a real market with no liquidity, and could place orders against an empty book. Replace the silent-empty return with a typed ``KalshiOrderbookUnavailableError`` carrying the affected ticker. Strategies can ``except`` it explicitly to halt or wait for the next snapshot. Choice rationale: raising is preferred over silently skipping (continue) because: - The iterator's API contract ("each next() yields a fresh Orderbook for ticker") is preserved without a retry budget heuristic. - "no book" is a genuine error condition the consumer needs to know about — silently looping until a snapshot arrives could mask a permanent teardown. - The error is recoverable: the consumer can drop into a fresh iterator after the resync snapshot lands. New ``KalshiOrderbookUnavailableError`` is declared in ``kalshi/errors.py`` (subclass of ``KalshiWebSocketError`` with a ``ticker: str | None`` field), exported via ``kalshi.__all__``, and documented in ``docs/errors.md``. Closes #257
…rdown frames
Orderbook snapshots/deltas arriving with a sid that no longer maps to a
live subscription (server-initiated unsubscribe reaped the sid, or
``resubscribe_one`` tore down + reassigned a new sid) were still being
``model_validate``'d and applied to ``OrderbookManager`` before the
dispatcher's downstream sid check rejected them. The effects:
- A stale ``orderbook_snapshot`` re-seeds the manager under the old
sid index, recreating a book the gap handler just cleared.
- A stale ``orderbook_delta`` mutates the freshly-resynced book under
the same ticker (e.g. flipping prices on the new sid's snapshot).
Either produces inconsistent state that the consumer reads via
``get(ticker)`` until the next gap or reconnect.
Add an early sid-existence check in ``_process_frame`` for the two
orderbook message types: if the sid is present and ``_sub_mgr`` says
it has no current subscription, log debug and short-circuit before
seq tracking, validation, apply, or dispatch. Frames without an integer
sid (control envelopes, untracked channels) fall through to the
existing path.
Closes #255
…d/maxsize fields Consumer code iterating multiple subscribe queues sees `except KalshiBackpressureError as e` with only `str(e)` to discriminate which channel/sid/client_id overflowed. The error had to be parsed from the message string, which is brittle and breaks the structured-error contract #226 established for `KalshiSequenceGapError` and `KalshiSubscriptionError`. Add `__init__` with keyword-only `channel` / `sid` / `client_id` / `maxsize` fields on `KalshiBackpressureError`, mirroring the shape of `KalshiSequenceGapError`. Population path: - `MessageQueue` learns `channel` / `client_id` at construction (kwargs on `__init__`, defaults None for back-compat). The `put()` raise site fills `channel` / `client_id` / `maxsize` from the queue's own metadata so the error is fully structured at the point of failure. - `SubscriptionManager.subscribe()` threads the new subscription's `channel` + `client_id` into the default queue it constructs, so an overflow on a queue the SDK built has identity intact without caller intervention. - `SubscriptionManager.broadcast_error()` enriches `sid` from the subscription's current `server_sid` when the error is a `KalshiBackpressureError` with no sid set. Sid is intentionally NOT tracked by the queue (it's assigned at subscribe-ack and may change on resubscribe), so the broadcast site is the right place to attach it. Docs updated in `docs/errors.md`. Closes #256
Code Review — WS Reliability Bundle (#254, #257, #255, #256)OverviewThis PR bundles four targeted WebSocket reliability fixes that address real correctness hazards in the orderbook/subscription lifecycle. Each fix is well-scoped and doesn't reach beyond the problem it's solving. The overall approach is solid. What's Good#255 stale-frame gate placement is correct. #256 structured error fields are well-designed. #254 stash drain is wired to the right path only. Test coverage is thorough. Issues1. Issue numbers in source comments violate
|
| Area | Status |
|---|---|
| Correctness | ✅ All four fixes are logically sound |
| Test coverage | ✅ Thorough; one missing assertion (#2 above) |
| Style / conventions | |
| Docs | |
| Performance | ✅ No concerns |
| Security | ✅ No concerns |
The core changes are correct and safe to merge. Items #1 and #2 are the most worth addressing before merge — #1 for long-term maintainability, #2 for regression confidence.
Closes #254
Closes #257
Closes #255
Closes #256
Part of #273 (post-v2.4 audit), Wave 2.