From 13a31d922555315111d8ada61ec9fea5a4dd0d19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20De=20la=20Torre=20Vico?= Date: Mon, 31 Aug 2026 17:36:19 +0200 Subject: [PATCH] feat(mcp): raise instead of returning error objects in the Prowler Docs tools (#12534) Co-authored-by: Claude Opus 5 (1M context) --- .../mcp-docs-search-swallowed-errors.fixed.md | 1 + .../prowler_documentation/search_engine.py | 127 +++++++++--------- .../prowler_documentation/server.py | 13 +- .../prowler_documentation/test_server.py | 69 +++++++++- 4 files changed, 142 insertions(+), 68 deletions(-) create mode 100644 mcp_server/changelog.d/mcp-docs-search-swallowed-errors.fixed.md diff --git a/mcp_server/changelog.d/mcp-docs-search-swallowed-errors.fixed.md b/mcp_server/changelog.d/mcp-docs-search-swallowed-errors.fixed.md new file mode 100644 index 0000000000..d24445ea71 --- /dev/null +++ b/mcp_server/changelog.d/mcp-docs-search-swallowed-errors.fixed.md @@ -0,0 +1 @@ +`prowler_docs_search` no longer reports a failed search as zero matches or an unreadable answer as a bad search term, and `prowler_docs_get_document` no longer reports a failed fetch as a missing page diff --git a/mcp_server/prowler_mcp_server/prowler_documentation/search_engine.py b/mcp_server/prowler_mcp_server/prowler_documentation/search_engine.py index 1eb1d10820..990a225239 100644 --- a/mcp_server/prowler_mcp_server/prowler_documentation/search_engine.py +++ b/mcp_server/prowler_mcp_server/prowler_documentation/search_engine.py @@ -2,6 +2,7 @@ import httpx from pydantic import BaseModel, Field from prowler_mcp_server import __version__ +from prowler_mcp_server.lib.errors import parse_json_response class SearchResult(BaseModel): @@ -58,8 +59,7 @@ class ProwlerDocsSearchEngine: ) def search(self, query: str, page_size: int = 5) -> list[SearchResult]: - """ - Search documentation using Mintlify API. + """Search documentation using Mintlify API. Args: query: Search query string @@ -69,82 +69,85 @@ class ProwlerDocsSearchEngine: Returns: list of search results + + Raises: + httpx.HTTPError: If the search request failed, which is not the same + answer as no matches + UpstreamInvalidResponse: If the answer is not JSON, which is the + documentation site's fault and not the search term's """ - try: - # Make request to Mintlify API - response = self.mintlify_client.post( - self.api_base_url, - json={"query": query, "filters": {}}, - ) - response.raise_for_status() - data = response.json() + # Make request to Mintlify API + response = self.mintlify_client.post( + self.api_base_url, + json={"query": query, "filters": {}}, + ) + response.raise_for_status() + # Not `response.json()`: the decode error it raises is a ValueError, which + # the shared classifier reads as a malformed argument and answers by + # telling the caller to fix a search term that was never the problem. + data = parse_json_response(response) - # Parse results - results = [] - for match in data.get("results", [])[:page_size]: - metadata = match.get("metadata", {}) - breadcrumbs = metadata.get("breadcrumbs", []) - doc_path = match.get("page", "") + # Parse results + results = [] + for match in data.get("results", [])[:page_size]: + metadata = match.get("metadata", {}) + breadcrumbs = metadata.get("breadcrumbs", []) + doc_path = match.get("page", "") - # A match is one section of a page rather than the page: the - # heading it was found under is its header, and the page's own - # title is the last step of its breadcrumb trail. - section = match.get("header", "") - title = breadcrumbs[-1] if breadcrumbs else section + # A match is one section of a page rather than the page: the + # heading it was found under is its header, and the page's own + # title is the last step of its breadcrumb trail. + section = match.get("header", "") + title = breadcrumbs[-1] if breadcrumbs else section - # Sent as "" for the section a page opens with and as null for - # the pages that have no anchors at all; both mean the page. - anchor = metadata.get("hash") - url = f"{self.docs_base_url}/{doc_path}" - if anchor: - url = f"{url}#{anchor}" + # Sent as "" for the section a page opens with and as null for + # the pages that have no anchors at all; both mean the page. + anchor = metadata.get("hash") + url = f"{self.docs_base_url}/{doc_path}" + if anchor: + url = f"{url}#{anchor}" - results.append( - SearchResult( - path=doc_path, - title=title, - section=section, - breadcrumbs=breadcrumbs, - url=url, - excerpt=match.get("content", ""), - score=match.get("score", 0.0), - ) + results.append( + SearchResult( + path=doc_path, + title=title, + section=section, + breadcrumbs=breadcrumbs, + url=url, + excerpt=match.get("content", ""), + score=match.get("score", 0.0), ) + ) - return results - - except Exception as e: - # Return empty list on error - print(f"Search error: {e}") - return [] + return results def get_document(self, doc_path: str) -> str | None: - """ - Get full document content from Mintlify documentation. + """Get full document content from Mintlify documentation. Args: doc_path: Path to the documentation file (e.g., "getting-started/installation") Returns: - Full markdown content of the documentation, or None if not found + Full markdown content of the documentation, or None if there is no + page at that path + + Raises: + httpx.HTTPError: If the fetch failed for any reason other than a 404 """ - try: - # Clean up the path - doc_path = doc_path.rstrip("/") + # Clean up the path + doc_path = doc_path.rstrip("/") - # Add .md extension if not present (Mintlify serves both .md and .mdx) - if not doc_path.endswith(".md"): - doc_path = f"{doc_path}.md" + # Add .md extension if not present (Mintlify serves both .md and .mdx) + if not doc_path.endswith(".md"): + doc_path = f"{doc_path}.md" - # Construct Mintlify URL - url = f"{self.docs_base_url}/{doc_path}" + # Construct Mintlify URL + url = f"{self.docs_base_url}/{doc_path}" - # Fetch the documentation page - response = self.docs_client.get(url) - response.raise_for_status() - - return response.text - - except Exception as e: - print(f"Error fetching document: {e}") + # Fetch the documentation page + response = self.docs_client.get(url) + if response.status_code == 404: return None + response.raise_for_status() + + return response.text diff --git a/mcp_server/prowler_mcp_server/prowler_documentation/server.py b/mcp_server/prowler_mcp_server/prowler_documentation/server.py index de0756a505..1121d7c419 100644 --- a/mcp_server/prowler_mcp_server/prowler_documentation/server.py +++ b/mcp_server/prowler_mcp_server/prowler_documentation/server.py @@ -1,6 +1,7 @@ from typing import Any from fastmcp import FastMCP +from fastmcp.exceptions import ToolError from pydantic import Field from prowler_mcp_server.lib.types import NonBlankStr @@ -9,7 +10,7 @@ from prowler_mcp_server.prowler_documentation.search_engine import ( ) # Initialize FastMCP server -docs_mcp_server = FastMCP("prowler-docs") +docs_mcp_server = FastMCP("prowler-docs", mask_error_details=True) prowler_docs_search_engine = ProwlerDocsSearchEngine() @@ -56,6 +57,10 @@ def get_document( """ content: str | None = prowler_docs_search_engine.get_document(doc_path) if content is None: - return {"error": f"Document '{doc_path}' not found."} - else: - return {"content": content} + # No `from`: this names the path asked for and the tool that produces a + # valid one, neither of which the shared classifier can know. + raise ToolError( + f"The Prowler documentation has no page at '{doc_path}'. Use " + "prowler_docs_search and pass the 'path' field of a result verbatim." + ) + return {"content": content} diff --git a/mcp_server/tests/prowler_documentation/test_server.py b/mcp_server/tests/prowler_documentation/test_server.py index da5e9f17f4..67ee57decf 100644 --- a/mcp_server/tests/prowler_documentation/test_server.py +++ b/mcp_server/tests/prowler_documentation/test_server.py @@ -1,7 +1,9 @@ -"""Tests for the Prowler documentation search tool. +"""Tests for the Prowler documentation tools. Mintlify moved the docs search to a new endpoint that answers with page -sections, so a result is a part of a page and has to read as one. +sections, so a result is a part of a page and has to read as one. And a failed +request must not reach an agent as "the documentation has nothing on this", +which is an answer it would act on, confidently and wrongly. """ import json @@ -9,6 +11,7 @@ import json from fastmcp import Client SEARCH = "/api/search/prowler" +DOC = "/getting-started/installation.md" def search_match( @@ -107,3 +110,65 @@ async def test_page_size_caps_a_response_the_api_did_not_size( ) assert len(result.data) == 2 + + +async def test_a_search_that_failed_is_not_reported_as_no_matches( + mcp_root_server, docs_router +): + """An empty list is an answer. A failed request is not, and must not look like one.""" + docs_router.add("POST", SEARCH, status=500, text="upstream error") + + async with Client(mcp_root_server) as client: + result = await client.call_tool_mcp("prowler_docs_search", {"term": "install"}) + + assert result.isError is True + assert result.structuredContent is None + + +async def test_a_missing_page_fails_and_names_the_tool_that_finds_a_valid_path( + mcp_root_server, docs_router +): + """A 404 answers the question, and still reaches the agent as an error.""" + docs_router.add("GET", DOC, status=404, text="Not Found") + + async with Client(mcp_root_server) as client: + result = await client.call_tool_mcp( + "prowler_docs_get_document", {"doc_path": "getting-started/installation"} + ) + + assert result.isError is True + assert "prowler_docs_search" in result.content[0].text + + +async def test_a_fetch_that_failed_is_not_reported_as_a_missing_page( + mcp_root_server, docs_router +): + """Only a 404 answers the question; every other status left it unanswered.""" + docs_router.add("GET", DOC, status=503, text="upstream error") + + async with Client(mcp_root_server) as client: + result = await client.call_tool_mcp( + "prowler_docs_get_document", {"doc_path": "getting-started/installation"} + ) + + assert result.isError is True + assert "no page at" not in result.content[0].text + + +async def test_an_unreadable_body_is_not_reported_as_a_bad_search_term( + mcp_root_server, docs_router +): + """An edge serving HTML is the site's fault; the caller has no term to fix.""" + docs_router.add("POST", SEARCH, status=200, text="edge error page") + + async with Client(mcp_root_server) as client: + result = await client.call_tool_mcp("prowler_docs_search", {"term": "install"}) + + assert result.isError is True + message = result.content[0].text + # Named as the upstream at fault, and explicitly not the caller's arguments, + # which is the story the shared ValueError branch would otherwise tell. + assert "leaves.mintlify.com" in message + assert "changing them will not help" in message + # The body it choked on is upstream text, which this server never relays. + assert "edge error page" not in message