Skip to content

Commit 995770b

Browse files
tmunzer-AIDEclaude
andauthored
Surface WebSocket errors on UtilResponse (closes #29) (#31)
* 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> * docs(changelog): note ws_error/ws_close_code on UtilResponse (#29) * chore(release): bump version to 0.63.1 * refactor(device_utils): address Copilot review on ws_error capture - Guard the WS-factory-None ws_error assignment (first-write-wins, matching _on_error / _on_close). - Neutralize the except-path ws_error message ('trigger/WebSocket setup error') since that except also wraps trigger_fn(), not only WS setup. --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 49df2db commit 995770b

5 files changed

Lines changed: 127 additions & 3 deletions

File tree

CHANGELOG.md

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,24 @@
11
# CHANGELOG
2+
## Version 0.63.1 (June 2026)
3+
4+
**Released**: June 14, 2026
5+
6+
This patch release lets callers of the device-utility helpers detect WebSocket failures that were previously only logged.
7+
8+
---
9+
10+
### 1. BUG FIXES
11+
12+
#### **Device Utilities: surface WebSocket errors on `UtilResponse`**
13+
`UtilResponse` now exposes `ws_error` and `ws_close_code` so callers can distinguish a clean WebSocket completion from an errored, abnormally-closed, or never-started one. Previously a WebSocket transport error was only logged and discarded, and a WS-backed command whose WebSocket failed to start was indistinguishable from a healthy trigger-only command.
14+
- `_on_error` records the first transport error (was a discard-only callback).
15+
- `_on_close` records the close status code and flags a non-1000 abnormal close.
16+
- `start_with_trigger` records `ws_error` when the WebSocket factory returns `None` or raises during setup.
17+
18+
Additive and backward-compatible: `ws_error` stays `None` on a clean completion or a trigger-only command. (#29)
19+
20+
---
21+
222
## Version 0.63.0 (June 2026)
323

424
**Released**: June 12, 2026

pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ build-backend = "hatchling.build"
44

55
[project]
66
name = "mistapi"
7-
version = "0.63.0"
7+
version = "0.63.1"
88
authors = [{ name = "Thomas Munzer", email = "tmunzer@juniper.net" }]
99
description = "Python package to simplify the Mist System APIs usage"
1010
keywords = ["Mist", "Juniper", "API"]

src/mistapi/__version.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
__version__ = "0.63.0"
1+
__version__ = "0.63.1"
22
__author__ = "Thomas Munzer <tmunzer@juniper.net>"

src/mistapi/device_utils/__tools/__ws_wrapper.py

Lines changed: 31 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,11 @@ 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+
# Guarded (first-write-wins) to match _on_error / _on_close.
591+
if self.util_response.ws_error is None:
592+
self.util_response.ws_error = "WebSocket factory returned None"
567593
else:
568594
LOGGER.error(
569595
"Failed to trigger command: %s - %s",
@@ -572,6 +598,10 @@ def _run():
572598
)
573599
except Exception as e:
574600
LOGGER.error("Error during trigger: %s", e)
601+
# This except also wraps trigger_fn(), so the message stays neutral
602+
# rather than implying the failure was always in WebSocket setup.
603+
if self.util_response.ws_error is None:
604+
self.util_response.ws_error = f"trigger/WebSocket setup error: {e}"
575605
# Mark done (failure or WS factory returned None)
576606
self.util_response._queue.put(None)
577607
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)