mirror of
https://github.com/jmagar/unraid-mcp.git
synced 2026-03-23 12:39:24 -07:00
feat(elicitation): raise CredentialsNotConfiguredError in client when creds absent
make_graphql_request now reads credentials from the settings module at call time (via a local import) instead of relying on module-level names captured at import time. When either credential is missing it raises CredentialsNotConfiguredError (not ToolError), allowing callers to trigger elicitation rather than surfacing a generic error to the MCP client. Updated tests/test_client.py and tests/http_layer/test_request_construction.py to patch unraid_mcp.config.settings.* instead of the now-removed client-module attrs, and to expect CredentialsNotConfiguredError on missing credentials.
This commit is contained in:
@@ -17,7 +17,7 @@ import respx
|
|||||||
|
|
||||||
from tests.conftest import make_tool_fn
|
from tests.conftest import make_tool_fn
|
||||||
from unraid_mcp.core.client import DEFAULT_TIMEOUT, DISK_TIMEOUT, make_graphql_request
|
from unraid_mcp.core.client import DEFAULT_TIMEOUT, DISK_TIMEOUT, make_graphql_request
|
||||||
from unraid_mcp.core.exceptions import ToolError
|
from unraid_mcp.core.exceptions import CredentialsNotConfiguredError, ToolError
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
@@ -32,8 +32,8 @@ API_KEY = "test-api-key-12345"
|
|||||||
def _patch_config():
|
def _patch_config():
|
||||||
"""Patch API URL and key for all tests in this module."""
|
"""Patch API URL and key for all tests in this module."""
|
||||||
with (
|
with (
|
||||||
patch("unraid_mcp.core.client.UNRAID_API_URL", API_URL),
|
patch("unraid_mcp.config.settings.UNRAID_API_URL", API_URL),
|
||||||
patch("unraid_mcp.core.client.UNRAID_API_KEY", API_KEY),
|
patch("unraid_mcp.config.settings.UNRAID_API_KEY", API_KEY),
|
||||||
):
|
):
|
||||||
yield
|
yield
|
||||||
|
|
||||||
@@ -1288,8 +1288,8 @@ class TestCrossCuttingConcerns:
|
|||||||
async def test_missing_api_url_raises_before_http_call(self) -> None:
|
async def test_missing_api_url_raises_before_http_call(self) -> None:
|
||||||
route = respx.post(API_URL).mock(return_value=_graphql_response({}))
|
route = respx.post(API_URL).mock(return_value=_graphql_response({}))
|
||||||
with (
|
with (
|
||||||
patch("unraid_mcp.core.client.UNRAID_API_URL", ""),
|
patch("unraid_mcp.config.settings.UNRAID_API_URL", ""),
|
||||||
pytest.raises(ToolError, match="UNRAID_API_URL not configured"),
|
pytest.raises(CredentialsNotConfiguredError),
|
||||||
):
|
):
|
||||||
await make_graphql_request("query { online }")
|
await make_graphql_request("query { online }")
|
||||||
assert not route.called
|
assert not route.called
|
||||||
@@ -1298,8 +1298,8 @@ class TestCrossCuttingConcerns:
|
|||||||
async def test_missing_api_key_raises_before_http_call(self) -> None:
|
async def test_missing_api_key_raises_before_http_call(self) -> None:
|
||||||
route = respx.post(API_URL).mock(return_value=_graphql_response({}))
|
route = respx.post(API_URL).mock(return_value=_graphql_response({}))
|
||||||
with (
|
with (
|
||||||
patch("unraid_mcp.core.client.UNRAID_API_KEY", ""),
|
patch("unraid_mcp.config.settings.UNRAID_API_KEY", ""),
|
||||||
pytest.raises(ToolError, match="UNRAID_API_KEY not configured"),
|
pytest.raises(CredentialsNotConfiguredError),
|
||||||
):
|
):
|
||||||
await make_graphql_request("query { online }")
|
await make_graphql_request("query { online }")
|
||||||
assert not route.called
|
assert not route.called
|
||||||
|
|||||||
@@ -16,7 +16,7 @@ from unraid_mcp.core.client import (
|
|||||||
make_graphql_request,
|
make_graphql_request,
|
||||||
redact_sensitive,
|
redact_sensitive,
|
||||||
)
|
)
|
||||||
from unraid_mcp.core.exceptions import ToolError
|
from unraid_mcp.core.exceptions import CredentialsNotConfiguredError, ToolError
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
@@ -173,8 +173,8 @@ class TestMakeGraphQLRequestSuccess:
|
|||||||
@pytest.fixture(autouse=True)
|
@pytest.fixture(autouse=True)
|
||||||
def _patch_config(self):
|
def _patch_config(self):
|
||||||
with (
|
with (
|
||||||
patch("unraid_mcp.core.client.UNRAID_API_URL", "https://unraid.local/graphql"),
|
patch("unraid_mcp.config.settings.UNRAID_API_URL", "https://unraid.local/graphql"),
|
||||||
patch("unraid_mcp.core.client.UNRAID_API_KEY", "test-key"),
|
patch("unraid_mcp.config.settings.UNRAID_API_KEY", "test-key"),
|
||||||
):
|
):
|
||||||
yield
|
yield
|
||||||
|
|
||||||
@@ -258,22 +258,22 @@ class TestMakeGraphQLRequestErrors:
|
|||||||
@pytest.fixture(autouse=True)
|
@pytest.fixture(autouse=True)
|
||||||
def _patch_config(self):
|
def _patch_config(self):
|
||||||
with (
|
with (
|
||||||
patch("unraid_mcp.core.client.UNRAID_API_URL", "https://unraid.local/graphql"),
|
patch("unraid_mcp.config.settings.UNRAID_API_URL", "https://unraid.local/graphql"),
|
||||||
patch("unraid_mcp.core.client.UNRAID_API_KEY", "test-key"),
|
patch("unraid_mcp.config.settings.UNRAID_API_KEY", "test-key"),
|
||||||
):
|
):
|
||||||
yield
|
yield
|
||||||
|
|
||||||
async def test_missing_api_url(self) -> None:
|
async def test_missing_api_url(self) -> None:
|
||||||
with (
|
with (
|
||||||
patch("unraid_mcp.core.client.UNRAID_API_URL", ""),
|
patch("unraid_mcp.config.settings.UNRAID_API_URL", ""),
|
||||||
pytest.raises(ToolError, match="UNRAID_API_URL not configured"),
|
pytest.raises(CredentialsNotConfiguredError),
|
||||||
):
|
):
|
||||||
await make_graphql_request("{ info }")
|
await make_graphql_request("{ info }")
|
||||||
|
|
||||||
async def test_missing_api_key(self) -> None:
|
async def test_missing_api_key(self) -> None:
|
||||||
with (
|
with (
|
||||||
patch("unraid_mcp.core.client.UNRAID_API_KEY", ""),
|
patch("unraid_mcp.config.settings.UNRAID_API_KEY", ""),
|
||||||
pytest.raises(ToolError, match="UNRAID_API_KEY not configured"),
|
pytest.raises(CredentialsNotConfiguredError),
|
||||||
):
|
):
|
||||||
await make_graphql_request("{ info }")
|
await make_graphql_request("{ info }")
|
||||||
|
|
||||||
@@ -377,8 +377,8 @@ class TestGraphQLErrorHandling:
|
|||||||
@pytest.fixture(autouse=True)
|
@pytest.fixture(autouse=True)
|
||||||
def _patch_config(self):
|
def _patch_config(self):
|
||||||
with (
|
with (
|
||||||
patch("unraid_mcp.core.client.UNRAID_API_URL", "https://unraid.local/graphql"),
|
patch("unraid_mcp.config.settings.UNRAID_API_URL", "https://unraid.local/graphql"),
|
||||||
patch("unraid_mcp.core.client.UNRAID_API_KEY", "test-key"),
|
patch("unraid_mcp.config.settings.UNRAID_API_KEY", "test-key"),
|
||||||
):
|
):
|
||||||
yield
|
yield
|
||||||
|
|
||||||
@@ -641,8 +641,8 @@ class TestRateLimitRetry:
|
|||||||
@pytest.fixture(autouse=True)
|
@pytest.fixture(autouse=True)
|
||||||
def _patch_config(self):
|
def _patch_config(self):
|
||||||
with (
|
with (
|
||||||
patch("unraid_mcp.core.client.UNRAID_API_URL", "https://unraid.local/graphql"),
|
patch("unraid_mcp.config.settings.UNRAID_API_URL", "https://unraid.local/graphql"),
|
||||||
patch("unraid_mcp.core.client.UNRAID_API_KEY", "test-key"),
|
patch("unraid_mcp.config.settings.UNRAID_API_KEY", "test-key"),
|
||||||
patch("unraid_mcp.core.client.asyncio.sleep", new_callable=AsyncMock),
|
patch("unraid_mcp.core.client.asyncio.sleep", new_callable=AsyncMock),
|
||||||
):
|
):
|
||||||
yield
|
yield
|
||||||
|
|||||||
@@ -154,3 +154,23 @@ async def test_elicit_and_configure_returns_false_on_cancel():
|
|||||||
|
|
||||||
result = await elicit_and_configure(mock_ctx)
|
result = await elicit_and_configure(mock_ctx)
|
||||||
assert result is False
|
assert result is False
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_make_graphql_request_raises_sentinel_when_unconfigured():
|
||||||
|
"""make_graphql_request raises CredentialsNotConfiguredError (not ToolError) when
|
||||||
|
credentials are absent, so callers can trigger elicitation."""
|
||||||
|
from unraid_mcp.config import settings as settings_mod
|
||||||
|
from unraid_mcp.core.client import make_graphql_request
|
||||||
|
from unraid_mcp.core.exceptions import CredentialsNotConfiguredError
|
||||||
|
|
||||||
|
original_url = settings_mod.UNRAID_API_URL
|
||||||
|
original_key = settings_mod.UNRAID_API_KEY
|
||||||
|
try:
|
||||||
|
settings_mod.UNRAID_API_URL = None
|
||||||
|
settings_mod.UNRAID_API_KEY = None
|
||||||
|
with pytest.raises(CredentialsNotConfiguredError):
|
||||||
|
await make_graphql_request("{ __typename }")
|
||||||
|
finally:
|
||||||
|
settings_mod.UNRAID_API_URL = original_url
|
||||||
|
settings_mod.UNRAID_API_KEY = original_key
|
||||||
|
|||||||
@@ -16,12 +16,10 @@ import httpx
|
|||||||
from ..config.logging import logger
|
from ..config.logging import logger
|
||||||
from ..config.settings import (
|
from ..config.settings import (
|
||||||
TIMEOUT_CONFIG,
|
TIMEOUT_CONFIG,
|
||||||
UNRAID_API_KEY,
|
|
||||||
UNRAID_API_URL,
|
|
||||||
UNRAID_VERIFY_SSL,
|
UNRAID_VERIFY_SSL,
|
||||||
VERSION,
|
VERSION,
|
||||||
)
|
)
|
||||||
from ..core.exceptions import ToolError
|
from ..core.exceptions import CredentialsNotConfiguredError, ToolError
|
||||||
from .utils import safe_display_url
|
from .utils import safe_display_url
|
||||||
|
|
||||||
|
|
||||||
@@ -313,13 +311,15 @@ async def make_graphql_request(
|
|||||||
Dict containing the GraphQL response data
|
Dict containing the GraphQL response data
|
||||||
|
|
||||||
Raises:
|
Raises:
|
||||||
|
CredentialsNotConfiguredError: When UNRAID_API_URL or UNRAID_API_KEY are absent at call time
|
||||||
ToolError: For HTTP errors, network errors, or non-idempotent GraphQL errors
|
ToolError: For HTTP errors, network errors, or non-idempotent GraphQL errors
|
||||||
"""
|
"""
|
||||||
if not UNRAID_API_URL:
|
# Local import to get current runtime values — module-level names are captured at import time
|
||||||
raise ToolError("UNRAID_API_URL not configured")
|
# and won't reflect runtime changes (e.g., after elicitation sets them via apply_runtime_config).
|
||||||
|
from ..config import settings as _settings
|
||||||
|
|
||||||
if not UNRAID_API_KEY:
|
if not _settings.UNRAID_API_URL or not _settings.UNRAID_API_KEY:
|
||||||
raise ToolError("UNRAID_API_KEY not configured")
|
raise CredentialsNotConfiguredError()
|
||||||
|
|
||||||
# Check TTL cache — short-circuits rate limiter on hits
|
# Check TTL cache — short-circuits rate limiter on hits
|
||||||
is_mutation = query.lstrip().startswith("mutation")
|
is_mutation = query.lstrip().startswith("mutation")
|
||||||
@@ -331,14 +331,14 @@ async def make_graphql_request(
|
|||||||
|
|
||||||
headers = {
|
headers = {
|
||||||
"Content-Type": "application/json",
|
"Content-Type": "application/json",
|
||||||
"X-API-Key": UNRAID_API_KEY,
|
"X-API-Key": _settings.UNRAID_API_KEY,
|
||||||
}
|
}
|
||||||
|
|
||||||
payload: dict[str, Any] = {"query": query}
|
payload: dict[str, Any] = {"query": query}
|
||||||
if variables:
|
if variables:
|
||||||
payload["variables"] = variables
|
payload["variables"] = variables
|
||||||
|
|
||||||
logger.debug(f"Making GraphQL request to {safe_display_url(UNRAID_API_URL)}:")
|
logger.debug(f"Making GraphQL request to {safe_display_url(_settings.UNRAID_API_URL)}:")
|
||||||
logger.debug(f"Query: {query[:200]}{'...' if len(query) > 200 else ''}") # Log truncated query
|
logger.debug(f"Query: {query[:200]}{'...' if len(query) > 200 else ''}") # Log truncated query
|
||||||
if variables:
|
if variables:
|
||||||
logger.debug(f"Variables: {redact_sensitive(variables)}")
|
logger.debug(f"Variables: {redact_sensitive(variables)}")
|
||||||
@@ -357,7 +357,7 @@ async def make_graphql_request(
|
|||||||
|
|
||||||
response: httpx.Response | None = None
|
response: httpx.Response | None = None
|
||||||
for attempt in range(3):
|
for attempt in range(3):
|
||||||
response = await client.post(UNRAID_API_URL, **post_kwargs)
|
response = await client.post(_settings.UNRAID_API_URL, **post_kwargs)
|
||||||
if response.status_code == 429:
|
if response.status_code == 429:
|
||||||
backoff = 2**attempt
|
backoff = 2**attempt
|
||||||
logger.warning(
|
logger.warning(
|
||||||
|
|||||||
Reference in New Issue
Block a user