Skip to content

Commit 732449e

Browse files
authored
fix: treat 504 proxy error as expected during ha_restart (#621)
When Home Assistant is behind a reverse proxy, the proxy returns a 504 Gateway Timeout when HA shuts down during restart. The error handler only matched "connect" and "closed" patterns, so 504 was incorrectly reported as a failure even though HA restarted successfully. Add "504" to the pattern matching in both ha_restart and ha_install_mcp_tools. Also align the patterns in tools_mcp_component which previously only checked "connection". Fixes #612
1 parent 915ea36 commit 732449e

3 files changed

Lines changed: 78 additions & 4 deletions

File tree

src/ha_mcp/tools/tools_mcp_component.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -276,8 +276,12 @@ async def ha_install_mcp_tools(
276276
result["message"] += ". Home Assistant is restarting."
277277
result["note"] = "Wait 1-5 minutes for Home Assistant to restart."
278278
except Exception as restart_error:
279-
# Connection errors during restart are expected
280-
if "connection" in str(restart_error).lower():
279+
# Connection/proxy errors during restart are expected
280+
# (HA closes connections, proxies may return 504)
281+
if any(
282+
pattern in str(restart_error).lower()
283+
for pattern in ("connect", "closed", "504")
284+
):
281285
result["restarted"] = True
282286
result["message"] += ". Home Assistant is restarting."
283287
result["note"] = (

src/ha_mcp/tools/tools_system.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,8 +168,9 @@ async def ha_restart(
168168
error_msg = str(e)
169169
# Connection errors after restart initiated are expected
170170
# (HA closes connections during restart)
171-
if restart_initiated and (
172-
"connect" in error_msg.lower() or "closed" in error_msg.lower()
171+
if restart_initiated and any(
172+
pattern in error_msg.lower()
173+
for pattern in ("connect", "closed", "504")
173174
):
174175
return {
175176
"success": True,
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
"""Unit tests for tools_system module.
2+
3+
Regression tests for https://github.com/homeassistant-ai/ha-mcp/issues/612
4+
ha_restart reports failure when a reverse proxy returns 504 during restart.
5+
"""
6+
7+
import pytest
8+
from unittest.mock import AsyncMock, MagicMock
9+
10+
from ha_mcp.client.rest_client import (
11+
HomeAssistantAPIError,
12+
)
13+
from ha_mcp.tools.tools_system import register_system_tools
14+
15+
16+
def _register_and_capture_restart(mock_client):
17+
"""Register system tools with a mock MCP and return the ha_restart function."""
18+
mock_mcp = MagicMock()
19+
captured = {}
20+
21+
def fake_tool(**kwargs):
22+
def decorator(fn):
23+
captured[fn.__name__] = fn
24+
return fn
25+
return decorator
26+
27+
mock_mcp.tool = fake_tool
28+
register_system_tools(mock_mcp, mock_client)
29+
assert "ha_restart" in captured, "ha_restart was not registered"
30+
return captured["ha_restart"]
31+
32+
33+
def _make_client_that_fails_on_restart(exception):
34+
"""Create a mock client where check_config succeeds but call_service raises."""
35+
mock_client = AsyncMock()
36+
mock_client.check_config.return_value = {"result": "valid"}
37+
mock_client.call_service.side_effect = exception
38+
return mock_client
39+
40+
41+
class TestHaRestartErrorHandling:
42+
"""Tests for ha_restart handling of expected errors during restart."""
43+
44+
@pytest.mark.asyncio
45+
async def test_504_gateway_timeout_treated_as_success(self):
46+
"""A 504 from a reverse proxy after restart initiated should be success.
47+
48+
Reproduces issue #612: user behind a reverse proxy gets 504 when HA
49+
shuts down, but HA actually restarted successfully.
50+
"""
51+
error = HomeAssistantAPIError("API error: 504 - ", status_code=504)
52+
client = _make_client_that_fails_on_restart(error)
53+
ha_restart = _register_and_capture_restart(client)
54+
55+
result = await ha_restart(confirm=True)
56+
57+
assert result["success"] is True
58+
59+
@pytest.mark.asyncio
60+
async def test_unrelated_error_still_fails(self):
61+
"""Errors unrelated to restart should still report failure."""
62+
error = Exception("Something completely unrelated went wrong")
63+
client = _make_client_that_fails_on_restart(error)
64+
ha_restart = _register_and_capture_restart(client)
65+
66+
result = await ha_restart(confirm=True)
67+
68+
assert result["success"] is False
69+
assert "error" in result

0 commit comments

Comments
 (0)