Skip to content

Commit 521e3b0

Browse files
julienldclaude
andauthored
fix: resolve WebSocket race conditions and improve error handling (#378)
* fix: resolve WebSocket race conditions and improve error handling ## Summary Fixes race conditions from parallel tool calls creating multiple WebSocket connections and improves error handling for common failure scenarios. ## Changes ### WebSocket Race Condition Fix - **rest_client.py**: Use singleton WebSocket connection instead of ephemeral connections - Prevents multiple simultaneous handshakes that trigger 403 throttling - Add automatic retry (2 attempts) for transient 403 errors - Affects 15+ tools using send_websocket_message ### Error Handling Improvements - **ha_eval_template**: Detect 403 errors, provide specific suggestions about reverse proxy/rate limiting - **ha_get_logbook**: Detect 500 errors, suggest reducing hours_back or adding entity_id filter - Both tools now reference ha_bug_report for detailed debugging ### Automation Configuration - **tools_config_automations.py**: Make _normalize_automation_config() recursive - Now handles nested structures (choose/repeat/if-then-else) - Normalizes plural→singular fields at all nesting levels ### Tool Documentation - **ha_get_bulk_status**: Improve description to clarify it's for operation IDs, not entity states - Add examples and comparison with ha_get_state ## Related Issues - Closes issues identified in debug report (/github/perso/ha-mcp/debug/) - Opens #376 (ha_get_notifications) - Opens #377 (ha_get_states bulk getter) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * fix: move websocket import outside retry loop Address Gemini Code Assist review suggestion to improve efficiency by moving import statement outside the retry loop. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent 1a1102f commit 521e3b0

4 files changed

Lines changed: 136 additions & 39 deletions

File tree

src/ha_mcp/client/rest_client.py

Lines changed: 48 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -586,35 +586,59 @@ async def delete_automation_config(self, identifier: str) -> dict[str, Any]:
586586
raise
587587

588588
async def send_websocket_message(self, message: dict[str, Any]) -> dict[str, Any]:
589-
"""Send message via WebSocket and wait for response."""
590-
ws_client = None
591-
try:
592-
# Use client's own URL and token for WebSocket connection
593-
from .websocket_client import HomeAssistantWebSocketClient
589+
"""Send message via WebSocket and wait for response.
590+
591+
Uses the global WebSocket singleton to avoid race conditions from
592+
parallel tool calls creating multiple simultaneous connections.
593+
"""
594+
from .websocket_client import get_websocket_client
594595

595-
ws_client = HomeAssistantWebSocketClient(self.base_url, self.token)
596+
max_retries = 2
597+
retry_delay = 0.5 # seconds
596598

597-
# Connect if not already connected
598-
if not ws_client.is_connected:
599-
await ws_client.connect()
599+
for attempt in range(max_retries):
600+
try:
601+
# Use singleton WebSocket client (shared, reused connection)
602+
ws_client = await get_websocket_client()
600603

601-
# Special handling for render_template which returns an event with the actual result
602-
if message.get("type") == "render_template":
603-
return await self._handle_render_template(ws_client, message)
604+
# Special handling for render_template which returns an event with the actual result
605+
if message.get("type") == "render_template":
606+
return await self._handle_render_template(ws_client, message)
604607

605-
# Extract command type and parameters for other commands
606-
message_copy = message.copy()
607-
command_type = message_copy.pop("type")
608-
result = await ws_client.send_command(command_type, **message_copy)
608+
# Extract command type and parameters for other commands
609+
message_copy = message.copy()
610+
command_type = message_copy.pop("type")
611+
result = await ws_client.send_command(command_type, **message_copy)
609612

610-
return result
611-
except Exception as e:
612-
logger.error(f"WebSocket message failed: {e}")
613-
return {"success": False, "error": str(e)}
614-
finally:
615-
# Clean up WebSocket connection
616-
if ws_client and ws_client.is_connected:
617-
await ws_client.disconnect()
613+
return result
614+
615+
except Exception as e:
616+
error_str = str(e)
617+
618+
# Detect transient 403 errors (rate limiting / reverse proxy throttling)
619+
if "403" in error_str and "Forbidden" in error_str:
620+
if attempt < max_retries - 1:
621+
logger.warning(
622+
f"WebSocket 403 error (attempt {attempt + 1}/{max_retries}), "
623+
f"retrying after {retry_delay}s: {error_str}"
624+
)
625+
await asyncio.sleep(retry_delay)
626+
continue
627+
else:
628+
logger.error(f"WebSocket 403 error after {max_retries} attempts: {error_str}")
629+
return {
630+
"success": False,
631+
"error": f"WebSocket request blocked (403 Forbidden): {error_str}",
632+
"suggestions": [
633+
"This may be caused by a reverse proxy or security filter",
634+
"Try simplifying the request (e.g., shorter templates, fewer parameters)",
635+
"If using complex templates, try breaking them into smaller parts",
636+
"Check if your Home Assistant is behind a reverse proxy with security rules",
637+
],
638+
}
639+
640+
logger.error(f"WebSocket message failed: {e}")
641+
return {"success": False, "error": str(e)}
618642

619643
async def _handle_render_template(
620644
self, ws_client: Any, message: dict[str, Any]

src/ha_mcp/tools/tools_config_automations.py

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,37 +21,54 @@
2121
logger = logging.getLogger(__name__)
2222

2323

24-
def _normalize_automation_config(config: dict[str, Any]) -> dict[str, Any]:
24+
def _normalize_automation_config(config: Any) -> Any:
2525
"""
26-
Normalize automation config field names to HA API format.
26+
Recursively normalize automation config field names to HA API format.
2727
2828
Home Assistant accepts both singular ('trigger', 'action', 'condition')
2929
and plural ('triggers', 'actions', 'conditions') field names in YAML,
3030
but the API expects singular forms. This function normalizes plural
31-
to singular for consistency.
31+
to singular for consistency, recursively processing nested structures
32+
like 'choose', 'repeat', 'if/then/else'.
3233
3334
Args:
34-
config: Automation configuration dict
35+
config: Automation configuration (dict, list, or primitive)
3536
3637
Returns:
3738
Normalized configuration with singular field names
3839
"""
40+
# Handle lists - recursively process each item
41+
if isinstance(config, list):
42+
return [_normalize_automation_config(item) for item in config]
43+
44+
# Handle primitives (strings, numbers, etc.)
45+
if not isinstance(config, dict):
46+
return config
47+
48+
# Process dictionary
3949
normalized = config.copy()
4050

4151
# Map plural field names to singular (HA API format)
4252
field_mappings = {
4353
"triggers": "trigger",
4454
"actions": "action",
4555
"conditions": "condition",
56+
# Note: 'sequence' is already singular, but some users might use 'sequences'
57+
"sequences": "sequence",
4658
}
4759

60+
# Apply field mapping to current level
4861
for plural, singular in field_mappings.items():
4962
if plural in normalized and singular not in normalized:
5063
normalized[singular] = normalized.pop(plural)
5164
elif plural in normalized and singular in normalized:
5265
# Both exist - prefer singular, remove plural
5366
del normalized[plural]
5467

68+
# Recursively process all values in the dictionary
69+
for key, value in normalized.items():
70+
normalized[key] = _normalize_automation_config(value)
71+
5572
return normalized
5673

5774

src/ha_mcp/tools/tools_service.py

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -258,9 +258,35 @@ async def ha_bulk_control(
258258
)
259259
return cast(dict[str, Any], result)
260260

261-
@mcp.tool(annotations={"readOnlyHint": True, "title": "Get Bulk Status"})
261+
@mcp.tool(annotations={"readOnlyHint": True, "title": "Get Bulk Operation Status"})
262262
async def ha_get_bulk_status(operation_ids: list[str]) -> dict[str, Any]:
263-
"""Check status of multiple WebSocket-monitored operations."""
263+
"""
264+
Check status of multiple device control operations.
265+
266+
Use this tool to check the status of operations initiated by ha_bulk_control
267+
or control_device_smart. Each of these tools returns unique operation_ids
268+
that can be tracked here.
269+
270+
**IMPORTANT:** This tool is for tracking async device operations, NOT for
271+
checking current entity states. To get current states of entities, use
272+
ha_get_state instead.
273+
274+
**Args:**
275+
operation_ids: List of operation IDs returned by ha_bulk_control or
276+
control_device_smart (e.g., ["op_1234", "op_5678"])
277+
278+
**Returns:**
279+
Status summary with completion/pending/failed counts and detailed
280+
results for each operation.
281+
282+
**Example:**
283+
# After calling control_device_smart
284+
result = control_device_smart("light.kitchen", "on")
285+
op_id = result["operation_id"] # e.g., "op_1234"
286+
287+
# Check operation status
288+
status = ha_get_bulk_status([op_id])
289+
"""
264290
result = await device_tools.get_bulk_operation_status(
265291
operation_ids=operation_ids
266292
)

src/ha_mcp/tools/tools_utility.py

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -189,10 +189,25 @@ async def ha_get_logbook(
189189
return await add_timezone_metadata(client, logbook_data)
190190

191191
except Exception as e:
192+
error_str = str(e)
193+
suggestions = []
194+
195+
# Detect 500 errors (server crash from heavy query)
196+
if "500" in error_str:
197+
suggestions = [
198+
"The query returned too many results causing a server error (500).",
199+
"This often happens with very active entities or long time periods.",
200+
"Try reducing 'hours_back' parameter (e.g., from 24 to 1 hour)",
201+
"Add a specific 'entity_id' filter to narrow down results",
202+
"If debugging an automation, filter by that automation's entity_id",
203+
"Use ha_bug_report tool to check Home Assistant logs for crash details",
204+
]
205+
192206
error_data = {
193207
"success": False,
194-
"error": f"Failed to retrieve logbook: {str(e)}",
208+
"error": f"Failed to retrieve logbook: {error_str}",
195209
"period": f"{hours_back_int} hours back from {end_dt.isoformat()}",
210+
"suggestions": suggestions if suggestions else None,
196211
}
197212
return await add_timezone_metadata(client, error_data)
198213

@@ -389,17 +404,32 @@ async def ha_eval_template(
389404
}
390405

391406
except Exception as e:
407+
error_str = str(e)
408+
suggestions = [
409+
"Check Home Assistant WebSocket connection",
410+
"Verify template syntax is valid Jinja2",
411+
"Try a simpler template to test basic functionality",
412+
"Check if referenced entities exist",
413+
"Ensure template doesn't exceed timeout limit",
414+
]
415+
416+
# Add specific suggestions for 403 errors
417+
if "403" in error_str and "Forbidden" in error_str:
418+
suggestions = [
419+
"The request was blocked (403 Forbidden) - this may be caused by:",
420+
" • Reverse proxy security rules (Apache, Nginx, Traefik)",
421+
" • Rate limiting from multiple simultaneous requests",
422+
" • Complex template triggering security filters",
423+
"Try simplifying the template (remove newlines, reduce complexity)",
424+
"Break complex templates into multiple simpler calls",
425+
"Use ha_bug_report tool to check Home Assistant logs for details",
426+
] + suggestions
427+
392428
return {
393429
"success": False,
394430
"template": template,
395-
"error": f"Template evaluation failed: {str(e)}",
396-
"suggestions": [
397-
"Check Home Assistant WebSocket connection",
398-
"Verify template syntax is valid Jinja2",
399-
"Try a simpler template to test basic functionality",
400-
"Check if referenced entities exist",
401-
"Ensure template doesn't exceed timeout limit",
402-
],
431+
"error": f"Template evaluation failed: {error_str}",
432+
"suggestions": suggestions,
403433
}
404434

405435
@mcp.tool(annotations={"readOnlyHint": True, "title": "Get Domain Docs"})

0 commit comments

Comments
 (0)