feat(server): Add structured output to all tool/resource output
BREAKING CHANGE
This commit is contained in:
Chris Coutinho
@@ -0,0 +1,186 @@
|
||||
"""Test error propagation in the MCP server for various error scenarios."""
|
||||
|
||||
import logging
|
||||
from mcp import ClientSession
|
||||
from mcp.shared.exceptions import McpError
|
||||
|
||||
import pytest
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
@pytest.mark.integration
|
||||
async def test_missing_note_resource_error(nc_mcp_client: ClientSession):
|
||||
"""Test that accessing a non-existent note resource returns proper error."""
|
||||
# Try to get a non-existent note via resource - should raise McpError with improved message
|
||||
with pytest.raises(McpError, match=r"Note 999999 not found"):
|
||||
await nc_mcp_client.read_resource("nc://Notes/999999")
|
||||
|
||||
|
||||
@pytest.mark.integration
|
||||
async def test_delete_missing_note_tool_error(nc_mcp_client: ClientSession):
|
||||
"""Test that deleting a non-existent note returns proper error."""
|
||||
# Try to delete a non-existent note
|
||||
response = await nc_mcp_client.call_tool(
|
||||
"nc_notes_delete_note", {"note_id": 999999}
|
||||
)
|
||||
|
||||
logger.info(f"Delete missing note response: {response}")
|
||||
|
||||
# Should return structured error response with improved message
|
||||
assert response is not None
|
||||
assert (
|
||||
response.isError is False
|
||||
) # Tools now return structured responses, not MCP errors
|
||||
|
||||
# Check structured content for error
|
||||
assert "success" in response.structuredContent["result"]
|
||||
assert response.structuredContent["result"]["success"] is False
|
||||
assert "Note 999999 not found" in response.structuredContent["result"]["error"]
|
||||
|
||||
|
||||
@pytest.mark.integration
|
||||
async def test_search_with_empty_query(nc_mcp_client: ClientSession):
|
||||
"""Test search behavior with empty query."""
|
||||
# Search with empty query
|
||||
response = await nc_mcp_client.call_tool("nc_notes_search_notes", {"query": ""})
|
||||
|
||||
logger.info(f"Empty search query response: {response}")
|
||||
|
||||
# Should return successful response with empty or valid results
|
||||
assert response is not None
|
||||
assert response.isError is False
|
||||
|
||||
|
||||
@pytest.mark.integration
|
||||
async def test_tool_missing_required_parameters(nc_mcp_client: ClientSession):
|
||||
"""Test calling a tool with missing required parameters."""
|
||||
# Try to create note with missing parameters
|
||||
response = await nc_mcp_client.call_tool(
|
||||
"nc_notes_create_note",
|
||||
{"title": "Test"}, # Missing content and category
|
||||
)
|
||||
logger.info(f"Missing params response: {response}")
|
||||
|
||||
# Should return error response for missing required parameters
|
||||
assert response is not None
|
||||
assert response.isError is True
|
||||
assert (
|
||||
"required" in response.content[0].text.lower()
|
||||
or "missing" in response.content[0].text.lower()
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.integration
|
||||
async def test_update_note_with_invalid_etag(nc_mcp_client: ClientSession, nc_client):
|
||||
"""Test updating a note with invalid ETag."""
|
||||
# First create a note
|
||||
note_data = await nc_client.notes.create_note(
|
||||
title="Test Note for ETag", content="Test content", category=""
|
||||
)
|
||||
note_id = note_data["id"]
|
||||
|
||||
try:
|
||||
# Try to update with invalid ETag
|
||||
response = await nc_mcp_client.call_tool(
|
||||
"nc_notes_update_note",
|
||||
{
|
||||
"note_id": note_id,
|
||||
"etag": "invalid-etag",
|
||||
"title": "Updated Title",
|
||||
"content": None,
|
||||
"category": None,
|
||||
},
|
||||
)
|
||||
|
||||
logger.info(f"Invalid ETag response: {response}")
|
||||
|
||||
# Should return structured error response with improved message
|
||||
assert response is not None
|
||||
assert response.isError is False # Tools now return structured responses
|
||||
assert "success" in response.structuredContent["result"]
|
||||
assert response.structuredContent["result"]["success"] is False
|
||||
assert (
|
||||
"modified by someone else" in response.structuredContent["result"]["error"]
|
||||
)
|
||||
|
||||
finally:
|
||||
# Clean up
|
||||
await nc_client.notes.delete_note(note_id)
|
||||
|
||||
|
||||
@pytest.mark.integration
|
||||
async def test_calendar_missing_calendar_error(nc_mcp_client: ClientSession):
|
||||
"""Test calendar operations with non-existent calendar."""
|
||||
# Try to create event in non-existent calendar
|
||||
response = await nc_mcp_client.call_tool(
|
||||
"nc_calendar_create_event",
|
||||
{
|
||||
"calendar_name": "non-existent-calendar",
|
||||
"title": "Test Event",
|
||||
"start_datetime": "2025-01-15T14:00:00",
|
||||
},
|
||||
)
|
||||
|
||||
logger.info(f"Non-existent calendar response: {response}")
|
||||
|
||||
# Should return structured error response
|
||||
assert response is not None
|
||||
# Note: Some modules may not have improved error handling yet
|
||||
# Check if we have structured content with success=false or isError=true
|
||||
if (
|
||||
hasattr(response, "structuredContent")
|
||||
and response.structuredContent
|
||||
and "result" in response.structuredContent
|
||||
):
|
||||
assert response.structuredContent["result"]["success"] is False
|
||||
else:
|
||||
assert response.isError is True
|
||||
|
||||
|
||||
@pytest.mark.integration
|
||||
async def test_webdav_read_missing_file_error(nc_mcp_client: ClientSession):
|
||||
"""Test WebDAV operations with non-existent file."""
|
||||
# Try to read a non-existent file
|
||||
response = await nc_mcp_client.call_tool(
|
||||
"nc_webdav_read_file", {"path": "non-existent-file.txt"}
|
||||
)
|
||||
|
||||
logger.info(f"Missing file response: {response}")
|
||||
|
||||
# Should return structured error response
|
||||
assert response is not None
|
||||
# Note: Some modules may not have improved error handling yet
|
||||
# Check if we have structured content with success=false or isError=true
|
||||
if (
|
||||
hasattr(response, "structuredContent")
|
||||
and response.structuredContent
|
||||
and "result" in response.structuredContent
|
||||
):
|
||||
assert response.structuredContent["result"]["success"] is False
|
||||
else:
|
||||
assert response.isError is True
|
||||
|
||||
|
||||
@pytest.mark.integration
|
||||
async def test_tables_missing_table_error(nc_mcp_client: ClientSession):
|
||||
"""Test Tables operations with non-existent table."""
|
||||
# Try to get schema of non-existent table
|
||||
response = await nc_mcp_client.call_tool(
|
||||
"nc_tables_get_schema", {"table_id": 999999}
|
||||
)
|
||||
|
||||
logger.info(f"Missing table response: {response}")
|
||||
|
||||
# Should return structured error response
|
||||
assert response is not None
|
||||
# Note: Some modules may not have improved error handling yet
|
||||
# Check if we have structured content with success=false or isError=true
|
||||
if (
|
||||
hasattr(response, "structuredContent")
|
||||
and response.structuredContent
|
||||
and "result" in response.structuredContent
|
||||
):
|
||||
assert response.structuredContent["result"]["success"] is False
|
||||
else:
|
||||
assert response.isError is True
|
||||
@@ -24,7 +24,6 @@ async def test_mcp_connectivity(nc_mcp_client: ClientSession):
|
||||
|
||||
# Verify expected tools are present
|
||||
expected_tools = [
|
||||
"nc_get_note",
|
||||
"nc_notes_create_note",
|
||||
"nc_notes_update_note",
|
||||
"nc_notes_append_content",
|
||||
@@ -137,11 +136,9 @@ async def test_mcp_notes_crud_workflow(
|
||||
|
||||
# 3. Read note via MCP
|
||||
logger.info(f"Reading note via MCP: {note_id}")
|
||||
read_result = await nc_mcp_client.call_tool("nc_get_note", {"note_id": note_id})
|
||||
assert read_result.isError is False, (
|
||||
f"MCP note read failed: {read_result.content}"
|
||||
)
|
||||
read_note_data = json.loads(read_result.content[0].text)
|
||||
read_result = await nc_mcp_client.read_resource(f"nc://Notes/{note_id}")
|
||||
assert len(read_result.contents) == 1, "Expected exactly one content item"
|
||||
read_note_data = json.loads(read_result.contents[0].text)
|
||||
|
||||
assert read_note_data["title"] == test_title
|
||||
assert read_note_data["content"] == test_content
|
||||
@@ -199,14 +196,23 @@ async def test_mcp_notes_crud_workflow(
|
||||
)
|
||||
search_notes_text = search_result.content[0].text
|
||||
logger.info(f"Search result text: {search_notes_text}")
|
||||
search_notes = json.loads(search_notes_text)
|
||||
search_response = json.loads(search_notes_text)
|
||||
|
||||
# Ensure search_notes is a list
|
||||
if not isinstance(search_notes, list):
|
||||
logger.warning(
|
||||
f"Expected search results to be a list, got: {type(search_notes)}"
|
||||
)
|
||||
search_notes = [search_notes] if search_notes else []
|
||||
# Expect structured response with Pydantic format
|
||||
assert isinstance(search_response, dict), (
|
||||
f"Expected search response to be a dict with structured format, got: {type(search_response)}"
|
||||
)
|
||||
assert "results" in search_response, (
|
||||
f"Expected 'results' field in search response, got keys: {list(search_response.keys())}"
|
||||
)
|
||||
assert "success" in search_response and search_response["success"], (
|
||||
f"Expected successful search response, got: {search_response}"
|
||||
)
|
||||
|
||||
search_notes = search_response["results"]
|
||||
assert isinstance(search_notes, list), (
|
||||
f"Expected results to be a list, got: {type(search_notes)}"
|
||||
)
|
||||
|
||||
# Find our note in search results
|
||||
found_note = None
|
||||
@@ -216,7 +222,7 @@ async def test_mcp_notes_crud_workflow(
|
||||
break
|
||||
|
||||
assert found_note is not None, (
|
||||
f"Created note not found in search results. Search returned: {search_notes}"
|
||||
f"Created note not found in search results. Search returned: {search_response}"
|
||||
)
|
||||
assert found_note["title"] == updated_title
|
||||
|
||||
@@ -431,21 +437,33 @@ async def test_mcp_calendar_workflow(
|
||||
f"MCP calendar listing failed: {calendars_result.content}"
|
||||
)
|
||||
|
||||
calendars_data = json.loads(calendars_result.content[0].text)
|
||||
calendars_response = json.loads(calendars_result.content[0].text)
|
||||
|
||||
# Debug output to understand the structure
|
||||
logger.info(f"calendars_data type: {type(calendars_data)}")
|
||||
logger.info(f"calendars_data content: {calendars_data}")
|
||||
logger.info(f"calendars_response type: {type(calendars_response)}")
|
||||
logger.info(f"calendars_response content: {calendars_response}")
|
||||
|
||||
# Handle the case where MCP tool returns a single dict instead of a list
|
||||
if isinstance(calendars_data, dict):
|
||||
# Single calendar returned as dict instead of list
|
||||
calendar_name = calendars_data["name"]
|
||||
elif isinstance(calendars_data, list) and calendars_data:
|
||||
# Normal case - list of calendars
|
||||
calendar_name = calendars_data[0]["name"]
|
||||
else:
|
||||
# Expect structured response with Pydantic format
|
||||
assert isinstance(calendars_response, dict), (
|
||||
f"Expected calendar response to be a dict with structured format, got: {type(calendars_response)}"
|
||||
)
|
||||
assert "calendars" in calendars_response, (
|
||||
f"Expected 'calendars' field in response, got keys: {list(calendars_response.keys())}"
|
||||
)
|
||||
assert "success" in calendars_response and calendars_response["success"], (
|
||||
f"Expected successful calendar response, got: {calendars_response}"
|
||||
)
|
||||
|
||||
calendars_list = calendars_response["calendars"]
|
||||
assert isinstance(calendars_list, list), (
|
||||
f"Expected calendars to be a list, got: {type(calendars_list)}"
|
||||
)
|
||||
|
||||
if not calendars_list:
|
||||
pytest.skip("No calendars available for testing")
|
||||
|
||||
# Use the first available calendar
|
||||
calendar_name = calendars_list[0]["name"]
|
||||
logger.info(f"Using calendar: {calendar_name}")
|
||||
|
||||
# 2. Create event via MCP
|
||||
|
||||
Reference in New Issue
Block a user