From 4ef2f50e724ac7c9c8bb00a0117a91c2e0cb25c8 Mon Sep 17 00:00:00 2001 From: daniel Date: Sun, 29 Jun 2025 23:45:00 +0200 Subject: [PATCH] feat: better error handling. --- src/mcp_cordra/client.py | 68 ++++++++++++++++++++++++++-------------- src/mcp_cordra/server.py | 19 ++++++++--- tests/test_client.py | 4 ++- 3 files changed, 63 insertions(+), 28 deletions(-) diff --git a/src/mcp_cordra/client.py b/src/mcp_cordra/client.py index 384e2f8..2f9789a 100644 --- a/src/mcp_cordra/client.py +++ b/src/mcp_cordra/client.py @@ -32,6 +32,11 @@ class CordraNotFoundError(CordraClientError): pass +class CordraAuthenticationError(CordraClientError): + """Exception raised for authentication/authorization failures.""" + pass + + class CordraClient: """Client for interacting with Cordra repository using HTTP requests.""" @@ -51,6 +56,28 @@ class CordraClient: elif config.username or config.password: logger.warning("Only username or password provided, not both. Authentication may fail.") + def _handle_http_error(self, response: requests.Response, context: str) -> None: + """Handle HTTP errors and raise appropriate exceptions. + + Args: + response: The HTTP response object + context: Context description for the error message + + Raises: + CordraNotFoundError: For 404 errors + CordraAuthenticationError: For 401/403 errors + CordraClientError: For other HTTP errors + """ + status_code = response.status_code + if status_code == 404: + raise CordraNotFoundError(f"{context}: Resource not found") + elif status_code in (401, 403): + raise CordraAuthenticationError(f"{context}: Authentication failed (HTTP {status_code})") + elif status_code >= 500: + raise CordraClientError(f"{context}: Server error (HTTP {status_code})") + else: + raise CordraClientError(f"{context}: HTTP error {status_code}") + async def get_object(self, object_id: str) -> DigitalObject: """Retrieve a digital object by its ID. @@ -61,24 +88,22 @@ class CordraClient: The full digital object Raises: + ValueError: If object_id is empty CordraNotFoundError: If the object is not found + CordraAuthenticationError: If authentication fails CordraClientError: For other API errors """ + url = f"{self.config.cordra_url}/objects/{object_id}" + params = {"full": "true"} + try: - # Build URL: cordra_base_url/objects/prefix/postfix - url = f"{self.config.cordra_url}/objects/{object_id}" - - # Add full=true parameter to get complete object details - params = {"full": "true"} - response = self.session.get(url, params=params, timeout=self.config.timeout) - if response.status_code == 404: - raise CordraNotFoundError(f"Object not found: {object_id}") + if not response.ok: + self._handle_http_error(response, f"Failed to retrieve object {object_id}") - response.raise_for_status() cordra_obj = response.json() - + return DigitalObject( id=object_id, type=cordra_obj.get('type', ''), @@ -87,13 +112,9 @@ class CordraClient: acl=cordra_obj.get('acl'), payloads=cordra_obj.get('payloads'), ) - - except CordraNotFoundError: - raise + except requests.RequestException as e: raise CordraClientError(f"Failed to retrieve object {object_id}: {e}") from e - except Exception as e: - raise CordraClientError(f"Failed to retrieve object {object_id}: {e}") from e async def find(self, query: str) -> list[dict[str, Any]]: """Find objects using a Cordra query. @@ -105,15 +126,18 @@ class CordraClient: List of objects matching the query as dictionaries Raises: - CordraClientError: If there's an API error + ValueError: If query is empty + CordraAuthenticationError: If authentication fails + CordraClientError: For other API errors """ + url = f"{self.config.cordra_url}/search" + params = {"query": query} + try: - # Use HTTP GET request to search endpoint - url = f"{self.config.cordra_url}/search" - params = {"query": query} - response = self.session.get(url, params=params, timeout=self.config.timeout) - response.raise_for_status() + + if not response.ok: + self._handle_http_error(response, f"Failed to search with query '{query}'") search_result = response.json() @@ -124,6 +148,4 @@ class CordraClient: return [] except requests.RequestException as e: - raise CordraClientError(f"Failed to search with query '{query}': {e}") from e - except Exception as e: raise CordraClientError(f"Failed to search with query '{query}': {e}") from e \ No newline at end of file diff --git a/src/mcp_cordra/server.py b/src/mcp_cordra/server.py index aad3921..bcb92c6 100644 --- a/src/mcp_cordra/server.py +++ b/src/mcp_cordra/server.py @@ -5,7 +5,12 @@ import json from mcp.server.fastmcp import FastMCP -from .client import CordraClient, CordraClientError, CordraNotFoundError +from .client import ( + CordraClient, + CordraClientError, + CordraNotFoundError, + CordraAuthenticationError, +) from .config import CordraConfig # Initialize the MCP server @@ -36,8 +41,12 @@ async def get_cordra_object(prefix: str, suffix: str) -> str: object_dict = digital_object.model_dump() return json.dumps(object_dict, indent=2) - except CordraNotFoundError as e: - raise RuntimeError(f"Object not found: {object_id}") from e + except ValueError as e: + raise RuntimeError(f"Invalid parameters: {e}") from e + except CordraNotFoundError: + raise RuntimeError(f"Object not found: {object_id}") + except CordraAuthenticationError as e: + raise RuntimeError(f"Authentication failed: {e}") from e except CordraClientError as e: raise RuntimeError(f"Failed to retrieve object {object_id}: {e}") from e @@ -68,7 +77,9 @@ async def list_cordra_schemas() -> str: } return json.dumps(result, indent=2) - except Exception as e: + except CordraAuthenticationError as e: + raise RuntimeError(f"Authentication failed: {e}") from e + except CordraClientError as e: raise RuntimeError(f"Failed to list schemas: {e}") from e diff --git a/tests/test_client.py b/tests/test_client.py index 07c3466..2662939 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -8,6 +8,7 @@ from mcp_cordra.client import ( CordraClient, CordraClientError, CordraNotFoundError, + CordraAuthenticationError, DigitalObject, ) from mcp_cordra.config import CordraConfig @@ -141,11 +142,12 @@ class TestCordraClient: """Test object not found exception.""" mock_response = mock_get.return_value mock_response.status_code = 404 + mock_response.ok = False with pytest.raises(CordraNotFoundError) as exc_info: await client.get_object("test/nonexistent") - assert "Object not found: test/nonexistent" in str(exc_info.value) + assert "Resource not found" in str(exc_info.value) @patch('mcp_cordra.client.requests.Session.get') async def test_get_object_general_error(self, mock_get, client):