diff --git a/src/dashboard/routes/world.py b/src/dashboard/routes/world.py index 7321edec..3611a0db 100644 --- a/src/dashboard/routes/world.py +++ b/src/dashboard/routes/world.py @@ -151,14 +151,8 @@ async def world_ws(websocket: WebSocket) -> None: logger.info("World WS disconnected — %d clients", len(_ws_clients)) -async def broadcast_world_state(presence: dict) -> None: - """Broadcast a ``timmy_state`` message to all connected Workshop clients. - - Called by :class:`~timmy.workshop_state.WorkshopHeartbeat` via its - ``on_change`` callback. - """ - state = _build_world_state(presence) - message = json.dumps({"type": "timmy_state", **state["timmyState"]}) +async def _broadcast(message: str) -> None: + """Send *message* to every connected Workshop client, pruning dead ones.""" dead: list[WebSocket] = [] for ws in _ws_clients: try: @@ -170,6 +164,16 @@ async def broadcast_world_state(presence: dict) -> None: _ws_clients.remove(ws) +async def broadcast_world_state(presence: dict) -> None: + """Broadcast a ``timmy_state`` message to all connected Workshop clients. + + Called by :class:`~timmy.workshop_state.WorkshopHeartbeat` via its + ``on_change`` callback. + """ + state = _build_world_state(presence) + await _broadcast(json.dumps({"type": "timmy_state", **state["timmyState"]})) + + # --------------------------------------------------------------------------- # Visitor chat — bark engine # --------------------------------------------------------------------------- @@ -185,26 +189,35 @@ async def _handle_client_message(raw: str) -> None: if data.get("type") == "visitor_message": text = (data.get("text") or "").strip() if text: - asyncio.create_task(_bark_and_broadcast(text)) + task = asyncio.create_task(_bark_and_broadcast(text)) + task.add_done_callback(_log_bark_failure) + + +def _log_bark_failure(task: asyncio.Task) -> None: + """Log unhandled exceptions from fire-and-forget bark tasks.""" + if task.cancelled(): + return + exc = task.exception() + if exc is not None: + logger.error("Bark task failed: %s", exc) async def _bark_and_broadcast(visitor_text: str) -> None: """Generate a bark response and broadcast it to all Workshop clients.""" - # Signal "thinking" state - await _broadcast_speech({"type": "timmy_thinking"}) + await _broadcast(json.dumps({"type": "timmy_thinking"})) reply = await _generate_bark(visitor_text) - # Store exchange in conversation buffer _conversation.append({"visitor": visitor_text, "timmy": reply}) - # Broadcast speech bubble + conversation history - await _broadcast_speech( - { - "type": "timmy_speech", - "text": reply, - "recentExchanges": list(_conversation), - } + await _broadcast( + json.dumps( + { + "type": "timmy_speech", + "text": reply, + "recentExchanges": list(_conversation), + } + ) ) @@ -222,17 +235,3 @@ async def _generate_bark(visitor_text: str) -> str: except Exception as exc: logger.warning("Bark generation failed: %s", exc) return "Hmm, my thoughts are a bit tangled right now." - - -async def _broadcast_speech(payload: dict) -> None: - """Broadcast a speech message to all connected Workshop clients.""" - message = json.dumps(payload) - dead: list[WebSocket] = [] - for ws in _ws_clients: - try: - await ws.send_text(message) - except Exception: - dead.append(ws) - for ws in dead: - if ws in _ws_clients: - _ws_clients.remove(ws) diff --git a/tests/dashboard/test_world_api.py b/tests/dashboard/test_world_api.py index 4037ee3d..ea46fac1 100644 --- a/tests/dashboard/test_world_api.py +++ b/tests/dashboard/test_world_api.py @@ -1,19 +1,21 @@ """Tests for GET /api/world/state endpoint and /api/world/ws relay.""" import json +import logging import time -from unittest.mock import AsyncMock, patch +from unittest.mock import AsyncMock, MagicMock, patch import pytest from dashboard.routes.world import ( _STALE_THRESHOLD, _bark_and_broadcast, - _broadcast_speech, + _broadcast, _build_world_state, _conversation, _generate_bark, _handle_client_message, + _log_bark_failure, _read_presence_file, broadcast_world_state, ) @@ -331,15 +333,15 @@ async def test_bark_and_broadcast_sends_thinking_then_speech(): @pytest.mark.asyncio -async def test_broadcast_speech_removes_dead_clients(): - """Dead clients are cleaned up during speech broadcast.""" +async def test_broadcast_removes_dead_clients(): + """Dead clients are cleaned up during broadcast.""" from dashboard.routes.world import _ws_clients dead = AsyncMock() dead.send_text.side_effect = ConnectionError("gone") _ws_clients.append(dead) try: - await _broadcast_speech({"type": "timmy_speech", "text": "test"}) + await _broadcast(json.dumps({"type": "timmy_speech", "text": "test"})) assert dead not in _ws_clients finally: _ws_clients.clear() @@ -368,3 +370,29 @@ async def test_conversation_buffer_caps_at_max(): finally: _ws_clients.clear() _conversation.clear() + + +def test_log_bark_failure_logs_exception(caplog): + """_log_bark_failure logs errors from failed bark tasks.""" + import asyncio + + loop = asyncio.new_event_loop() + + async def _fail(): + raise RuntimeError("bark boom") + + task = loop.create_task(_fail()) + loop.run_until_complete(asyncio.sleep(0.01)) + loop.close() + with caplog.at_level(logging.ERROR): + _log_bark_failure(task) + assert "bark boom" in caplog.text + + +def test_log_bark_failure_ignores_cancelled(): + """_log_bark_failure silently ignores cancelled tasks.""" + import asyncio + + task = MagicMock(spec=asyncio.Task) + task.cancelled.return_value = True + _log_bark_failure(task) # should not raise