fix(mcp): report connection errors and timeouts to MCP clients

application/mcp/rest_client.py

@@ -24,7 +24,7 @@
     Set,
     Tuple,
 )
-from urllib.parse import quote, urljoin
+from urllib.parse import quote, urljoin, urlsplit
 
 import requests
 from jsonschema import Draft7Validator
@@ -189,7 +189,11 @@ def call_spec(self, tool: ToolSpec, arguments: Mapping[str, Any]) -> RestResult:
 
         # Credentials come from the environment only, never from arguments.
         headers, token = self._auth_headers(tool)
-        response = self._send(method, url, query_params, body, headers)
+        try:
+            response = self._send(method, url, query_params, body, headers)
+        except requests.RequestException as exc:
+            message = _send_failure_message(exc, self._base_url, self._timeout)
+            raise RestClientError(_scrub(message, token)) from exc
         # Never retain cookies across tool calls (no credential forwarding).
         if hasattr(self._session, "cookies"):
             try:
@@ -258,6 +262,23 @@ def _scrub(text: str, token: Optional[str]) -> str:
     return text.replace(token, "<redacted>") if token else text
 
 
+def _send_failure_message(
+    exc: requests.RequestException, base_url: str, timeout: float
+) -> str:
+    """Say why the request failed without leaking request details or URL secrets."""
+    parts = urlsplit(base_url)
+    # Secrets can sit in user:password@, the query string or the fragment.
+    netloc = parts.netloc.rpartition("@")[2]
+    where = parts._replace(netloc=netloc, query="", fragment="").geturl()
+    reason = type(exc).__name__
+    # Timeout first: ConnectTimeout is also a ConnectionError.
+    if isinstance(exc, requests.Timeout):
+        return f"OpenCRE at {where} timed out after {timeout:g}s"
+    if isinstance(exc, requests.ConnectionError):
+        return f"Could not reach OpenCRE at {where} ({reason})"
+    return f"Request to OpenCRE at {where} failed ({reason})"
+
+
 def flask_test_session(flask_client: Any) -> "_FlaskTestSession":
     """Adapt Flask's test_client to the requests Session interface."""
     return _FlaskTestSession(flask_client)

application/tests/mcp_public_tools_test.py

@@ -8,11 +8,13 @@
 
 import json
 import os
+import socket
 import unittest
 from typing import Any, Dict
 from unittest.mock import patch
 
 import networkx as nx
+import requests
 
 from application import create_app, sqla  # type: ignore
 from application.database import db
@@ -30,6 +32,7 @@
 )
 from application.mcp.rest_client import (
     RestClient,
+    RestClientError,
     RestRequestError,
     RestResponseError,
     _parse_response,
@@ -53,6 +56,16 @@
 ]
 
 
+class _FailingSession:
+    """Stands in for requests.Session when the request gets no response."""
+
+    def __init__(self, error: Exception) -> None:
+        self.error = error
+
+    def request(self, *args: Any, **kwargs: Any) -> Any:
+        raise self.error
+
+
 class McpPublicToolsTest(unittest.TestCase):
     def setUp(self) -> None:
         self.app = create_app(mode="test")
@@ -340,6 +353,32 @@ def test_rest_404_is_tool_failure_not_empty_success(self) -> None:
             client.call_tool("get_cre_by_id", {"creid": "999-999"})
         self.assertEqual(ctx.exception.status_code, 404)
 
+    def test_send_failures_say_what_went_wrong(self) -> None:
+        """Each failure gets its own message, and secrets in the URL never show up."""
+        errors = requests.exceptions
+        site = "OpenCRE at http://127.0.0.1:5999"
+        unreachable = f"Could not reach {site}"
+        failed = f"Request to {site} failed"
+        cases = [
+            (errors.ConnectionError(), f"{unreachable} (ConnectionError)"),
+            (errors.SSLError(), f"{unreachable} (SSLError)"),
+            (errors.ReadTimeout(), f"{site} timed out after 2.5s"),
+            # ConnectTimeout is also a ConnectionError, but it reads as a timeout.
+            (errors.ConnectTimeout(), f"{site} timed out after 2.5s"),
+            (errors.ChunkedEncodingError(), f"{failed} (ChunkedEncodingError)"),
+            (errors.InvalidJSONError(), f"{failed} (InvalidJSONError)"),
+        ]
+        for error, expected in cases:
+            with self.subTest(error=type(error).__name__):
+                client = RestClient(
+                    base_url="http://alice:s3cr3t@127.0.0.1:5999?key=s3cr3t#s3cr3t",
+                    session=_FailingSession(error),
+                    timeout=2.5,
+                )
+                with self.assertRaises(RestClientError) as ctx:
+                    client.call_tool("list_standards", {})
+                self.assertEqual(str(ctx.exception), expected)
+
     def test_missing_required_input_fails(self) -> None:
         client = self._rest()
         with self.assertRaises(RestRequestError):
@@ -469,6 +508,37 @@ async def _run() -> None:
         with self.assertLogs("application.mcp.server", level="ERROR"):
             asyncio.run(_run())
 
+    def test_network_errors_are_tool_errors_not_internal_errors(self) -> None:
+        """MCP clients see why the call failed; the server log keeps the cause."""
+        # Pick a free loopback port and close it, so the real session is refused.
+        with socket.socket() as probe:
+            probe.bind(("127.0.0.1", 0))
+            base_url = f"http://127.0.0.1:{probe.getsockname()[1]}"
+        server = build_server(rest_client=RestClient(base_url=base_url))
+
+        async def _run() -> None:
+            call_handler = server.get_request_handler("tools/call")
+            assert call_handler is not None
+            result_raw = await call_handler.handler(
+                None,
+                types.CallToolRequestParams(name="list_standards", arguments={}),
+            )
+            assert isinstance(result_raw, types.CallToolResult)
+            self.assertTrue(result_raw.is_error)
+            first = result_raw.content[0]
+            assert isinstance(first, types.TextContent)
+            self.assertEqual(
+                first.text, f"Could not reach OpenCRE at {base_url} (ConnectionError)"
+            )
+
+        import asyncio
+
+        with self.assertLogs("application.mcp.server", level="ERROR") as logs:
+            asyncio.run(_run())
+        logged = logs.records[0].exc_info
+        assert logged is not None and logged[1] is not None
+        self.assertIsInstance(logged[1].__cause__, requests.ConnectionError)
+
     def test_base_url_comes_from_env_not_tool_args(self) -> None:
         with patch.dict(os.environ, {"OPENCRE_BASE_URL": "http://example.test:9"}):
             client = RestClient()