Skip to content

Commit 6e11316

Browse files
tmunzer-AIDEclaude
andcommitted
feat(device_utils): surface WebSocket errors on UtilResponse (closes #29)
UtilResponse gains ws_error / ws_close_code so a consumer can distinguish a clean WebSocket completion from an errored, abnormally-closed, or never-started one (previously the error was only logged and discarded): - on_error records the first transport error (was a discard-only lambda); - _on_close records the close code and flags a non-1000 abnormal close; - start_with_trigger records ws_error when the WS factory returns None or raises during setup. Additive and backward-compatible: ws_error stays None on a clean completion or a trigger-only command. Adds unit tests for each path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 49df2db commit 6e11316

2 files changed

Lines changed: 101 additions & 1 deletion

File tree

src/mistapi/device_utils/__tools/__ws_wrapper.py

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,13 @@ def __init__(
219219
self.ws_required: bool = False
220220
self.ws_data: list[str] = []
221221
self.ws_raw_events: list[str] = []
222+
# Set when the WebSocket leg did NOT complete cleanly (transport error,
223+
# abnormal close, or failure to start). Stays None on a clean completion or
224+
# a trigger-only command, so `ws_error is not None` is a reliable
225+
# "did not cleanly confirm" signal for consumers. ws_close_code records the
226+
# WebSocket close status code when one was received.
227+
self.ws_error: str | None = None
228+
self.ws_close_code: int | None = None
222229
self._queue: queue.Queue[str | None] = queue.Queue()
223230
self._closed = threading.Event()
224231
self._await_timeout: float | None = None
@@ -349,8 +356,22 @@ def _on_open(self):
349356
# Start the max duration timer
350357
self._timeout_handler(Timer.MAX_DURATION, TimerAction.START)
351358

359+
def _on_error(self, error):
360+
LOGGER.error("Error: %s", error)
361+
# Record the first WebSocket transport error so a consumer can tell an
362+
# errored stream from a clean one (otherwise the error is only logged).
363+
if self.util_response.ws_error is None:
364+
self.util_response.ws_error = str(error)
365+
352366
def _on_close(self, code, msg):
353367
LOGGER.info("WebSocket closed: %s - %s", code, msg)
368+
self.util_response.ws_close_code = code
369+
# An abnormal close (anything other than a normal 1000 / no-status close)
370+
# is recorded as an error so a clean completion is distinguishable.
371+
if code is not None and code != 1000 and self.util_response.ws_error is None:
372+
self.util_response.ws_error = (
373+
f"WebSocket closed abnormally (code {code}): {msg}"
374+
)
354375
self._stop_all_timers()
355376
self.util_response._queue.put(None) # sentinel for receive()
356377
self.util_response._closed.set() # signal completion
@@ -507,7 +528,7 @@ def start(self, ws) -> UtilResponse:
507528
"""
508529
self.ws = ws
509530
ws.on_message(self._handle_message)
510-
ws.on_error(lambda error: LOGGER.error("Error: %s", error))
531+
ws.on_error(self._on_error)
511532
ws.on_close(self._on_close)
512533
ws.on_open(self._on_open)
513534

@@ -564,6 +585,9 @@ def _run():
564585
self.start(ws)
565586
return # start() / _on_close manages _closed
566587
LOGGER.error("WS factory returned None")
588+
# The WebSocket leg was expected but never started; record it so
589+
# consumers don't mistake this for a clean trigger-only completion.
590+
self.util_response.ws_error = "WebSocket factory returned None"
567591
else:
568592
LOGGER.error(
569593
"Failed to trigger command: %s - %s",
@@ -572,6 +596,8 @@ def _run():
572596
)
573597
except Exception as e:
574598
LOGGER.error("Error during trigger: %s", e)
599+
if self.util_response.ws_error is None:
600+
self.util_response.ws_error = f"WebSocket setup error: {e}"
575601
# Mark done (failure or WS factory returned None)
576602
self.util_response._queue.put(None)
577603
self.util_response._closed.set()

tests/unit/test_ws_wrapper.py

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,3 +183,77 @@ def test_render_trims_trailing_spaces_and_empty_lines(self) -> None:
183183
rendered = s.render()
184184
assert rendered == "A"
185185
assert not rendered.endswith(" ")
186+
187+
188+
# ------------------------------------------------------------------
189+
# ws_error / ws_close_code (issue #29): let consumers distinguish a clean
190+
# WebSocket completion from an errored / abnormal / never-started one.
191+
# ------------------------------------------------------------------
192+
class TestUtilResponseWsErrorDefaults:
193+
def test_defaults_are_none(self) -> None:
194+
r = UtilResponse()
195+
assert r.ws_error is None
196+
assert r.ws_close_code is None
197+
198+
199+
class TestOnError:
200+
def test_records_first_error(self) -> None:
201+
w = _make_wrapper()
202+
w._on_error(Exception("boom"))
203+
assert w.util_response.ws_error == "boom"
204+
205+
def test_does_not_overwrite_existing(self) -> None:
206+
w = _make_wrapper()
207+
w._on_error(Exception("first"))
208+
w._on_error(Exception("second"))
209+
assert w.util_response.ws_error == "first"
210+
211+
212+
class TestOnClose:
213+
def test_normal_close_is_clean(self) -> None:
214+
w = _make_wrapper()
215+
w._on_close(1000, "")
216+
assert w.util_response.ws_close_code == 1000
217+
assert w.util_response.ws_error is None
218+
219+
def test_abnormal_close_sets_error(self) -> None:
220+
w = _make_wrapper()
221+
w._on_close(1006, "abnormal closure")
222+
assert w.util_response.ws_close_code == 1006
223+
assert "1006" in (w.util_response.ws_error or "")
224+
225+
def test_no_status_close_is_not_flagged(self) -> None:
226+
w = _make_wrapper()
227+
w._on_close(None, "")
228+
assert w.util_response.ws_close_code is None
229+
assert w.util_response.ws_error is None
230+
231+
232+
class TestStartWithTriggerWsError:
233+
"""A WS-backed command whose WebSocket never starts must record ws_error so it
234+
is not mistaken for a clean trigger-only completion."""
235+
236+
@staticmethod
237+
def _trigger(status: int = 200, data=None):
238+
return Mock(status_code=status, data=data if data is not None else {})
239+
240+
def test_ws_factory_returning_none_sets_ws_error(self) -> None:
241+
w = _make_wrapper()
242+
w.start_with_trigger(
243+
trigger_fn=lambda: self._trigger(),
244+
ws_factory_fn=lambda trigger: None,
245+
)
246+
w.util_response.wait(timeout=5)
247+
assert w.util_response.done
248+
assert w.util_response.ws_required is False # WS never started
249+
assert w.util_response.ws_error == "WebSocket factory returned None"
250+
251+
def test_ws_factory_raising_sets_ws_error(self) -> None:
252+
def boom(trigger):
253+
raise RuntimeError("factory kaboom")
254+
255+
w = _make_wrapper()
256+
w.start_with_trigger(trigger_fn=lambda: self._trigger(), ws_factory_fn=boom)
257+
w.util_response.wait(timeout=5)
258+
assert w.util_response.done
259+
assert "factory kaboom" in (w.util_response.ws_error or "")

0 commit comments

Comments
 (0)