From 4c9270743bbae81c06bb2944be6433274c400c92 Mon Sep 17 00:00:00 2001 From: Windsor Date: Wed, 17 Dec 2025 14:10:48 -0800 Subject: [PATCH] fix: improve error handling --- src/dedalus_mcp/dispatch.py | 35 ++++++++++++----- tests/test_dispatch.py | 75 ++++++++++++++++++++++++++++++++----- 2 files changed, 92 insertions(+), 18 deletions(-) diff --git a/src/dedalus_mcp/dispatch.py b/src/dedalus_mcp/dispatch.py index 5541919..050d048 100644 --- a/src/dedalus_mcp/dispatch.py +++ b/src/dedalus_mcp/dispatch.py @@ -144,15 +144,23 @@ class DispatchErrorCode(str, Enum): These represent infrastructure failures - NOT HTTP 4xx/5xx from downstream. A downstream 404 is still success=True with response.status=404. + + Wire format uses SCREAMING_CASE for machine-readability and grep-ability. """ - CONNECTION_NOT_FOUND = "connection_not_found" - CONNECTION_REVOKED = "connection_revoked" - DECRYPTION_FAILED = "decryption_failed" - INVALID_REQUEST = "invalid_request" # Malformed path, invalid URI, bad headers - DOWNSTREAM_TIMEOUT = "downstream_timeout" - DOWNSTREAM_UNREACHABLE = "downstream_unreachable" - DOWNSTREAM_TLS_ERROR = "downstream_tls_error" + CONNECTION_NOT_FOUND = "CONNECTION_NOT_FOUND" + CONNECTION_REVOKED = "CONNECTION_REVOKED" + CONNECTION_SUSPENDED = "CONNECTION_SUSPENDED" + ORG_MISMATCH = "ORG_MISMATCH" + DECRYPTION_FAILED = "DECRYPTION_FAILED" + INVALID_REQUEST = "INVALID_REQUEST" + BAD_REQUEST = "BAD_REQUEST" + DOWNSTREAM_TIMEOUT = "DOWNSTREAM_TIMEOUT" + DOWNSTREAM_UNREACHABLE = "DOWNSTREAM_UNREACHABLE" + DOWNSTREAM_TLS_ERROR = "DOWNSTREAM_TLS_ERROR" + DOWNSTREAM_AUTH_FAILURE = "DOWNSTREAM_AUTH_FAILURE" + DOWNSTREAM_RATE_LIMITED = "DOWNSTREAM_RATE_LIMITED" + ENCLAVE_UNAVAILABLE = "ENCLAVE_UNAVAILABLE" class DispatchError(BaseModel): @@ -548,7 +556,7 @@ async def dispatch(self, request: DispatchWireRequest) -> DispatchResponse: data = response.json() - # Enclave returns DispatchResponse format + # Enclave returns canonical DispatchResponse format if data.get("success"): http_resp = data.get("response", {}) return DispatchResponse.ok( @@ -560,8 +568,17 @@ async def dispatch(self, request: DispatchWireRequest) -> DispatchResponse: ) else: error_data = data.get("error", {}) + code_str = error_data.get("code", "DOWNSTREAM_UNREACHABLE") + try: + code = DispatchErrorCode(code_str) + except ValueError: + _logger.warning( + "unknown dispatch error code", + extra={"event": "dispatch.unknown_code", "code": code_str}, + ) + code = DispatchErrorCode.DOWNSTREAM_UNREACHABLE return DispatchResponse.fail( - DispatchErrorCode(error_data.get("code", "downstream_unreachable")), + code, error_data.get("message", "Unknown error"), retryable=error_data.get("retryable", False), ) diff --git a/tests/test_dispatch.py b/tests/test_dispatch.py index e8f2b7a..9156ca3 100644 --- a/tests/test_dispatch.py +++ b/tests/test_dispatch.py @@ -518,7 +518,7 @@ def resolver(handle: str) -> tuple[str, str, str]: ) assert result.success is False - assert result.error.code.value == "downstream_unreachable" + assert result.error.code.value == "DOWNSTREAM_UNREACHABLE" assert result.error.retryable is True assert "connect" in result.error.message.lower() @@ -548,7 +548,7 @@ def resolver(handle: str) -> tuple[str, str, str]: ) assert result.success is False - assert result.error.code.value == "downstream_timeout" + assert result.error.code.value == "DOWNSTREAM_TIMEOUT" assert result.error.retryable is True assert "timed out" in result.error.message.lower() @@ -741,7 +741,7 @@ async def test_enclave_dispatch_handles_403(self, respx_mock): ) assert result.success is False - assert result.error.code.value == "connection_not_found" + assert result.error.code.value == "CONNECTION_NOT_FOUND" assert "403" in result.error.message @pytest.mark.asyncio @@ -773,7 +773,7 @@ async def test_enclave_dispatch_handles_500(self, respx_mock): ) assert result.success is False - assert result.error.code.value == "downstream_unreachable" + assert result.error.code.value == "DOWNSTREAM_UNREACHABLE" assert "500" in result.error.message @pytest.mark.asyncio @@ -794,7 +794,7 @@ async def test_enclave_dispatch_error_response(self, respx_mock): json={ "success": False, "error": { - "code": "downstream_timeout", + "code": "DOWNSTREAM_TIMEOUT", "message": "Request timed out", "retryable": True, }, @@ -815,7 +815,7 @@ async def test_enclave_dispatch_error_response(self, respx_mock): ) assert result.success is False - assert result.error.code.value == "downstream_timeout" + assert result.error.code.value == "DOWNSTREAM_TIMEOUT" assert result.error.message == "Request timed out" assert result.error.retryable is True @@ -848,7 +848,7 @@ async def test_enclave_dispatch_network_error(self, respx_mock): ) assert result.success is False - assert result.error.code.value == "downstream_unreachable" + assert result.error.code.value == "DOWNSTREAM_UNREACHABLE" assert result.error.retryable is True @pytest.mark.asyncio @@ -935,7 +935,7 @@ def resolver(handle: str) -> tuple[str, str, str]: ) assert result.success is False - assert result.error.code.value == "downstream_unreachable" + assert result.error.code.value == "DOWNSTREAM_UNREACHABLE" assert "unexpected error" in result.error.message.lower() @@ -971,7 +971,7 @@ async def test_enclave_dispatch_unexpected_exception(self, respx_mock): ) assert result.success is False - assert result.error.code.value == "downstream_unreachable" + assert result.error.code.value == "DOWNSTREAM_UNREACHABLE" def test_sign_request_without_deployment_auth(self): """Signing without deployment_id/auth_secret should return empty dict.""" @@ -1027,6 +1027,63 @@ def test_creates_direct_backend_when_url_not_set(self, monkeypatch): assert isinstance(backend, DirectDispatchBackend) +class TestDispatchResponseConformance: + """Conformance tests for DispatchResponse wire format (ADR-013).""" + + @pytest.mark.asyncio + async def test_enclave_dispatch_handles_unknown_error_code(self, respx_mock): + """Unknown error codes should fall back to DOWNSTREAM_UNREACHABLE.""" + import httpx + + from dedalus_mcp.dispatch import ( + EnclaveDispatchBackend, + DispatchErrorCode, + DispatchWireRequest, + HttpMethod, + HttpRequest, + ) + + # Simulate enclave returning an unknown error code + respx_mock.post("https://enclave.example.com/dispatch").mock( + return_value=httpx.Response( + 200, + json={ + "success": False, + "error": { + "code": "SOME_FUTURE_ERROR_CODE", + "message": "Some new error", + "retryable": False, + }, + }, + ) + ) + + backend = EnclaveDispatchBackend( + enclave_url="https://enclave.example.com", + access_token="test_token", + ) + + result = await backend.dispatch( + DispatchWireRequest( + connection_handle="ddls:conn:github", + request=HttpRequest(method=HttpMethod.GET, path="/user"), + ) + ) + + assert result.success is False + assert result.error.code == DispatchErrorCode.DOWNSTREAM_UNREACHABLE + assert result.error.message == "Some new error" + + def test_dispatch_error_code_wire_format(self): + """Error codes must be SCREAMING_CASE on the wire.""" + from dedalus_mcp.dispatch import DispatchErrorCode + + # All error codes should be uppercase (wire format) + for code in DispatchErrorCode: + assert code.value == code.value.upper(), f"{code.name} value must be uppercase" + assert "_" in code.value or code.value.isalpha(), f"{code.name} must be SCREAMING_CASE" + + class TestDispatchIntegration: """Integration tests for dispatch flow."""