From ed6479b4c38a6fca0d4d866e5aaba0da89ca2035 Mon Sep 17 00:00:00 2001 From: Rasmus Widing Date: Tue, 19 Aug 2025 16:07:07 +0300 Subject: [PATCH] refactor(mcp): Apply consistent error handling to all MCP tools Comprehensive update to MCP server error handling: Error Handling Improvements: - Applied MCPErrorFormatter to all remaining MCP tool files - Replaced all hardcoded timeout values with configurable timeout system - Converted all simple string errors to structured error format - Added proper httpx exception handling with detailed context Tools Updated: - document_tools.py: All 5 document management tools - version_tools.py: All 4 version management tools - feature_tools.py: Project features tool - project_tools.py: Remaining 3 project tools (get, list, delete) - task_tools.py: Remaining 4 task tools (get, list, update, delete) Test Improvements: - Removed backward compatibility checks from all tests - Tests now enforce structured error format (dict not string) - Any string error response is now considered a bug - All 20 tests passing with new strict validation This completes the error handling refactor for all MCP tools, ensuring consistent client experience and better debugging. --- .../features/documents/document_tools.py | 81 +++++++---- .../features/documents/version_tools.py | 129 +++++++++++------- .../src/mcp_server/features/feature_tools.py | 24 ++-- .../features/projects/project_tools.py | 66 +++++---- .../mcp_server/features/tasks/task_tools.py | 68 +++++---- .../features/documents/test_document_tools.py | 6 +- .../features/documents/test_version_tools.py | 5 +- .../features/projects/test_project_tools.py | 6 +- .../features/tasks/test_task_tools.py | 6 +- .../mcp_server/features/test_feature_tools.py | 6 +- 10 files changed, 255 insertions(+), 142 deletions(-) diff --git a/python/src/mcp_server/features/documents/document_tools.py b/python/src/mcp_server/features/documents/document_tools.py index 405e5360..ceeebf93 100644 --- a/python/src/mcp_server/features/documents/document_tools.py +++ b/python/src/mcp_server/features/documents/document_tools.py @@ -13,6 +13,8 @@ from urllib.parse import urljoin import httpx from mcp.server.fastmcp import Context, FastMCP +from src.mcp_server.utils.error_handling import MCPErrorFormatter +from src.mcp_server.utils.timeout_config import get_default_timeout from src.server.config.service_discovery import get_api_url logger = logging.getLogger(__name__) @@ -90,7 +92,7 @@ def register_document_tools(mcp: FastMCP): """ try: api_url = get_api_url() - timeout = httpx.Timeout(30.0, connect=5.0) + timeout = get_default_timeout() async with httpx.AsyncClient(timeout=timeout) as client: response = await client.post( @@ -113,12 +115,15 @@ def register_document_tools(mcp: FastMCP): "message": result.get("message", "Document created successfully"), }) else: - error_detail = response.text - return json.dumps({"success": False, "error": error_detail}) + return MCPErrorFormatter.from_http_error(response, "create document") + except httpx.RequestError as e: + return MCPErrorFormatter.from_exception( + e, "create document", {"project_id": project_id, "title": title} + ) except Exception as e: - logger.error(f"Error creating document: {e}") - return json.dumps({"success": False, "error": str(e)}) + logger.error(f"Error creating document: {e}", exc_info=True) + return MCPErrorFormatter.from_exception(e, "create document") @mcp.tool() async def list_documents(ctx: Context, project_id: str) -> str: @@ -136,7 +141,7 @@ def register_document_tools(mcp: FastMCP): """ try: api_url = get_api_url() - timeout = httpx.Timeout(30.0, connect=5.0) + timeout = get_default_timeout() async with httpx.AsyncClient(timeout=timeout) as client: response = await client.get(urljoin(api_url, f"/api/projects/{project_id}/docs")) @@ -149,14 +154,15 @@ def register_document_tools(mcp: FastMCP): "count": len(result.get("documents", [])), }) else: - return json.dumps({ - "success": False, - "error": f"HTTP {response.status_code}: {response.text}", - }) + return MCPErrorFormatter.from_http_error(response, "list documents") + except httpx.RequestError as e: + return MCPErrorFormatter.from_exception( + e, "list documents", {"project_id": project_id} + ) except Exception as e: - logger.error(f"Error listing documents: {e}") - return json.dumps({"success": False, "error": str(e)}) + logger.error(f"Error listing documents: {e}", exc_info=True) + return MCPErrorFormatter.from_exception(e, "list documents") @mcp.tool() async def get_document(ctx: Context, project_id: str, doc_id: str) -> str: @@ -175,7 +181,7 @@ def register_document_tools(mcp: FastMCP): """ try: api_url = get_api_url() - timeout = httpx.Timeout(30.0, connect=5.0) + timeout = get_default_timeout() async with httpx.AsyncClient(timeout=timeout) as client: response = await client.get( @@ -186,13 +192,22 @@ def register_document_tools(mcp: FastMCP): document = response.json() return json.dumps({"success": True, "document": document}) elif response.status_code == 404: - return json.dumps({"success": False, "error": f"Document {doc_id} not found"}) + return MCPErrorFormatter.format_error( + error_type="not_found", + message=f"Document {doc_id} not found", + suggestion="Verify the document ID is correct and exists in this project", + http_status=404, + ) else: - return json.dumps({"success": False, "error": "Failed to get document"}) + return MCPErrorFormatter.from_http_error(response, "get document") + except httpx.RequestError as e: + return MCPErrorFormatter.from_exception( + e, "get document", {"project_id": project_id, "doc_id": doc_id} + ) except Exception as e: - logger.error(f"Error getting document: {e}") - return json.dumps({"success": False, "error": str(e)}) + logger.error(f"Error getting document: {e}", exc_info=True) + return MCPErrorFormatter.from_exception(e, "get document") @mcp.tool() async def update_document( @@ -224,7 +239,7 @@ def register_document_tools(mcp: FastMCP): """ try: api_url = get_api_url() - timeout = httpx.Timeout(30.0, connect=5.0) + timeout = get_default_timeout() # Build update fields update_fields: Dict[str, Any] = {} @@ -251,12 +266,15 @@ def register_document_tools(mcp: FastMCP): "message": result.get("message", "Document updated successfully"), }) else: - error_detail = response.text - return json.dumps({"success": False, "error": error_detail}) + return MCPErrorFormatter.from_http_error(response, "update document") + except httpx.RequestError as e: + return MCPErrorFormatter.from_exception( + e, "update document", {"project_id": project_id, "doc_id": doc_id} + ) except Exception as e: - logger.error(f"Error updating document: {e}") - return json.dumps({"success": False, "error": str(e)}) + logger.error(f"Error updating document: {e}", exc_info=True) + return MCPErrorFormatter.from_exception(e, "update document") @mcp.tool() async def delete_document(ctx: Context, project_id: str, doc_id: str) -> str: @@ -275,7 +293,7 @@ def register_document_tools(mcp: FastMCP): """ try: api_url = get_api_url() - timeout = httpx.Timeout(30.0, connect=5.0) + timeout = get_default_timeout() async with httpx.AsyncClient(timeout=timeout) as client: response = await client.delete( @@ -289,10 +307,19 @@ def register_document_tools(mcp: FastMCP): "message": result.get("message", f"Document {doc_id} deleted successfully"), }) elif response.status_code == 404: - return json.dumps({"success": False, "error": f"Document {doc_id} not found"}) + return MCPErrorFormatter.format_error( + error_type="not_found", + message=f"Document {doc_id} not found", + suggestion="Verify the document ID is correct and exists in this project", + http_status=404, + ) else: - return json.dumps({"success": False, "error": "Failed to delete document"}) + return MCPErrorFormatter.from_http_error(response, "delete document") + except httpx.RequestError as e: + return MCPErrorFormatter.from_exception( + e, "delete document", {"project_id": project_id, "doc_id": doc_id} + ) except Exception as e: - logger.error(f"Error deleting document: {e}") - return json.dumps({"success": False, "error": str(e)}) + logger.error(f"Error deleting document: {e}", exc_info=True) + return MCPErrorFormatter.from_exception(e, "delete document") diff --git a/python/src/mcp_server/features/documents/version_tools.py b/python/src/mcp_server/features/documents/version_tools.py index 1c88d458..041917a7 100644 --- a/python/src/mcp_server/features/documents/version_tools.py +++ b/python/src/mcp_server/features/documents/version_tools.py @@ -13,6 +13,8 @@ from urllib.parse import urljoin import httpx from mcp.server.fastmcp import Context, FastMCP +from src.mcp_server.utils.error_handling import MCPErrorFormatter +from src.mcp_server.utils.timeout_config import get_default_timeout from src.server.config.service_discovery import get_api_url logger = logging.getLogger(__name__) @@ -89,7 +91,7 @@ def register_version_tools(mcp: FastMCP): """ try: api_url = get_api_url() - timeout = httpx.Timeout(30.0, connect=5.0) + timeout = get_default_timeout() async with httpx.AsyncClient(timeout=timeout) as client: response = await client.post( @@ -116,41 +118,57 @@ def register_version_tools(mcp: FastMCP): elif response.status_code == 400: error_text = response.text.lower() if "invalid field_name" in error_text: - return json.dumps({ - "success": False, - "error": f"Invalid field_name '{field_name}'. Must be one of: docs, features, data, or prd" - }) + return MCPErrorFormatter.format_error( + error_type="validation_error", + message=f"Invalid field_name '{field_name}'. Must be one of: docs, features, data, or prd", + suggestion="Use one of the valid field names: docs, features, data, or prd", + http_status=400 + ) elif "content" in error_text and "required" in error_text: - return json.dumps({ - "success": False, - "error": "Content is required and cannot be empty. Provide the complete data to version." - }) + return MCPErrorFormatter.format_error( + error_type="validation_error", + message="Content is required and cannot be empty. Provide the complete data to version.", + suggestion="Provide the complete data to version", + http_status=400 + ) elif "format" in error_text or "type" in error_text: if field_name == "docs": - return json.dumps({ - "success": False, - "error": f"For field_name='docs', content must be an array. Example: [{{'id': 'doc1', 'title': 'Guide', 'content': {{...}}}}]" - }) + return MCPErrorFormatter.format_error( + error_type="validation_error", + message=f"For field_name='docs', content must be an array. Example: [{{'id': 'doc1', 'title': 'Guide', 'content': {{...}}}}]", + suggestion="Ensure content is an array of document objects", + http_status=400 + ) else: - return json.dumps({ - "success": False, - "error": f"For field_name='{field_name}', content must be a dictionary/object. Example: {{'key': 'value'}}" - }) - return json.dumps({"success": False, "error": f"Bad request: {response.text}"}) + return MCPErrorFormatter.format_error( + error_type="validation_error", + message=f"For field_name='{field_name}', content must be a dictionary/object. Example: {{'key': 'value'}}", + suggestion="Ensure content is a dictionary/object", + http_status=400 + ) + return MCPErrorFormatter.format_error( + error_type="validation_error", + message=f"Invalid request: {response.text}", + suggestion="Check that all required fields are provided and valid", + http_status=400, + ) elif response.status_code == 404: - return json.dumps({ - "success": False, - "error": f"Project {project_id} not found. Please check the project ID." - }) + return MCPErrorFormatter.format_error( + error_type="not_found", + message=f"Project {project_id} not found", + suggestion="Please check the project ID is correct", + http_status=404, + ) else: - return json.dumps({ - "success": False, - "error": f"Failed to create version (HTTP {response.status_code}): {response.text}" - }) + return MCPErrorFormatter.from_http_error(response, "create version") + except httpx.RequestError as e: + return MCPErrorFormatter.from_exception( + e, "create version", {"project_id": project_id, "field_name": field_name} + ) except Exception as e: - logger.error(f"Error creating version: {e}") - return json.dumps({"success": False, "error": str(e)}) + logger.error(f"Error creating version: {e}", exc_info=True) + return MCPErrorFormatter.from_exception(e, "create version") @mcp.tool() async def list_versions( @@ -173,7 +191,7 @@ def register_version_tools(mcp: FastMCP): """ try: api_url = get_api_url() - timeout = httpx.Timeout(30.0, connect=5.0) + timeout = get_default_timeout() params = {} if field_name: @@ -193,14 +211,15 @@ def register_version_tools(mcp: FastMCP): "count": len(result.get("versions", [])) }) else: - return json.dumps({ - "success": False, - "error": f"HTTP {response.status_code}: {response.text}" - }) + return MCPErrorFormatter.from_http_error(response, "list versions") + except httpx.RequestError as e: + return MCPErrorFormatter.from_exception( + e, "list versions", {"project_id": project_id, "field_name": field_name} + ) except Exception as e: - logger.error(f"Error listing versions: {e}") - return json.dumps({"success": False, "error": str(e)}) + logger.error(f"Error listing versions: {e}", exc_info=True) + return MCPErrorFormatter.from_exception(e, "list versions") @mcp.tool() async def get_version( @@ -225,7 +244,7 @@ def register_version_tools(mcp: FastMCP): """ try: api_url = get_api_url() - timeout = httpx.Timeout(30.0, connect=5.0) + timeout = get_default_timeout() async with httpx.AsyncClient(timeout=timeout) as client: response = await client.get( @@ -245,14 +264,16 @@ def register_version_tools(mcp: FastMCP): "error": f"Version {version_number} not found for field {field_name}" }) else: - return json.dumps({ - "success": False, - "error": "Failed to get version" - }) + return MCPErrorFormatter.from_http_error(response, "get version") + except httpx.RequestError as e: + return MCPErrorFormatter.from_exception( + e, "get version", + {"project_id": project_id, "field_name": field_name, "version_number": version_number} + ) except Exception as e: - logger.error(f"Error getting version: {e}") - return json.dumps({"success": False, "error": str(e)}) + logger.error(f"Error getting version: {e}", exc_info=True) + return MCPErrorFormatter.from_exception(e, "get version") @mcp.tool() async def restore_version( @@ -279,7 +300,7 @@ def register_version_tools(mcp: FastMCP): """ try: api_url = get_api_url() - timeout = httpx.Timeout(30.0, connect=5.0) + timeout = get_default_timeout() async with httpx.AsyncClient(timeout=timeout) as client: response = await client.post( @@ -294,14 +315,20 @@ def register_version_tools(mcp: FastMCP): "message": result.get("message", f"Version {version_number} restored successfully") }) elif response.status_code == 404: - return json.dumps({ - "success": False, - "error": f"Version {version_number} not found for field {field_name}" - }) + return MCPErrorFormatter.format_error( + error_type="not_found", + message=f"Version {version_number} not found for field {field_name}", + suggestion="Check that the version number exists for this field", + http_status=404, + ) else: - error_detail = response.text - return json.dumps({"success": False, "error": error_detail}) + return MCPErrorFormatter.from_http_error(response, "restore version") + except httpx.RequestError as e: + return MCPErrorFormatter.from_exception( + e, "restore version", + {"project_id": project_id, "field_name": field_name, "version_number": version_number} + ) except Exception as e: - logger.error(f"Error restoring version: {e}") - return json.dumps({"success": False, "error": str(e)}) \ No newline at end of file + logger.error(f"Error restoring version: {e}", exc_info=True) + return MCPErrorFormatter.from_exception(e, "restore version") \ No newline at end of file diff --git a/python/src/mcp_server/features/feature_tools.py b/python/src/mcp_server/features/feature_tools.py index ff5ef097..0a73a539 100644 --- a/python/src/mcp_server/features/feature_tools.py +++ b/python/src/mcp_server/features/feature_tools.py @@ -11,6 +11,8 @@ from urllib.parse import urljoin import httpx from mcp.server.fastmcp import Context, FastMCP +from src.mcp_server.utils.error_handling import MCPErrorFormatter +from src.mcp_server.utils.timeout_config import get_default_timeout from src.server.config.service_discovery import get_api_url logger = logging.getLogger(__name__) @@ -70,7 +72,7 @@ def register_feature_tools(mcp: FastMCP): """ try: api_url = get_api_url() - timeout = httpx.Timeout(30.0, connect=5.0) + timeout = get_default_timeout() async with httpx.AsyncClient(timeout=timeout) as client: response = await client.get( @@ -85,13 +87,19 @@ def register_feature_tools(mcp: FastMCP): "count": len(result.get("features", [])), }) elif response.status_code == 404: - return json.dumps({ - "success": False, - "error": f"Project {project_id} not found", - }) + return MCPErrorFormatter.format_error( + error_type="not_found", + message=f"Project {project_id} not found", + suggestion="Verify the project ID is correct", + http_status=404, + ) else: - return json.dumps({"success": False, "error": "Failed to get project features"}) + return MCPErrorFormatter.from_http_error(response, "get project features") + except httpx.RequestError as e: + return MCPErrorFormatter.from_exception( + e, "get project features", {"project_id": project_id} + ) except Exception as e: - logger.error(f"Error getting project features: {e}") - return json.dumps({"success": False, "error": str(e)}) + logger.error(f"Error getting project features: {e}", exc_info=True) + return MCPErrorFormatter.from_exception(e, "get project features") diff --git a/python/src/mcp_server/features/projects/project_tools.py b/python/src/mcp_server/features/projects/project_tools.py index dfd97d8d..f533cf1f 100644 --- a/python/src/mcp_server/features/projects/project_tools.py +++ b/python/src/mcp_server/features/projects/project_tools.py @@ -201,7 +201,7 @@ def register_project_tools(mcp: FastMCP): """ try: api_url = get_api_url() - timeout = httpx.Timeout(30.0, connect=5.0) + timeout = get_default_timeout() async with httpx.AsyncClient(timeout=timeout) as client: response = await client.get(urljoin(api_url, f"/api/projects/{project_id}")) @@ -210,16 +210,20 @@ def register_project_tools(mcp: FastMCP): project = response.json() return json.dumps({"success": True, "project": project}) elif response.status_code == 404: - return json.dumps({ - "success": False, - "error": f"Project {project_id} not found", - }) + return MCPErrorFormatter.format_error( + error_type="not_found", + message=f"Project {project_id} not found", + suggestion="Verify the project ID is correct", + http_status=404, + ) else: - return json.dumps({"success": False, "error": "Failed to get project"}) + return MCPErrorFormatter.from_http_error(response, "get project") + except httpx.RequestError as e: + return MCPErrorFormatter.from_exception(e, "get project", {"project_id": project_id}) except Exception as e: - logger.error(f"Error getting project: {e}") - return json.dumps({"success": False, "error": str(e)}) + logger.error(f"Error getting project: {e}", exc_info=True) + return MCPErrorFormatter.from_exception(e, "get project") @mcp.tool() async def delete_project(ctx: Context, project_id: str) -> str: @@ -237,7 +241,7 @@ def register_project_tools(mcp: FastMCP): """ try: api_url = get_api_url() - timeout = httpx.Timeout(30.0, connect=5.0) + timeout = get_default_timeout() async with httpx.AsyncClient(timeout=timeout) as client: response = await client.delete(urljoin(api_url, f"/api/projects/{project_id}")) @@ -248,16 +252,20 @@ def register_project_tools(mcp: FastMCP): "message": f"Project {project_id} deleted successfully", }) elif response.status_code == 404: - return json.dumps({ - "success": False, - "error": f"Project {project_id} not found", - }) + return MCPErrorFormatter.format_error( + error_type="not_found", + message=f"Project {project_id} not found", + suggestion="Verify the project ID is correct", + http_status=404, + ) else: - return json.dumps({"success": False, "error": "Failed to delete project"}) + return MCPErrorFormatter.from_http_error(response, "delete project") + except httpx.RequestError as e: + return MCPErrorFormatter.from_exception(e, "delete project", {"project_id": project_id}) except Exception as e: - logger.error(f"Error deleting project: {e}") - return json.dumps({"success": False, "error": str(e)}) + logger.error(f"Error deleting project: {e}", exc_info=True) + return MCPErrorFormatter.from_exception(e, "delete project") @mcp.tool() async def update_project( @@ -285,7 +293,7 @@ def register_project_tools(mcp: FastMCP): """ try: api_url = get_api_url() - timeout = httpx.Timeout(30.0, connect=5.0) + timeout = get_default_timeout() # Build update payload with only provided fields update_data = {} @@ -297,7 +305,11 @@ def register_project_tools(mcp: FastMCP): update_data["github_repo"] = github_repo if not update_data: - return json.dumps({"success": False, "error": "No fields to update"}) + return MCPErrorFormatter.format_error( + error_type="validation_error", + message="No fields to update", + suggestion="Provide at least one field to update (title, description, or github_repo)", + ) async with httpx.AsyncClient(timeout=timeout) as client: response = await client.put( @@ -312,13 +324,17 @@ def register_project_tools(mcp: FastMCP): "message": "Project updated successfully", }) elif response.status_code == 404: - return json.dumps({ - "success": False, - "error": f"Project {project_id} not found", - }) + return MCPErrorFormatter.format_error( + error_type="not_found", + message=f"Project {project_id} not found", + suggestion="Verify the project ID is correct", + http_status=404, + ) else: - return json.dumps({"success": False, "error": "Failed to update project"}) + return MCPErrorFormatter.from_http_error(response, "update project") + except httpx.RequestError as e: + return MCPErrorFormatter.from_exception(e, "update project", {"project_id": project_id}) except Exception as e: - logger.error(f"Error updating project: {e}") - return json.dumps({"success": False, "error": str(e)}) + logger.error(f"Error updating project: {e}", exc_info=True) + return MCPErrorFormatter.from_exception(e, "update project") diff --git a/python/src/mcp_server/features/tasks/task_tools.py b/python/src/mcp_server/features/tasks/task_tools.py index 05e18df1..64879bd9 100644 --- a/python/src/mcp_server/features/tasks/task_tools.py +++ b/python/src/mcp_server/features/tasks/task_tools.py @@ -138,12 +138,15 @@ def register_task_tools(mcp: FastMCP): "message": result.get("message", "Task created successfully"), }) else: - error_detail = response.text - return json.dumps({"success": False, "error": error_detail}) + return MCPErrorFormatter.from_http_error(response, "create task") + except httpx.RequestError as e: + return MCPErrorFormatter.from_exception( + e, "create task", {"project_id": project_id, "title": title} + ) except Exception as e: - logger.error(f"Error creating task: {e}") - return json.dumps({"success": False, "error": str(e)}) + logger.error(f"Error creating task: {e}", exc_info=True) + return MCPErrorFormatter.from_exception(e, "create task") @mcp.tool() async def list_tasks( @@ -278,13 +281,20 @@ def register_task_tools(mcp: FastMCP): task = response.json() return json.dumps({"success": True, "task": task}) elif response.status_code == 404: - return json.dumps({"success": False, "error": f"Task {task_id} not found"}) + return MCPErrorFormatter.format_error( + error_type="not_found", + message=f"Task {task_id} not found", + suggestion="Verify the task ID is correct", + http_status=404, + ) else: - return json.dumps({"success": False, "error": "Failed to get task"}) + return MCPErrorFormatter.from_http_error(response, "get task") + except httpx.RequestError as e: + return MCPErrorFormatter.from_exception(e, "get task", {"task_id": task_id}) except Exception as e: - logger.error(f"Error getting task: {e}") - return json.dumps({"success": False, "error": str(e)}) + logger.error(f"Error getting task: {e}", exc_info=True) + return MCPErrorFormatter.from_exception(e, "get task") @mcp.tool() async def update_task( @@ -323,12 +333,15 @@ def register_task_tools(mcp: FastMCP): "message": result.get("message", "Task updated successfully"), }) else: - error_detail = response.text - return json.dumps({"success": False, "error": error_detail}) + return MCPErrorFormatter.from_http_error(response, "update task") + except httpx.RequestError as e: + return MCPErrorFormatter.from_exception( + e, "update task", {"task_id": task_id, "update_fields": list(update_fields.keys())} + ) except Exception as e: - logger.error(f"Error updating task: {e}") - return json.dumps({"success": False, "error": str(e)}) + logger.error(f"Error updating task: {e}", exc_info=True) + return MCPErrorFormatter.from_exception(e, "update task") @mcp.tool() async def delete_task(ctx: Context, task_id: str) -> str: @@ -375,21 +388,24 @@ def register_task_tools(mcp: FastMCP): # More specific error for bad requests error_text = response.text if "already archived" in error_text.lower(): - return json.dumps({ - "success": False, - "error": f"Task {task_id} is already archived. No further action needed." - }) - return json.dumps({ - "success": False, - "error": f"Cannot delete task: {error_text}" - }) + return MCPErrorFormatter.format_error( + error_type="already_archived", + message=f"Task {task_id} is already archived", + suggestion="No further action needed - task is already archived", + http_status=400, + ) + return MCPErrorFormatter.format_error( + error_type="validation_error", + message=f"Cannot delete task: {error_text}", + suggestion="Check if the task meets deletion requirements", + http_status=400, + ) else: - return json.dumps({ - "success": False, - "error": f"Failed to delete task (HTTP {response.status_code}): {response.text}" - }) + return MCPErrorFormatter.from_http_error(response, "delete task") + except httpx.RequestError as e: + return MCPErrorFormatter.from_exception(e, "delete task", {"task_id": task_id}) except Exception as e: - logger.error(f"Error deleting task: {e}") - return json.dumps({"success": False, "error": str(e)}) + logger.error(f"Error deleting task: {e}", exc_info=True) + return MCPErrorFormatter.from_exception(e, "delete task") diff --git a/python/tests/mcp_server/features/documents/test_document_tools.py b/python/tests/mcp_server/features/documents/test_document_tools.py index 27611b10..45754f04 100644 --- a/python/tests/mcp_server/features/documents/test_document_tools.py +++ b/python/tests/mcp_server/features/documents/test_document_tools.py @@ -169,4 +169,8 @@ async def test_delete_document_not_found(mock_mcp, mock_context): result_data = json.loads(result) assert result_data["success"] is False - assert "not found" in result_data["error"] \ No newline at end of file + # Error must be structured format (dict), not string + assert "error" in result_data + assert isinstance(result_data["error"], dict), "Error should be structured format, not string" + assert result_data["error"]["type"] == "not_found" + assert "not found" in result_data["error"]["message"].lower() \ No newline at end of file diff --git a/python/tests/mcp_server/features/documents/test_version_tools.py b/python/tests/mcp_server/features/documents/test_version_tools.py index 3390e748..abb6834c 100644 --- a/python/tests/mcp_server/features/documents/test_version_tools.py +++ b/python/tests/mcp_server/features/documents/test_version_tools.py @@ -95,7 +95,10 @@ async def test_create_version_invalid_field(mock_mcp, mock_context): result_data = json.loads(result) assert result_data["success"] is False - assert "Must be one of: docs, features, data, or prd" in result_data["error"] + # Error must be structured format (dict), not string + assert "error" in result_data + assert isinstance(result_data["error"], dict), "Error should be structured format, not string" + assert result_data["error"]["type"] == "validation_error" @pytest.mark.asyncio diff --git a/python/tests/mcp_server/features/projects/test_project_tools.py b/python/tests/mcp_server/features/projects/test_project_tools.py index fd040941..17d6129b 100644 --- a/python/tests/mcp_server/features/projects/test_project_tools.py +++ b/python/tests/mcp_server/features/projects/test_project_tools.py @@ -171,4 +171,8 @@ async def test_get_project_not_found(mock_mcp, mock_context): result_data = json.loads(result) assert result_data["success"] is False - assert "not found" in result_data["error"] \ No newline at end of file + # Error must be structured format (dict), not string + assert "error" in result_data + assert isinstance(result_data["error"], dict), "Error should be structured format, not string" + assert result_data["error"]["type"] == "not_found" + assert "not found" in result_data["error"]["message"].lower() \ No newline at end of file diff --git a/python/tests/mcp_server/features/tasks/test_task_tools.py b/python/tests/mcp_server/features/tasks/test_task_tools.py index 4c0c9c31..10b815a4 100644 --- a/python/tests/mcp_server/features/tasks/test_task_tools.py +++ b/python/tests/mcp_server/features/tasks/test_task_tools.py @@ -210,4 +210,8 @@ async def test_delete_task_already_archived(mock_mcp, mock_context): result_data = json.loads(result) assert result_data["success"] is False - assert "already archived" in result_data["error"] \ No newline at end of file + # Error must be structured format (dict), not string + assert "error" in result_data + assert isinstance(result_data["error"], dict), "Error should be structured format, not string" + assert result_data["error"]["type"] == "already_archived" + assert "already archived" in result_data["error"]["message"].lower() \ No newline at end of file diff --git a/python/tests/mcp_server/features/test_feature_tools.py b/python/tests/mcp_server/features/test_feature_tools.py index f43fbba4..08a51e9b 100644 --- a/python/tests/mcp_server/features/test_feature_tools.py +++ b/python/tests/mcp_server/features/test_feature_tools.py @@ -120,4 +120,8 @@ async def test_get_project_features_not_found(mock_mcp, mock_context): result_data = json.loads(result) assert result_data["success"] is False - assert "not found" in result_data["error"] \ No newline at end of file + # Error must be structured format (dict), not string + assert "error" in result_data + assert isinstance(result_data["error"], dict), "Error should be structured format, not string" + assert result_data["error"]["type"] == "not_found" + assert "not found" in result_data["error"]["message"].lower() \ No newline at end of file