Skip to content

Commit 25deea4

Browse files
committed
fix(haos): bound supervisor websocket sends
1 parent baabc93 commit 25deea4

3 files changed

Lines changed: 101 additions & 9 deletions

File tree

tests/haos_image_build/build_image.py

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import subprocess
2626
import sys
2727
import tempfile
28+
import threading
2829
import time
2930
import urllib.error
3031
import urllib.parse
@@ -762,6 +763,45 @@ def _wait_supervisor_api_ready(self, timeout: float = 60.0) -> None:
762763
f"within {timeout:.0f}s after Core restart (attempts={attempts})"
763764
) from last_error
764765

766+
def _send_with_deadline(
767+
self,
768+
message: str,
769+
*,
770+
deadline: float,
771+
operation: str,
772+
) -> None:
773+
"""Send one frame within a deadline, cancelling a stalled socket write."""
774+
connection = self._ws
775+
if connection is None:
776+
raise ConnectionError(f"WebSocket is not connected for {operation}")
777+
_remaining_deadline_budget(deadline, operation)
778+
send_errors: list[Exception] = []
779+
780+
def send() -> None:
781+
try:
782+
_remaining_deadline_budget(deadline, operation)
783+
connection.send(message)
784+
except Exception as exc:
785+
send_errors.append(exc)
786+
787+
worker = threading.Thread(target=send, name="haos-ws-send", daemon=True)
788+
worker.start()
789+
worker.join(max(0.0, deadline - time.monotonic()))
790+
if worker.is_alive():
791+
try:
792+
connection.close_socket()
793+
except (OSError, RuntimeError) as exc:
794+
LOG.debug("WS close error after stalled send: %r", exc)
795+
finally:
796+
if self._ws is connection:
797+
self._ws = None
798+
raise TimeoutError(
799+
f"{operation} exceeded its deadline after dispatch; "
800+
"command outcome is unknown"
801+
)
802+
if send_errors:
803+
raise send_errors[0]
804+
765805
def supervisor_api(
766806
self,
767807
endpoint: str,
@@ -790,11 +830,12 @@ def supervisor_api(
790830
}
791831
if data is not None:
792832
msg["data"] = data
793-
_remaining_deadline_budget(
794-
deadline,
795-
f"supervisor/api {method} {endpoint} send",
833+
message = json.dumps(msg)
834+
self._send_with_deadline(
835+
message,
836+
deadline=deadline,
837+
operation=f"supervisor/api {method} {endpoint} send",
796838
)
797-
self._ws.send(json.dumps(msg))
798839
# Skip any out-of-band messages (events on subscriptions etc.) and
799840
# match by id.
800841
while True:

tests/src/e2e/haos_only/test_zz_manage_app_tls.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,17 @@ async def test_manage_app_reproduces_legacy_tls_failure_then_uses_fix(
157157
MCPAssertions(mcp) as assertions,
158158
):
159159
slug = await _resolve_slug(mcp, NODERED_NAME)
160+
initial_detail = (
161+
await assertions.call_tool_success("ha_get_app", {"slug": slug})
162+
).get("addon") or {}
163+
if initial_detail.get("state") != "started":
164+
# Core's protocol restart can terminate Node-RED while its
165+
# Supervisor WebSocket proxy reconnects. Start it again so the
166+
# TLS assertions begin from a settled app state.
167+
await assertions.call_tool_success(
168+
"ha_manage_app",
169+
{"slug": slug, "action": "start"},
170+
)
160171
await _wait_addon_running(mcp, slug)
161172
detail = (
162173
await assertions.call_tool_success("ha_get_app", {"slug": slug})

tests/src/unit/test_haos_supervisor_wait.py

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
from __future__ import annotations
1212

1313
import json
14+
from threading import Event
1415
from typing import Any
1516
from unittest.mock import Mock, call, patch
1617

@@ -58,15 +59,15 @@ def test_supervisor_api_shares_receive_deadline_across_frames() -> None:
5859

5960
with patch(
6061
"tests.haos_image_build.build_image.time.monotonic",
61-
side_effect=[10.0, 11.0, 12.5],
62+
side_effect=[10.0, 10.5, 10.5, 10.5, 11.0, 12.5],
6263
):
6364
assert ws.supervisor_api("/supervisor/info", timeout=5.0) == {}
6465

6566
assert socket.recv.call_args_list == [call(timeout=4.0), call(timeout=2.5)]
6667

6768

68-
def test_supervisor_api_expired_deadline_does_not_dispatch() -> None:
69-
"""An expired command budget cannot produce an uncertain write outcome."""
69+
def test_supervisor_api_expiry_during_serialization_does_not_dispatch() -> None:
70+
"""Serialization time is charged before a command can be dispatched."""
7071
ws = HAWebSocket(
7172
"http://127.0.0.1:18123",
7273
OAuthCredentials(access_token="access", refresh_token="refresh"),
@@ -77,19 +78,58 @@ def test_supervisor_api_expired_deadline_does_not_dispatch() -> None:
7778
with (
7879
patch(
7980
"tests.haos_image_build.build_image.time.monotonic",
80-
side_effect=[10.0, 10.0],
81+
side_effect=[10.0, 12.0],
8182
),
83+
patch(
84+
"tests.haos_image_build.build_image.json.dumps",
85+
return_value='{"serialized": true}',
86+
) as serialize,
8287
pytest.raises(
8388
TimeoutError,
8489
match=r"supervisor/api post /core/update send exceeded its deadline",
8590
),
8691
):
87-
ws.supervisor_api("/core/update", method="post", data={}, timeout=0.0)
92+
ws.supervisor_api("/core/update", method="post", data={}, timeout=1.0)
8893

94+
serialize.assert_called_once()
8995
socket.send.assert_not_called()
9096
socket.recv.assert_not_called()
9197

9298

99+
def test_supervisor_api_stalled_send_is_bounded() -> None:
100+
"""A stalled socket write is cancelled within the command's deadline."""
101+
ws = HAWebSocket(
102+
"http://127.0.0.1:18123",
103+
OAuthCredentials(access_token="access", refresh_token="refresh"),
104+
)
105+
socket = Mock()
106+
release_send = Event()
107+
108+
def stalled_send(_: str) -> None:
109+
release_send.wait()
110+
111+
socket.send.side_effect = stalled_send
112+
socket.close_socket.side_effect = release_send.set
113+
ws._ws = socket
114+
115+
with (
116+
patch(
117+
"tests.haos_image_build.build_image.time.monotonic",
118+
return_value=10.0,
119+
),
120+
pytest.raises(
121+
TimeoutError,
122+
match="command outcome is unknown",
123+
),
124+
):
125+
ws.supervisor_api("/core/update", method="post", data={}, timeout=0.01)
126+
127+
socket.send.assert_called_once()
128+
socket.close_socket.assert_called_once()
129+
socket.recv.assert_not_called()
130+
assert ws._ws is None
131+
132+
93133
@pytest.mark.parametrize(
94134
("error", "expected_code", "expected_message"),
95135
[

0 commit comments

Comments
 (0)