Skip to content

Commit 6fcea2e

Browse files
kingpanther13claude
andcommitted
refactor: add suggestions parameter to exception_to_structured_error
Address review feedback from @sergeykad — collapse the repeated 3-line pattern (get error dict, insert suggestions, raise) into a single call by adding an optional `suggestions` parameter to `exception_to_structured_error()`. This simplifies ~15 call sites across 7 tool modules while preserving behavior. The `raise_error=False` path remains for `_fetch_state` in ha_get_states which collects per-entity errors inside asyncio.gather. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 27922ee commit 6fcea2e

8 files changed

Lines changed: 92 additions & 99 deletions

File tree

src/ha_mcp/tools/helpers.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ def exception_to_structured_error(
8989
context: dict[str, Any] | None = None,
9090
*,
9191
raise_error: Literal[True] = True,
92+
suggestions: list[str] | None = None,
9293
) -> NoReturn: ...
9394

9495

@@ -98,6 +99,7 @@ def exception_to_structured_error(
9899
context: dict[str, Any] | None = None,
99100
*,
100101
raise_error: Literal[False],
102+
suggestions: list[str] | None = None,
101103
) -> dict[str, Any]: ...
102104

103105

@@ -106,6 +108,7 @@ def exception_to_structured_error(
106108
context: dict[str, Any] | None = None,
107109
*,
108110
raise_error: bool = True,
111+
suggestions: list[str] | None = None,
109112
) -> dict[str, Any]:
110113
"""
111114
Convert an exception to a structured error response.
@@ -119,6 +122,8 @@ def exception_to_structured_error(
119122
context: Additional context to include in the response
120123
raise_error: If True (default), raises ToolError with the structured error.
121124
If False, returns the error dict for further modification.
125+
suggestions: Optional list of actionable suggestions to embed in the error.
126+
Saves callers from manually inserting suggestions after the call.
122127
123128
Returns:
124129
Structured error response dictionary (only if raise_error=False)
@@ -208,6 +213,9 @@ def exception_to_structured_error(
208213
context=context,
209214
)
210215

216+
if suggestions and "error" in error_response and isinstance(error_response["error"], dict):
217+
error_response["error"]["suggestions"] = suggestions
218+
211219
if raise_error:
212220
raise_tool_error(error_response)
213221

src/ha_mcp/tools/tools_config_automations.py

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -258,19 +258,15 @@ async def ha_config_get_automation(
258258
raise_tool_error(error_response)
259259

260260
logger.error(f"Error getting automation: {e}")
261-
error_response = exception_to_structured_error(
261+
exception_to_structured_error(
262262
e,
263263
context={"identifier": identifier, "action": "get"},
264-
raise_error=False,
265-
)
266-
# Add automation-specific suggestions
267-
if "error" in error_response and isinstance(error_response["error"], dict):
268-
error_response["error"]["suggestions"] = [
264+
suggestions=[
269265
"Verify automation exists using ha_search_entities(domain_filter='automation')",
270266
"Check Home Assistant connection",
271267
"Use ha_get_domain_docs('automation') for configuration help",
272-
]
273-
raise_tool_error(error_response)
268+
],
269+
)
274270

275271
@mcp.tool(
276272
annotations={
@@ -483,21 +479,17 @@ async def ha_config_set_automation(
483479
raise
484480
except Exception as e:
485481
logger.error(f"Error upserting automation: {e}")
486-
error_response = exception_to_structured_error(
482+
exception_to_structured_error(
487483
e,
488484
context={"identifier": identifier},
489-
raise_error=False,
490-
)
491-
# Add automation-specific suggestions
492-
if "error" in error_response and isinstance(error_response["error"], dict):
493-
error_response["error"]["suggestions"] = [
485+
suggestions=[
494486
"Check automation configuration format",
495487
"Ensure required fields: alias, trigger, action",
496488
"Use entity_id format: automation.morning_routine or unique_id",
497489
"Use ha_search_entities(domain_filter='automation') to find automations",
498490
"Use ha_get_domain_docs('automation') for comprehensive configuration help",
499-
]
500-
raise_tool_error(error_response)
491+
],
492+
)
501493

502494
@mcp.tool(
503495
annotations={

src/ha_mcp/tools/tools_config_dashboards.py

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1688,7 +1688,7 @@ async def ha_dashboard_find_card(
16881688
f"error={e}",
16891689
exc_info=True,
16901690
)
1691-
error_response = exception_to_structured_error(
1691+
return exception_to_structured_error(
16921692
e,
16931693
context={
16941694
"action": "find_card",
@@ -1698,15 +1698,8 @@ async def ha_dashboard_find_card(
16981698
"heading": heading,
16991699
},
17001700
raise_error=False,
1701-
)
1702-
if "error" in error_response and isinstance(error_response["error"], dict):
1703-
error_response["error"]["suggestions"] = [
1701+
suggestions=[
17041702
"Check HA connection",
17051703
"Verify dashboard with ha_config_get_dashboard(list_only=True)",
1706-
]
1707-
else:
1708-
logger.warning(
1709-
f"Unexpected error response structure, could not add suggestions: "
1710-
f"{type(error_response.get('error'))}"
1711-
)
1712-
return error_response
1704+
],
1705+
)

src/ha_mcp/tools/tools_hacs.py

Lines changed: 18 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -124,13 +124,12 @@ async def ha_hacs_info() -> dict[str, Any]:
124124
e,
125125
context={"tool": "ha_hacs_info"},
126126
raise_error=False,
127-
)
128-
if "error" in error_response and isinstance(error_response["error"], dict):
129-
error_response["error"]["suggestions"] = [
127+
suggestions=[
130128
"Verify HACS is installed: https://hacs.xyz/",
131129
"Check Home Assistant connection",
132130
"Restart Home Assistant if HACS was recently installed",
133-
]
131+
],
132+
)
134133
error_with_tz = await add_timezone_metadata(client, error_response)
135134
raise_tool_error(error_with_tz)
136135

@@ -244,13 +243,12 @@ async def ha_hacs_list_installed(
244243
e,
245244
context={"tool": "ha_hacs_list_installed", "category": category},
246245
raise_error=False,
247-
)
248-
if "error" in error_response and isinstance(error_response["error"], dict):
249-
error_response["error"]["suggestions"] = [
246+
suggestions=[
250247
"Verify HACS is installed: https://hacs.xyz/",
251248
"Check category name is valid: integration, lovelace, theme, appdaemon, python_script",
252249
"Check Home Assistant connection",
253-
]
250+
],
251+
)
254252
error_with_tz = await add_timezone_metadata(client, error_response)
255253
raise_tool_error(error_with_tz)
256254

@@ -425,13 +423,12 @@ async def ha_hacs_search(
425423
e,
426424
context={"tool": "ha_hacs_search", "query": query, "category": category},
427425
raise_error=False,
428-
)
429-
if "error" in error_response and isinstance(error_response["error"], dict):
430-
error_response["error"]["suggestions"] = [
426+
suggestions=[
431427
"Verify HACS is installed: https://hacs.xyz/",
432428
"Try a simpler search query",
433429
"Check category name is valid: integration, lovelace, theme, appdaemon, python_script",
434-
]
430+
],
431+
)
435432
error_with_tz = await add_timezone_metadata(client, error_response)
436433
raise_tool_error(error_with_tz)
437434

@@ -543,13 +540,12 @@ async def ha_hacs_repository_info(repository_id: str) -> dict[str, Any]:
543540
e,
544541
context={"tool": "ha_hacs_repository_info", "repository_id": repository_id},
545542
raise_error=False,
546-
)
547-
if "error" in error_response and isinstance(error_response["error"], dict):
548-
error_response["error"]["suggestions"] = [
543+
suggestions=[
549544
"Verify HACS is installed: https://hacs.xyz/",
550545
"Check repository ID format (e.g., 'hacs/integration' or 'owner/repo')",
551546
"Use ha_hacs_search() to find the correct repository ID",
552-
]
547+
],
548+
)
553549
error_with_tz = await add_timezone_metadata(client, error_response)
554550
raise_tool_error(error_with_tz)
555551

@@ -668,15 +664,14 @@ async def ha_hacs_add_repository(
668664
"category": category,
669665
},
670666
raise_error=False,
671-
)
672-
if "error" in error_response and isinstance(error_response["error"], dict):
673-
error_response["error"]["suggestions"] = [
667+
suggestions=[
674668
"Verify HACS is installed: https://hacs.xyz/",
675669
"Check repository format: 'owner/repo'",
676670
"Verify the repository exists on GitHub",
677671
"Ensure category matches repository type",
678672
"Check repository follows HACS guidelines: https://hacs.xyz/docs/publish/start",
679-
]
673+
],
674+
)
680675
error_with_tz = await add_timezone_metadata(client, error_response)
681676
raise_tool_error(error_with_tz)
682677

@@ -806,13 +801,12 @@ async def ha_hacs_download(
806801
"version": version,
807802
},
808803
raise_error=False,
809-
)
810-
if "error" in error_response and isinstance(error_response["error"], dict):
811-
error_response["error"]["suggestions"] = [
804+
suggestions=[
812805
"Verify HACS is installed: https://hacs.xyz/",
813806
"Check repository ID is valid (use ha_hacs_search() to find it)",
814807
"Ensure the repository is in HACS (use ha_hacs_add_repository() if needed)",
815808
"Check version format (e.g., 'v1.2.3' or '1.2.3')",
816-
]
809+
],
810+
)
817811
error_with_tz = await add_timezone_metadata(client, error_response)
818812
raise_tool_error(error_with_tz)

src/ha_mcp/tools/tools_mcp_component.py

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

1515
from pydantic import Field
1616

17-
from .helpers import exception_to_structured_error, log_tool_usage, raise_tool_error
17+
from .helpers import exception_to_structured_error, log_tool_usage
1818
from .util_helpers import add_timezone_metadata
1919

2020
logger = logging.getLogger(__name__)
@@ -296,17 +296,12 @@ async def ha_install_mcp_tools(
296296
return await add_timezone_metadata(client, result)
297297

298298
except Exception as e:
299-
error_response = exception_to_structured_error(
299+
exception_to_structured_error(
300300
e,
301301
context={"tool": "ha_install_mcp_tools", "restart": restart},
302-
raise_error=False,
303-
)
304-
if "error" in error_response and isinstance(error_response["error"], dict):
305-
suggestions = [
302+
suggestions=[
306303
"Verify HACS is installed: https://hacs.xyz/",
307304
"Check Home Assistant logs for errors",
308305
"Ensure GitHub is accessible",
309-
]
310-
error_response["error"]["suggestions"] = suggestions
311-
error_response["error"]["suggestion"] = suggestions[0]
312-
raise_tool_error(error_response)
306+
],
307+
)

src/ha_mcp/tools/tools_search.py

Lines changed: 7 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -441,19 +441,12 @@ async def ha_search_entities(
441441
"area_filter": area_filter,
442442
},
443443
raise_error=False,
444-
)
445-
# Add search-specific suggestions
446-
if "error" in error_response and isinstance(error_response["error"], dict):
447-
error_response["error"]["suggestions"] = [
444+
suggestions=[
448445
"Check Home Assistant connection",
449446
"Try simpler search terms",
450447
"Check area/domain filter spelling",
451-
]
452-
else:
453-
logger.warning(
454-
f"Unexpected error response structure, could not add suggestions: "
455-
f"{type(error_response.get('error'))}"
456-
)
448+
],
449+
)
457450
error_with_tz = await add_timezone_metadata(client, error_response)
458451
raise_tool_error(error_with_tz)
459452

@@ -627,26 +620,19 @@ async def ha_deep_search(
627620
f"error={e}",
628621
exc_info=True,
629622
)
630-
error_response = exception_to_structured_error(
623+
return exception_to_structured_error(
631624
e,
632625
context={
633626
"query": query,
634627
"search_types": parsed_search_types,
635628
"limit": limit,
636629
},
637630
raise_error=False,
638-
)
639-
if "error" in error_response and isinstance(error_response["error"], dict):
640-
error_response["error"]["suggestions"] = [
631+
suggestions=[
641632
"Check Home Assistant connection",
642633
"Try simpler search terms",
643-
]
644-
else:
645-
logger.warning(
646-
f"Unexpected error response structure, could not add suggestions: "
647-
f"{type(error_response.get('error'))}"
648-
)
649-
return error_response
634+
],
635+
)
650636

651637
@mcp.tool(
652638
annotations={

src/ha_mcp/tools/tools_service.py

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -211,41 +211,34 @@ async def ha_call_service(
211211
),
212212
}
213213
# Non-timeout connection errors are real failures
214-
error_response = exception_to_structured_error(
214+
exception_to_structured_error(
215215
error,
216216
context={
217217
"domain": domain,
218218
"service": service,
219219
"entity_id": entity_id,
220220
},
221-
raise_error=False,
221+
suggestions=_build_service_suggestions(domain, service, entity_id),
222222
)
223-
if "error" in error_response and isinstance(error_response["error"], dict):
224-
error_response["error"]["suggestions"] = _build_service_suggestions(domain, service, entity_id)
225-
raise_tool_error(error_response)
226223
except ToolError:
227224
raise
228225
except Exception as error:
229226
# Use structured error response
230-
error_response = exception_to_structured_error(
227+
suggestions = _build_service_suggestions(domain, service, entity_id)
228+
if entity_id:
229+
suggestions.extend([
230+
f"For automation: ha_call_service('automation', 'trigger', entity_id='{entity_id}')",
231+
f"For universal control: ha_call_service('homeassistant', 'toggle', entity_id='{entity_id}')",
232+
])
233+
exception_to_structured_error(
231234
error,
232235
context={
233236
"domain": domain,
234237
"service": service,
235238
"entity_id": entity_id,
236239
},
237-
raise_error=False,
240+
suggestions=suggestions,
238241
)
239-
suggestions = _build_service_suggestions(domain, service, entity_id)
240-
if entity_id:
241-
suggestions.extend([
242-
f"For automation: ha_call_service('automation', 'trigger', entity_id='{entity_id}')",
243-
f"For universal control: ha_call_service('homeassistant', 'toggle', entity_id='{entity_id}')",
244-
])
245-
# Merge suggestions into error response
246-
if "error" in error_response and isinstance(error_response["error"], dict):
247-
error_response["error"]["suggestions"] = suggestions
248-
raise_tool_error(error_response)
249242

250243
@mcp.tool(annotations={"readOnlyHint": True, "title": "Get Operation Status"})
251244
@log_tool_usage

0 commit comments

Comments
 (0)