From 979348cd8d05227a7c0b4ea4d3d7be47e0cd4fad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20Wr=C3=B3bel?= Date: Wed, 1 Jul 2026 14:53:51 +0200 Subject: [PATCH 1/3] Dont ignore MCP exceptions from backend --- mcp_server/services/specflow_backend.py | 46 +++++++++----- .../tests/test_backend_contract_rejection.py | 62 +++++++++++++++++++ 2 files changed, 91 insertions(+), 17 deletions(-) diff --git a/mcp_server/services/specflow_backend.py b/mcp_server/services/specflow_backend.py index 8d5fd15..21dc7e6 100644 --- a/mcp_server/services/specflow_backend.py +++ b/mcp_server/services/specflow_backend.py @@ -71,24 +71,36 @@ async def call_backend( timeout_seconds: float = 600.0, connect_timeout_seconds: float = 10.0, ) -> str: - """Call backend; return response text. On HTTPError, returns an error string.""" - try: - url = f"{self.base_url}{endpoint}" - headers = self._get_headers() - async with self._client(timeout_seconds, connect_timeout_seconds) as client: - m = method.upper() - if m == "POST": - response = await client.post(url, json=json_data, headers=headers) - elif m == "GET": - response = await client.get(url, headers=headers) - elif m == "DELETE": - response = await client.delete(url, headers=headers) - else: - raise ValueError(f"Unsupported HTTP method: {method}") + """Call backend; return response text. Raises on HTTP errors (mirrors upload_file / + post_form_data) instead of returning a non-JSON error string — callers that blindly + json.loads() the return value would otherwise fail with a misleading + "Expecting value" error that hides the actual backend detail. + """ + url = f"{self.base_url}{endpoint}" + headers = self._get_headers() + async with self._client(timeout_seconds, connect_timeout_seconds) as client: + m = method.upper() + if m == "POST": + response = await client.post(url, json=json_data, headers=headers) + elif m == "GET": + response = await client.get(url, headers=headers) + elif m == "DELETE": + response = await client.delete(url, headers=headers) + else: + raise ValueError(f"Unsupported HTTP method: {method}") + try: response.raise_for_status() - return response.text - except httpx.HTTPError as e: - return f"Error calling backend: {str(e)}" + except httpx.HTTPStatusError as e: + rejection = _parse_contract_rejection(e.response) + if rejection is not None: + raise BackendContractRejection(rejection) from e + detail = f"HTTP {e.response.status_code}: {e.response.text[:500]}" + logger.error("HTTP error calling backend %s: %s", endpoint, detail) + raise Exception(f"Backend returned {detail}") from e + except httpx.HTTPError as e: + logger.error("HTTP error calling backend %s: %s, type: %s", endpoint, e, type(e).__name__) + raise Exception(f"Failed to call backend: {e}") from e + return response.text async def call_backend_bytes( self, diff --git a/mcp_server/tests/test_backend_contract_rejection.py b/mcp_server/tests/test_backend_contract_rejection.py index 068afd8..74d758f 100644 --- a/mcp_server/tests/test_backend_contract_rejection.py +++ b/mcp_server/tests/test_backend_contract_rejection.py @@ -31,6 +31,12 @@ def __init__(self, resp): async def post(self, *args, **kwargs): return self._resp + async def get(self, *args, **kwargs): + return self._resp + + async def delete(self, *args, **kwargs): + return self._resp + def _patch_client(resp): @asynccontextmanager @@ -106,3 +112,59 @@ async def test_success_returns_body_text(self): form_data={}, ) assert "est-1" in text + + +class TestCallBackendErrorHandling: + """call_backend() must raise on HTTP errors, not return a non-JSON string. + + Regression coverage for the bug where a 4xx/5xx response was swallowed into + a plain "Error calling backend: ..." string; callers that json.loads() the + return value then failed with a misleading "Expecting value: line 1 column 1 + (char 0)" instead of ever seeing the actual backend error. + """ + + @pytest.mark.asyncio + async def test_success_returns_body_text(self): + resp = _response(200, {"status": "running"}) + with _patch_client(resp): + svc = SpecFlowBackendService() + text = await svc.call_backend(endpoint="/api/v1/generation-sessions/est-1/status", method="GET") + assert "running" in text + + @pytest.mark.asyncio + async def test_plain_error_detail_surfaces_in_exception_message(self): + resp = _response( + 500, + {"detail": "Cannot retry generation est-1: retry_count (3) has reached max_retries (3)."}, + ) + with _patch_client(resp): + svc = SpecFlowBackendService() + with pytest.raises(Exception) as exc_info: + await svc.call_backend(endpoint="/api/v1/generation-sessions/est-1/retry", method="POST") + assert not isinstance(exc_info.value, BackendContractRejection) + assert "max_retries" in str(exc_info.value) + + @pytest.mark.asyncio + async def test_structured_400_raises_typed_rejection(self): + resp = _response(400, {"detail": {"code": "PLAN_NO_PHASES", "error": "no phases"}}) + with _patch_client(resp): + svc = SpecFlowBackendService() + with pytest.raises(BackendContractRejection) as exc_info: + await svc.call_backend(endpoint="/api/v1/generation-sessions/sync", method="POST") + assert exc_info.value.detail["code"] == "PLAN_NO_PHASES" + + @pytest.mark.asyncio + async def test_connection_error_raises_not_returns_string(self): + @asynccontextmanager + async def _cm(*args, **kwargs): + class _RaisingClient: + async def get(self, *a, **k): + raise httpx.ConnectError("connection refused") + + yield _RaisingClient() + + with patch.object(SpecFlowBackendService, "_client", lambda self, *a, **k: _cm()): + svc = SpecFlowBackendService() + with pytest.raises(Exception) as exc_info: + await svc.call_backend(endpoint="/api/v1/generation-sessions/est-1/status", method="GET") + assert "connection refused" in str(exc_info.value) From 2db2a88973719d297cf1f25e472d48f178092a5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20Wr=C3=B3bel?= Date: Fri, 3 Jul 2026 15:08:00 +0200 Subject: [PATCH 2/3] Refactor HTTP method handling in SpecFlowBackendService to improve clarity and error handling --- mcp_server/services/specflow_backend.py | 44 ++++++++++++------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/mcp_server/services/specflow_backend.py b/mcp_server/services/specflow_backend.py index 21dc7e6..2338825 100644 --- a/mcp_server/services/specflow_backend.py +++ b/mcp_server/services/specflow_backend.py @@ -78,29 +78,29 @@ async def call_backend( """ url = f"{self.base_url}{endpoint}" headers = self._get_headers() - async with self._client(timeout_seconds, connect_timeout_seconds) as client: - m = method.upper() - if m == "POST": - response = await client.post(url, json=json_data, headers=headers) - elif m == "GET": - response = await client.get(url, headers=headers) - elif m == "DELETE": - response = await client.delete(url, headers=headers) - else: - raise ValueError(f"Unsupported HTTP method: {method}") - try: + m = method.upper() + if m not in ("POST", "GET", "DELETE"): + raise ValueError(f"Unsupported HTTP method: {method}") + try: + async with self._client(timeout_seconds, connect_timeout_seconds) as client: + if m == "POST": + response = await client.post(url, json=json_data, headers=headers) + elif m == "GET": + response = await client.get(url, headers=headers) + else: + response = await client.delete(url, headers=headers) response.raise_for_status() - except httpx.HTTPStatusError as e: - rejection = _parse_contract_rejection(e.response) - if rejection is not None: - raise BackendContractRejection(rejection) from e - detail = f"HTTP {e.response.status_code}: {e.response.text[:500]}" - logger.error("HTTP error calling backend %s: %s", endpoint, detail) - raise Exception(f"Backend returned {detail}") from e - except httpx.HTTPError as e: - logger.error("HTTP error calling backend %s: %s, type: %s", endpoint, e, type(e).__name__) - raise Exception(f"Failed to call backend: {e}") from e - return response.text + return response.text + except httpx.HTTPStatusError as e: + rejection = _parse_contract_rejection(e.response) + if rejection is not None: + raise BackendContractRejection(rejection) from e + detail = f"HTTP {e.response.status_code}: {e.response.text[:500]}" + logger.error("HTTP error calling backend %s: %s", endpoint, detail) + raise Exception(f"Backend returned {detail}") from e + except httpx.HTTPError as e: + logger.error("HTTP error calling backend %s: %s, type: %s", endpoint, e, type(e).__name__) + raise Exception(f"Failed to call backend: {e}") from e async def call_backend_bytes( self, From 7c26e4767d0af994e0afdffd58c13aaaca1e0e5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20Wr=C3=B3bel?= Date: Fri, 3 Jul 2026 15:14:05 +0200 Subject: [PATCH 3/3] Enhance error handling in backend contract rejection tests to ensure httpx.ConnectError is properly caught and not mistaken for httpx.HTTPError --- mcp_server/tests/test_backend_contract_rejection.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/mcp_server/tests/test_backend_contract_rejection.py b/mcp_server/tests/test_backend_contract_rejection.py index 74d758f..2a8c9ba 100644 --- a/mcp_server/tests/test_backend_contract_rejection.py +++ b/mcp_server/tests/test_backend_contract_rejection.py @@ -167,4 +167,8 @@ async def get(self, *a, **k): svc = SpecFlowBackendService() with pytest.raises(Exception) as exc_info: await svc.call_backend(endpoint="/api/v1/generation-sessions/est-1/status", method="GET") + # Must be caught and wrapped by call_backend's except httpx.HTTPError block, not + # leak out as the raw httpx.ConnectError — otherwise this assertion would pass + # even with the request call left outside the try/except (the bug being fixed). + assert not isinstance(exc_info.value, httpx.HTTPError) assert "connection refused" in str(exc_info.value)