Skip to content

Commit 5211cbb

Browse files
committed
fix(server): drain exposure pending futures on device disconnect
unregister_exposure() popped the ExposureRegistration from hub.exposures but did not resolve pending invoke_exposure() futures. An in-flight POST /exposures/<id>/invoke therefore silently hung for the full 60-second asyncio.wait_for timeout before the caller received a 504, even though the exposure was already gone. Fix: capture the exp before popping, then set LookupError("exposure disconnected") on every unresolved pending future. The existing `except LookupError` handler in invoke_exposure_http already converts this to a prompt 404 — correctly describing the situation. Same class of disconnect-drain bug as the connection and publisher fixes.
1 parent 4e32bb8 commit 5211cbb

2 files changed

Lines changed: 95 additions & 1 deletion

File tree

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
"""ExposureRegistration.pending is drained on WS disconnect.
2+
3+
Pre-fix: unregister_exposure() popped the exposure from hub.exposures but
4+
did not fail pending invoke_exposure() futures. An in-flight
5+
`POST /exposures/<id>/invoke` silently waited the full 60-second timeout
6+
before asyncio.wait_for raised TimeoutError, returning 504 instead of 404.
7+
8+
Post-fix: unregister_exposure() captures the ExposureRegistration before
9+
popping it, then sets LookupError("exposure disconnected") on every unresolved
10+
pending future. The invoke_exposure_http handler's existing
11+
`except LookupError` catch then converts it to 404 — correctly describing
12+
the situation (exposure is gone, not merely slow).
13+
"""
14+
15+
import asyncio
16+
from dataclasses import dataclass, field
17+
18+
import pytest
19+
20+
from zhub.server import ExposureRegistration
21+
22+
23+
def _make_exp() -> ExposureRegistration:
24+
exp = ExposureRegistration.__new__(ExposureRegistration)
25+
exp.exposure_id = "ex_test"
26+
exp.name = "test-device"
27+
exp.websocket = None # type: ignore[assignment]
28+
exp.manifest = {"capabilities": [{"name": "ping"}]}
29+
exp.device_key_hash = "abc"
30+
exp.pending = {}
31+
import time
32+
exp.created_at = time.time()
33+
return exp
34+
35+
36+
def _run_drain(exp: ExposureRegistration) -> None:
37+
"""Mirrors the cleanup added to unregister_exposure."""
38+
err = LookupError("exposure disconnected")
39+
for fut in list(exp.pending.values()):
40+
if not fut.done():
41+
fut.set_exception(err)
42+
exp.pending.clear()
43+
44+
45+
@pytest.mark.asyncio
46+
async def test_pending_future_raises_on_disconnect():
47+
"""An in-flight invoke future must raise LookupError immediately on
48+
exposure disconnect, not hang for the full 60-second timeout."""
49+
exp = _make_exp()
50+
loop = asyncio.get_running_loop()
51+
fut = loop.create_future()
52+
exp.pending["req-1"] = fut
53+
54+
_run_drain(exp)
55+
56+
assert not exp.pending, "pending dict should be empty after drain"
57+
assert fut.done(), "future must be resolved (not hanging)"
58+
with pytest.raises(LookupError, match="disconnected"):
59+
fut.result()
60+
61+
62+
@pytest.mark.asyncio
63+
async def test_cleanup_skips_already_resolved_futures():
64+
"""Already-resolved futures must not raise InvalidStateError."""
65+
exp = _make_exp()
66+
loop = asyncio.get_running_loop()
67+
fut = loop.create_future()
68+
fut.set_result({"ok": True, "result": "pong"})
69+
exp.pending["req-2"] = fut
70+
71+
_run_drain(exp) # must not raise
72+
73+
assert fut.result() == {"ok": True, "result": "pong"}
74+
75+
76+
@pytest.mark.asyncio
77+
async def test_pending_is_empty_after_drain():
78+
"""All entries cleared regardless of how many pending futures existed."""
79+
exp = _make_exp()
80+
loop = asyncio.get_running_loop()
81+
for i in range(3):
82+
exp.pending[f"req-{i}"] = loop.create_future()
83+
84+
_run_drain(exp)
85+
86+
assert len(exp.pending) == 0

zhub/server.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -448,10 +448,18 @@ async def register_exposure(self, name: str, manifest: dict[str, Any],
448448

449449
async def unregister_exposure(self, exposure_id: str) -> None:
450450
async with self.lock:
451-
self.exposures.pop(exposure_id, None)
451+
exp = self.exposures.pop(exposure_id, None)
452452
for k, v in list(self.device_keys.items()):
453453
if v == exposure_id:
454454
self.device_keys.pop(k, None)
455+
if exp is not None:
456+
# Fail any in-flight invoke_exposure() calls so callers raise
457+
# immediately on device disconnect instead of hanging for 60 s.
458+
err = LookupError("exposure disconnected")
459+
for fut in list(exp.pending.values()):
460+
if not fut.done():
461+
fut.set_exception(err)
462+
exp.pending.clear()
455463
log.info("exposure unregistered: %s", exposure_id)
456464

457465
def find_exposure_by_capability(self, capability_name: str) -> Optional[str]:

0 commit comments

Comments
 (0)