Skip to content

Commit 70f4e44

Browse files
pucedotethclaude
andcommitted
Check single-subscription channels before queueing, not during replay
`userEvents` and `orderUpdates` cannot be multiplexed, and `subscribe` rejects a second one with `NotImplementedError`. That check only ran on the connected path, so subscribing twice before the socket opened was accepted, queued, and only rejected later while `on_open` replayed the queue. The exception then escapes inside the websocket callback, where the caller cannot catch it, and it aborts the replay loop. Every subscription queued behind the duplicate is silently dropped: ws_manager.subscribe({"type": "userEvents"}, cb) # queued ws_manager.subscribe({"type": "userEvents"}, cb) # queued, no error ws_manager.subscribe({"type": "l2Book", "coin": "ETH"}, cb) # on_open -> NotImplementedError on the second entry # frames sent to the server: 1 # l2Book:eth registered: False The same two calls after the socket is open raise at the call site, so identical user code either raises where it is written or loses an unrelated market data feed, depending only on connection timing. Run the check in `subscribe` for both paths, counting queued entries as well as active ones, so the duplicate is refused where it is requested. `on_open` now takes the queue before replaying it: `subscribe` consults that list, and leaving entries in place would also replay them again on a later `on_open`. Behaviour on the connected path is unchanged, and channels that do multiplex still accept several callbacks. Tests: `tests/websocket_manager_test.py` covers the duplicate on both paths, the dropped-subscription case, queue replay and clearing, and multiplexing. Against the unmodified file three of the five fail; the two that pass either way are the connected-path duplicate and the multiplexing case. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 2fdb18f commit 70f4e44

2 files changed

Lines changed: 90 additions & 6 deletions

File tree

hyperliquid/websocket_manager.py

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,11 @@ def on_message(self, _ws, message):
127127
def on_open(self, _ws):
128128
logging.debug("on_open")
129129
self.ws_ready = True
130-
for subscription, active_subscription in self.queued_subscriptions:
130+
# Drain the queue before replaying it: subscribe() consults it for the
131+
# single-subscription channels, and leaving entries behind would also
132+
# replay them again on a later on_open.
133+
queued_subscriptions, self.queued_subscriptions = self.queued_subscriptions, []
134+
for subscription, active_subscription in queued_subscriptions:
131135
self.subscribe(subscription, active_subscription.callback, active_subscription.subscription_id)
132136

133137
def subscribe(
@@ -136,16 +140,19 @@ def subscribe(
136140
if subscription_id is None:
137141
self.subscription_id_counter += 1
138142
subscription_id = self.subscription_id_counter
143+
identifier = subscription_to_identifier(subscription)
144+
if identifier == "userEvents" or identifier == "orderUpdates":
145+
# TODO: ideally the userEvent and orderUpdates messages would include the user so that we can multiplex
146+
# Queued subscriptions count too, otherwise the duplicate is only caught while on_open replays the
147+
# queue, where it aborts the replay and silently drops every subscription behind it.
148+
already_queued = any(subscription_to_identifier(s) == identifier for s, _ in self.queued_subscriptions)
149+
if len(self.active_subscriptions[identifier]) != 0 or already_queued:
150+
raise NotImplementedError(f"Cannot subscribe to {identifier} multiple times")
139151
if not self.ws_ready:
140152
logging.debug("enqueueing subscription")
141153
self.queued_subscriptions.append((subscription, ActiveSubscription(callback, subscription_id)))
142154
else:
143155
logging.debug("subscribing")
144-
identifier = subscription_to_identifier(subscription)
145-
if identifier == "userEvents" or identifier == "orderUpdates":
146-
# TODO: ideally the userEvent and orderUpdates messages would include the user so that we can multiplex
147-
if len(self.active_subscriptions[identifier]) != 0:
148-
raise NotImplementedError(f"Cannot subscribe to {identifier} multiple times")
149156
self.active_subscriptions[identifier].append(ActiveSubscription(callback, subscription_id))
150157
self.ws.send(json.dumps({"method": "subscribe", "subscription": subscription}))
151158
return subscription_id

tests/websocket_manager_test.py

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
import json
2+
from collections import defaultdict
3+
from types import SimpleNamespace
4+
5+
import pytest
6+
7+
from hyperliquid.websocket_manager import WebsocketManager
8+
9+
10+
def make_manager(ws_ready: bool):
11+
"""A WebsocketManager with a stub socket, so no connection is opened."""
12+
ws_manager = WebsocketManager.__new__(WebsocketManager)
13+
ws_manager.subscription_id_counter = 0
14+
ws_manager.ws_ready = ws_ready
15+
ws_manager.queued_subscriptions = []
16+
ws_manager.active_subscriptions = defaultdict(list)
17+
sent = []
18+
ws_manager.ws = SimpleNamespace(send=sent.append)
19+
return ws_manager, sent
20+
21+
22+
def callback(_msg):
23+
pass
24+
25+
26+
def test_duplicate_single_subscription_raises_while_queued():
27+
# userEvents and orderUpdates cannot be multiplexed. Before this was checked on
28+
# the queued path the duplicate was only caught later, inside on_open.
29+
ws_manager, _ = make_manager(ws_ready=False)
30+
ws_manager.subscribe({"type": "userEvents"}, callback)
31+
32+
with pytest.raises(NotImplementedError):
33+
ws_manager.subscribe({"type": "userEvents"}, callback)
34+
35+
36+
def test_duplicate_single_subscription_raises_when_connected():
37+
ws_manager, _ = make_manager(ws_ready=True)
38+
ws_manager.subscribe({"type": "orderUpdates"}, callback)
39+
40+
with pytest.raises(NotImplementedError):
41+
ws_manager.subscribe({"type": "orderUpdates"}, callback)
42+
43+
44+
def test_rejected_duplicate_does_not_drop_later_subscriptions():
45+
# The duplicate used to surface inside on_open, which aborted the replay and
46+
# silently dropped every subscription queued behind it.
47+
ws_manager, sent = make_manager(ws_ready=False)
48+
ws_manager.subscribe({"type": "userEvents"}, callback)
49+
with pytest.raises(NotImplementedError):
50+
ws_manager.subscribe({"type": "userEvents"}, callback)
51+
ws_manager.subscribe({"type": "l2Book", "coin": "ETH"}, callback)
52+
53+
ws_manager.on_open(None)
54+
55+
assert len(ws_manager.active_subscriptions["l2Book:eth"]) == 1
56+
assert [json.loads(msg)["subscription"]["type"] for msg in sent] == ["userEvents", "l2Book"]
57+
58+
59+
def test_on_open_replays_and_clears_the_queue():
60+
ws_manager, sent = make_manager(ws_ready=False)
61+
ws_manager.subscribe({"type": "l2Book", "coin": "BTC"}, callback)
62+
ws_manager.subscribe({"type": "trades", "coin": "ETH"}, callback)
63+
64+
ws_manager.on_open(None)
65+
66+
assert len(sent) == 2
67+
assert len(ws_manager.active_subscriptions["l2Book:btc"]) == 1
68+
assert len(ws_manager.active_subscriptions["trades:eth"]) == 1
69+
assert ws_manager.queued_subscriptions == []
70+
71+
72+
def test_multiplexable_channel_still_accepts_several_callbacks():
73+
ws_manager, _ = make_manager(ws_ready=True)
74+
ws_manager.subscribe({"type": "l2Book", "coin": "ETH"}, callback)
75+
ws_manager.subscribe({"type": "l2Book", "coin": "ETH"}, callback)
76+
77+
assert len(ws_manager.active_subscriptions["l2Book:eth"]) == 2

0 commit comments

Comments
 (0)