Skip to content

Commit 1b8f7d7

Browse files
committed
fix: Signal tool errors via isError for remaining 9 tools (#518)
Migrate remaining tool modules to use ToolError/raise_tool_error for MCP protocol-level error signaling. Includes updated unit tests for voice assistant and entity tools. https://claude.ai/code/session_01MvDDV6qmWosBfGhmTLYzo8
1 parent 2217935 commit 1b8f7d7

11 files changed

Lines changed: 149 additions & 109 deletions

src/ha_mcp/tools/tools_areas.py

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,10 @@
1010

1111
from pydantic import Field
1212

13+
from fastmcp.exceptions import ToolError
14+
1315
from ..errors import ErrorCode, create_error_response
14-
from .helpers import log_tool_usage
16+
from .helpers import log_tool_usage, raise_tool_error
1517
from .util_helpers import parse_string_list_param
1618

1719
logger = logging.getLogger(__name__)
@@ -121,10 +123,10 @@ async def ha_config_set_area(
121123
try:
122124
parsed_aliases = parse_string_list_param(aliases, "aliases")
123125
except ValueError as e:
124-
return create_error_response(
126+
raise_tool_error(create_error_response(
125127
ErrorCode.VALIDATION_INVALID_PARAMETER,
126128
f"Invalid aliases parameter: {e}",
127-
)
129+
))
128130

129131
# Determine if this is a create or update operation
130132
if area_id:
@@ -193,6 +195,8 @@ async def ha_config_set_area(
193195
error_response["name"] = name
194196
return error_response
195197

198+
except ToolError:
199+
raise
196200
except Exception as e:
197201
logger.error(f"Error in ha_config_set_area: {e}")
198202
error_response = {
@@ -352,10 +356,10 @@ async def ha_config_set_floor(
352356
try:
353357
parsed_aliases = parse_string_list_param(aliases, "aliases")
354358
except ValueError as e:
355-
return create_error_response(
359+
raise_tool_error(create_error_response(
356360
ErrorCode.VALIDATION_INVALID_PARAMETER,
357361
f"Invalid aliases parameter: {e}",
358-
)
362+
))
359363

360364
# Determine if this is a create or update operation
361365
if floor_id:
@@ -420,6 +424,8 @@ async def ha_config_set_floor(
420424
error_response["name"] = name
421425
return error_response
422426

427+
except ToolError:
428+
raise
423429
except Exception as e:
424430
logger.error(f"Error in ha_config_set_floor: {e}")
425431
error_response = {

src/ha_mcp/tools/tools_config_entry_flow.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ async def ha_create_config_entry_helper(
157157
context = {"helper_type": helper_type}
158158
if flow_id:
159159
context["flow_id"] = flow_id
160-
return exception_to_structured_error(e, context=context)
160+
exception_to_structured_error(e, context=context)
161161

162162
@mcp.tool(
163163
annotations={
@@ -220,4 +220,4 @@ async def ha_get_helper_schema(
220220

221221
except Exception as e:
222222
logger.error(f"Error getting helper schema: {e}")
223-
return exception_to_structured_error(e, context={"helper_type": helper_type})
223+
exception_to_structured_error(e, context={"helper_type": helper_type})

src/ha_mcp/tools/tools_config_helpers.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,10 @@
1212

1313
from pydantic import Field
1414

15+
from fastmcp.exceptions import ToolError
16+
1517
from ..errors import ErrorCode, create_error_response
16-
from .helpers import log_tool_usage
18+
from .helpers import log_tool_usage, raise_tool_error
1719
from .util_helpers import parse_string_list_param
1820

1921
logger = logging.getLogger(__name__)
@@ -386,10 +388,10 @@ async def ha_config_set_helper(
386388
labels = parse_string_list_param(labels, "labels")
387389
options = parse_string_list_param(options, "options")
388390
except ValueError as e:
389-
return create_error_response(
391+
raise_tool_error(create_error_response(
390392
ErrorCode.VALIDATION_INVALID_PARAMETER,
391393
f"Invalid list parameter: {e}",
392-
)
394+
))
393395

394396
# Determine if this is a create or update based on helper_id
395397
action = "update" if helper_id else "create"
@@ -697,6 +699,8 @@ async def ha_config_set_helper(
697699
"error": f"Unexpected action: {action}",
698700
}
699701

702+
except ToolError:
703+
raise
700704
except Exception as e:
701705
return {
702706
"success": False,

src/ha_mcp/tools/tools_entities.py

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,10 @@
1111

1212
from pydantic import Field
1313

14+
from fastmcp.exceptions import ToolError
15+
1416
from ..errors import ErrorCode, create_error_response
15-
from .helpers import exception_to_structured_error, log_tool_usage
17+
from .helpers import exception_to_structured_error, log_tool_usage, raise_tool_error
1618
from .tools_voice_assistant import KNOWN_ASSISTANTS
1719
from .util_helpers import coerce_bool_param, parse_json_param, parse_string_list_param
1820

@@ -445,65 +447,65 @@ async def ha_set_entity(
445447
try:
446448
parsed_aliases = parse_string_list_param(aliases, "aliases")
447449
except ValueError as e:
448-
return create_error_response(
450+
raise_tool_error(create_error_response(
449451
ErrorCode.VALIDATION_INVALID_PARAMETER,
450452
f"Invalid aliases parameter: {e}",
451-
)
453+
))
452454

453455
parsed_labels = None
454456
if labels is not None:
455457
try:
456458
parsed_labels = parse_string_list_param(labels, "labels")
457459
except ValueError as e:
458-
return create_error_response(
460+
raise_tool_error(create_error_response(
459461
ErrorCode.VALIDATION_INVALID_PARAMETER,
460462
f"Invalid labels parameter: {e}",
461-
)
463+
))
462464

463465
# Parse and validate expose_to parameter
464466
parsed_expose_to: dict[str, bool] | None = None
465467
if expose_to is not None:
466468
try:
467469
parsed = parse_json_param(expose_to, "expose_to")
468470
except ValueError as e:
469-
return create_error_response(
471+
raise_tool_error(create_error_response(
470472
ErrorCode.VALIDATION_INVALID_PARAMETER,
471473
str(e),
472-
)
474+
))
473475

474476
if not isinstance(parsed, dict):
475-
return create_error_response(
477+
raise_tool_error(create_error_response(
476478
ErrorCode.VALIDATION_INVALID_PARAMETER,
477479
"expose_to must be a dict mapping assistant IDs to booleans, "
478480
'e.g. {"conversation": true, "cloud.alexa": false}',
479-
)
481+
))
480482
parsed_expose_to = parsed
481483

482484
# Validate assistant names
483485
invalid_assistants = [
484486
a for a in parsed_expose_to if a not in KNOWN_ASSISTANTS
485487
]
486488
if invalid_assistants:
487-
return create_error_response(
489+
raise_tool_error(create_error_response(
488490
ErrorCode.VALIDATION_INVALID_PARAMETER,
489491
f"Invalid assistant(s) in expose_to: {invalid_assistants}. "
490492
f"Valid: {KNOWN_ASSISTANTS}",
491-
)
493+
))
492494

493495
# Coerce values to bool
494496
for asst, val in parsed_expose_to.items():
495497
try:
496498
coerced = coerce_bool_param(val, f"expose_to[{asst}]")
497499
except ValueError as e:
498-
return create_error_response(
500+
raise_tool_error(create_error_response(
499501
ErrorCode.VALIDATION_INVALID_PARAMETER,
500502
str(e),
501-
)
503+
))
502504
if coerced is None:
503-
return create_error_response(
505+
raise_tool_error(create_error_response(
504506
ErrorCode.VALIDATION_INVALID_PARAMETER,
505507
f"expose_to[{asst}] must be a boolean value",
506-
)
508+
))
507509
parsed_expose_to[asst] = coerced
508510

509511
# Single entity case - use existing logic
@@ -579,10 +581,12 @@ async def ha_set_entity(
579581

580582
return response
581583

584+
except ToolError:
585+
raise
582586
except Exception as e:
583587
logger.error(f"Error updating entity: {e}")
584588
eid_context = entity_id if isinstance(entity_id, str) else entity_ids
585-
return exception_to_structured_error(e, context={"entity_id": eid_context})
589+
exception_to_structured_error(e, context={"entity_id": eid_context})
586590

587591
@mcp.tool(
588592
annotations={
@@ -768,6 +772,6 @@ async def _fetch_entity(eid: str) -> dict[str, Any]:
768772

769773
except Exception as e:
770774
logger.error(f"Error getting entity: {e}")
771-
return exception_to_structured_error(
775+
exception_to_structured_error(
772776
e, context={"entity_id": entity_id if isinstance(entity_id, str) else entity_ids}
773777
)

src/ha_mcp/tools/tools_filesystem.py

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -189,11 +189,10 @@ async def ha_list_files(
189189
)
190190

191191
except Exception as e:
192-
error_response = exception_to_structured_error(
192+
exception_to_structured_error(
193193
e,
194194
context={"tool": "ha_list_files", "path": path, "pattern": pattern},
195195
)
196-
return await add_timezone_metadata(client, error_response)
197196

198197
@mcp.tool(
199198
annotations={
@@ -313,11 +312,10 @@ async def ha_read_file(
313312
)
314313

315314
except Exception as e:
316-
error_response = exception_to_structured_error(
315+
exception_to_structured_error(
317316
e,
318317
context={"tool": "ha_read_file", "path": path},
319318
)
320-
return await add_timezone_metadata(client, error_response)
321319

322320
@mcp.tool(
323321
annotations={
@@ -455,11 +453,10 @@ async def ha_write_file(
455453
)
456454

457455
except Exception as e:
458-
error_response = exception_to_structured_error(
456+
exception_to_structured_error(
459457
e,
460458
context={"tool": "ha_write_file", "path": path},
461459
)
462-
return await add_timezone_metadata(client, error_response)
463460

464461
@mcp.tool(
465462
annotations={
@@ -582,8 +579,7 @@ async def ha_delete_file(
582579
)
583580

584581
except Exception as e:
585-
error_response = exception_to_structured_error(
582+
exception_to_structured_error(
586583
e,
587584
context={"tool": "ha_delete_file", "path": path},
588585
)
589-
return await add_timezone_metadata(client, error_response)

0 commit comments

Comments
 (0)