diff --git a/src/mcp_server_qdrant/mcp_server.py b/src/mcp_server_qdrant/mcp_server.py index 0617b9d8..416d3988 100644 --- a/src/mcp_server_qdrant/mcp_server.py +++ b/src/mcp_server_qdrant/mcp_server.py @@ -3,6 +3,7 @@ from typing import Annotated, Any, Optional from fastmcp import Context, FastMCP +from fastmcp.exceptions import ToolError from pydantic import Field from qdrant_client import models @@ -21,6 +22,22 @@ logger = logging.getLogger(__name__) +def describe_exception(exc: Exception) -> str: + """ + Build a human readable description of an exception. + + Some exceptions, such as a bare ``AssertionError`` or errors raised without a + message, have an empty string representation. When those propagate to the MCP + client they show up as an empty error (see issue #151). Falling back to the + exception class name guarantees the client always receives a non-empty, + actionable message. + """ + message = str(exc).strip() + if message: + return f"{type(exc).__name__}: {message}" + return type(exc).__name__ + + # FastMCP is an alternative interface for declaring the capabilities # of the server. Its API is based on FastAPI. class QdrantMCPServer(FastMCP): @@ -119,7 +136,15 @@ async def store( entry = Entry(content=information, metadata=metadata) - await self.qdrant_connector.store(entry, collection_name=collection_name) + try: + await self.qdrant_connector.store( + entry, collection_name=collection_name + ) + except Exception as exc: + await ctx.debug(f"qdrant-store failed: {exc!r}") + raise ToolError( + f"qdrant-store failed: {describe_exception(exc)}" + ) from exc if collection_name: return f"Remembered: {information} in collection {collection_name}" return f"Remembered: {information}" @@ -149,12 +174,18 @@ async def find( await ctx.debug(f"Finding results for query {query}") - entries = await self.qdrant_connector.search( - query, - collection_name=collection_name, - limit=self.qdrant_settings.search_limit, - query_filter=query_filter, - ) + try: + entries = await self.qdrant_connector.search( + query, + collection_name=collection_name, + limit=self.qdrant_settings.search_limit, + query_filter=query_filter, + ) + except Exception as exc: + await ctx.debug(f"qdrant-find failed: {exc!r}") + raise ToolError( + f"qdrant-find failed: {describe_exception(exc)}" + ) from exc if not entries: return None content = [ diff --git a/tests/test_error_surfacing.py b/tests/test_error_surfacing.py new file mode 100644 index 00000000..0420c251 --- /dev/null +++ b/tests/test_error_surfacing.py @@ -0,0 +1,93 @@ +import pytest +from fastmcp import Client +from fastmcp.exceptions import ToolError + +from mcp_server_qdrant.mcp_server import QdrantMCPServer, describe_exception +from mcp_server_qdrant.settings import ( + EmbeddingProviderSettings, + QdrantSettings, + ToolSettings, +) + + +@pytest.fixture +def mcp_server(): + """Fixture providing a QdrantMCPServer backed by an in-memory Qdrant.""" + return QdrantMCPServer( + tool_settings=ToolSettings(), + qdrant_settings=QdrantSettings(), + embedding_provider_settings=EmbeddingProviderSettings(), + ) + + +class TestDescribeException: + def test_uses_message_when_present(self): + assert describe_exception(ValueError("boom")) == "ValueError: boom" + + def test_falls_back_to_class_name_for_empty_message(self): + # A bare AssertionError has an empty string representation. + assert describe_exception(AssertionError()) == "AssertionError" + + def test_strips_whitespace_only_messages(self): + assert describe_exception(RuntimeError(" ")) == "RuntimeError" + + +class TestToolErrorSurfacing: + """Regression tests for issue #151: tools returned empty errors.""" + + @pytest.mark.asyncio + async def test_find_surfaces_non_empty_error(self, mcp_server, monkeypatch): + # Simulate a backend failure whose string representation is empty. + async def boom(*args, **kwargs): + raise AssertionError() + + monkeypatch.setattr(mcp_server.qdrant_connector, "search", boom) + + async with Client(mcp_server) as client: + with pytest.raises(ToolError) as exc_info: + await client.call_tool( + "qdrant-find", + {"query": "anything", "collection_name": "some-collection"}, + ) + + message = str(exc_info.value) + # Before the fix this was "Error calling tool 'qdrant-find': " with an + # empty tail. The real error must now propagate to the client. + assert message.strip() != "" + assert "qdrant-find failed" in message + assert "AssertionError" in message + + @pytest.mark.asyncio + async def test_store_surfaces_non_empty_error(self, mcp_server, monkeypatch): + async def boom(*args, **kwargs): + raise AssertionError() + + monkeypatch.setattr(mcp_server.qdrant_connector, "store", boom) + + async with Client(mcp_server) as client: + with pytest.raises(ToolError) as exc_info: + await client.call_tool( + "qdrant-store", + {"information": "hello", "collection_name": "some-collection"}, + ) + + message = str(exc_info.value) + assert message.strip() != "" + assert "qdrant-store failed" in message + assert "AssertionError" in message + + @pytest.mark.asyncio + async def test_find_propagates_real_message(self, mcp_server, monkeypatch): + async def boom(*args, **kwargs): + raise RuntimeError("connection refused") + + monkeypatch.setattr(mcp_server.qdrant_connector, "search", boom) + + async with Client(mcp_server) as client: + with pytest.raises(ToolError) as exc_info: + await client.call_tool( + "qdrant-find", + {"query": "anything", "collection_name": "some-collection"}, + ) + + assert "connection refused" in str(exc_info.value)