mirror of
https://github.com/jmagar/unraid-mcp.git
synced 2026-03-23 12:39:24 -07:00
feat(creds): tool_error_handler converts CredentialsNotConfiguredError to ToolError with path
Converts the CredentialsNotConfiguredError sentinel to a user-facing ToolError in tool_error_handler, including the exact CREDENTIALS_ENV_PATH so users know where to create the .env file. Removes the now-invalid per-tool elicitation test (replaced by 2 new tests for the handler conversion behavior).
This commit is contained in:
@@ -181,45 +181,6 @@ async def test_make_graphql_request_raises_sentinel_when_unconfigured():
|
|||||||
settings_mod.UNRAID_API_KEY = original_key
|
settings_mod.UNRAID_API_KEY = original_key
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
|
||||||
async def test_auto_elicitation_triggered_on_credentials_not_configured():
|
|
||||||
"""Any tool call with missing creds auto-triggers elicitation before erroring."""
|
|
||||||
from unittest.mock import AsyncMock, MagicMock, patch
|
|
||||||
|
|
||||||
from conftest import make_tool_fn
|
|
||||||
from fastmcp import FastMCP
|
|
||||||
|
|
||||||
from unraid_mcp.core.exceptions import CredentialsNotConfiguredError
|
|
||||||
from unraid_mcp.tools.info import register_info_tool
|
|
||||||
|
|
||||||
test_mcp = FastMCP("test")
|
|
||||||
register_info_tool(test_mcp)
|
|
||||||
tool_fn = make_tool_fn("unraid_mcp.tools.info", "register_info_tool", "unraid_info")
|
|
||||||
|
|
||||||
mock_ctx = MagicMock()
|
|
||||||
|
|
||||||
# First call raises CredentialsNotConfiguredError, second returns data
|
|
||||||
call_count = 0
|
|
||||||
|
|
||||||
async def side_effect(*args, **kwargs):
|
|
||||||
nonlocal call_count
|
|
||||||
call_count += 1
|
|
||||||
if call_count == 1:
|
|
||||||
raise CredentialsNotConfiguredError()
|
|
||||||
return {"info": {"os": {"hostname": "tootie"}}}
|
|
||||||
|
|
||||||
with (
|
|
||||||
patch("unraid_mcp.tools.info.make_graphql_request", side_effect=side_effect),
|
|
||||||
patch(
|
|
||||||
"unraid_mcp.tools.info.elicit_and_configure", new=AsyncMock(return_value=True)
|
|
||||||
) as mock_elicit,
|
|
||||||
):
|
|
||||||
result = await tool_fn(action="overview", ctx=mock_ctx)
|
|
||||||
|
|
||||||
mock_elicit.assert_called_once_with(mock_ctx)
|
|
||||||
assert result is not None
|
|
||||||
|
|
||||||
|
|
||||||
import os # noqa: E402 — needed for reload-based tests below
|
import os # noqa: E402 — needed for reload-based tests below
|
||||||
|
|
||||||
|
|
||||||
@@ -390,3 +351,37 @@ async def test_elicit_and_configure_returns_false_when_client_not_supported():
|
|||||||
|
|
||||||
result = await elicit_and_configure(mock_ctx)
|
result = await elicit_and_configure(mock_ctx)
|
||||||
assert result is False
|
assert result is False
|
||||||
|
|
||||||
|
|
||||||
|
def test_tool_error_handler_converts_credentials_not_configured_to_tool_error():
|
||||||
|
"""tool_error_handler wraps CredentialsNotConfiguredError in a ToolError."""
|
||||||
|
import logging
|
||||||
|
|
||||||
|
from unraid_mcp.core.exceptions import (
|
||||||
|
CredentialsNotConfiguredError,
|
||||||
|
ToolError,
|
||||||
|
tool_error_handler,
|
||||||
|
)
|
||||||
|
|
||||||
|
_log = logging.getLogger("test")
|
||||||
|
with pytest.raises(ToolError), tool_error_handler("docker", "list", _log):
|
||||||
|
raise CredentialsNotConfiguredError()
|
||||||
|
|
||||||
|
|
||||||
|
def test_tool_error_handler_credentials_error_message_includes_path():
|
||||||
|
"""ToolError from CredentialsNotConfiguredError includes the credentials path."""
|
||||||
|
import logging
|
||||||
|
|
||||||
|
from unraid_mcp.config.settings import CREDENTIALS_ENV_PATH
|
||||||
|
from unraid_mcp.core.exceptions import (
|
||||||
|
CredentialsNotConfiguredError,
|
||||||
|
ToolError,
|
||||||
|
tool_error_handler,
|
||||||
|
)
|
||||||
|
|
||||||
|
_log = logging.getLogger("test")
|
||||||
|
with pytest.raises(ToolError) as exc_info, tool_error_handler("docker", "list", _log):
|
||||||
|
raise CredentialsNotConfiguredError()
|
||||||
|
|
||||||
|
assert str(CREDENTIALS_ENV_PATH) in str(exc_info.value)
|
||||||
|
assert "setup" in str(exc_info.value).lower()
|
||||||
|
|||||||
@@ -41,9 +41,10 @@ def tool_error_handler(
|
|||||||
) -> Iterator[None]:
|
) -> Iterator[None]:
|
||||||
"""Context manager that standardizes tool error handling.
|
"""Context manager that standardizes tool error handling.
|
||||||
|
|
||||||
Re-raises ToolError as-is. Gives TimeoutError a descriptive message.
|
Re-raises ToolError as-is. Converts CredentialsNotConfiguredError to a ToolError
|
||||||
Catches all other exceptions, logs them with full traceback, and wraps them
|
with setup instructions including CREDENTIALS_ENV_PATH; does not log.
|
||||||
in ToolError with a descriptive message.
|
Gives TimeoutError a descriptive message. Catches all other exceptions,
|
||||||
|
logs them with full traceback, and wraps them in ToolError.
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
tool_name: The tool name for error messages (e.g., "docker", "vm").
|
tool_name: The tool name for error messages (e.g., "docker", "vm").
|
||||||
@@ -54,8 +55,14 @@ def tool_error_handler(
|
|||||||
yield
|
yield
|
||||||
except ToolError:
|
except ToolError:
|
||||||
raise
|
raise
|
||||||
except CredentialsNotConfiguredError:
|
except CredentialsNotConfiguredError as e:
|
||||||
raise # Let callers handle elicitation — do not wrap in ToolError
|
from ..config.settings import CREDENTIALS_ENV_PATH
|
||||||
|
|
||||||
|
raise ToolError(
|
||||||
|
f"Credentials not configured. Run unraid_health action=setup, "
|
||||||
|
f"or create {CREDENTIALS_ENV_PATH} with UNRAID_API_URL and UNRAID_API_KEY "
|
||||||
|
f"(cp .env.example {CREDENTIALS_ENV_PATH} to get started)."
|
||||||
|
) from e
|
||||||
except TimeoutError as e:
|
except TimeoutError as e:
|
||||||
logger.exception(
|
logger.exception(
|
||||||
f"Timeout in unraid_{tool_name} action={action}: request exceeded time limit"
|
f"Timeout in unraid_{tool_name} action={action}: request exceeded time limit"
|
||||||
|
|||||||
Reference in New Issue
Block a user