diff --git a/.claude/skills/build-error-adapter/SKILL.md b/.claude/skills/build-error-adapter/SKILL.md new file mode 100644 index 00000000..a945077f --- /dev/null +++ b/.claude/skills/build-error-adapter/SKILL.md @@ -0,0 +1,200 @@ +--- +name: build-error-adapter +description: Build new Arcade error adapters from scratch using public Arcade TDK patterns. Use when adding provider integrations, mapping SDK exceptions, or extending HTTP/GraphQL/auth adapter behavior. +--- + +# Build Error Adapter + +Use this workflow to create new error adapters that fit Arcade TDK conventions. + +## Official Reference + +Start here and align behavior with this doc: + +- [Arcade docs: Providing useful tool errors (Error adapters)](https://docs.arcade.dev/en/guides/create-tools/error-handling/useful-tool-errors#error-adapters) + +## Quick Context + +- Adapter protocol: `arcade_tdk.error_adapters.base.ErrorAdapter` +- Common error classes: + - `arcade_tdk.errors.UpstreamError` — upstream responded with an HTTP status code + - `arcade_tdk.errors.UpstreamRateLimitError` — 429 / quota-exhausted with `retry_after_ms` + - `arcade_tdk.errors.NetworkTransportError` — no complete response was received + (timeouts, connection/DNS/TLS failures, decoding errors, redirect exhaustion). + `status_code` is always `None`; use one of the `NETWORK_TRANSPORT_RUNTIME_*` + kinds: `_TIMEOUT`, `_UNREACHABLE`, `_UNMAPPED`. + - `arcade_tdk.errors.FatalToolError` — unrecoverable tool-authoring bug or + environment misconfiguration (invalid URL, unsupported protocol, bad headers, + TLS trust failures). Never retried. + - `arcade_tdk.errors.RetryableToolError` — transient tool-body failure with a + hint for the LLM to retry. + - `arcade_tdk.errors.ContextRequiredToolError` — needs human input before retry. + +## Rules To Follow + +1. Keep imports at top-level only (no inline imports), except optional dependency imports that must be lazy by design. +2. Adapter interface contract: + - `slug` class attribute + - `from_exception(self, exc: Exception) -> ToolRuntimeError | None` +3. Return `None` when the exception is not recognized for that adapter. +4. Return a `ToolRuntimeError` subclass for recognized exceptions (`UpstreamError`, `UpstreamRateLimitError`, etc.). +5. Preserve privacy: + - Agent-facing `message` must be safe. + - Put raw vendor detail into `developer_message` when needed. +6. Add tests for every new mapping path. +7. Match your installed Arcade version's decorator API and parameter names. + +## Privacy Rule When Uncertain + +If you are not fully sure what `str(exc)`, vendor `reason`, or nested payload fields can contain, treat them as potentially sensitive. + +- Default to a safe agent-facing message template: + - `"Upstream request failed with status code ."` + - `"Upstream error: unhandled ."` +- Put raw details in `developer_message` instead of `message`. +- Prefer structured non-secret context in `message` (status code, error class, stable provider error code). +- Never put tokens, auth headers, full URLs with query params, raw response bodies, or stack traces in agent-facing `message`. + +Use this decision rule: + +1. **Known-safe field** (documented stable code/reason without sensitive payload): may be included in `message`. +2. **Unknown or mixed-content field**: keep out of `message`; include only in `developer_message`. +3. **High-risk content** (headers/body/credential-like strings): never include in `message`; sanitize or omit even in `developer_message` if policy requires. + +When in doubt, prefer slightly less detail in `message` and richer diagnostics in `developer_message`. + +## Decide: Adapter vs explicit tool error + +Use an **error adapter** when: + +- You need repeatable translation from vendor exceptions to Arcade errors. +- The same exception family appears across multiple tools. + +Raise explicit tool errors in tool code when: + +- You need user guidance for immediate retry (`RetryableToolError`). +- You need user/orchestrator input before retry (`ContextRequiredToolError`). +- You need a special business rule for one endpoint/tool path only. + +## Implementation Pattern + +### 1) Create adapter skeleton + +```python +from arcade_core.errors import ToolRuntimeError + + +class VendorErrorAdapter: + slug = "_vendor" + + def from_exception(self, exc: Exception) -> ToolRuntimeError | None: + # recognize typed vendor exceptions + # return mapped ToolRuntimeError + return None +``` + +### 2) Use typed exception matching + +- Match most specific subclasses first. +- Keep a final typed fallback for broad vendor exceptions. +- Avoid broad `except Exception` handling inside `from_exception`. + +Example ordering: + +1. Rate limit subtype +2. Auth subtype +3. Timeout/transport subtype +4. General vendor exception fallback + +### 3) Normalize metadata + +For adapted errors: + +- Include `extra["service"] = self.slug` +- Include `extra["error_type"] = type(exc).__name__` for non-status failures +- Include sanitized endpoint/method when available + +### 4) Map status-like semantics consistently + +**Upstream responded with an HTTP status code → `UpstreamError`:** + +- 429 → `UpstreamRateLimitError` with `retry_after_ms` +- 5xx → retryable `UpstreamError` (`status_code >= 500`) +- 4xx → non-retryable `UpstreamError` + +`UpstreamError` derives retryability from status code, so predictable behavior is automatic. + +**No complete response from upstream → `NetworkTransportError`:** + +Use this class when the exception inherently means the request never reached the +upstream, or no complete response came back. `status_code` is `None` by design. + +| Exception kind | `kind=` | `can_retry=` | +|---|---|---| +| Timeouts (connect, read, pool) | `NETWORK_TRANSPORT_RUNTIME_TIMEOUT` | `True` | +| Connection refused, DNS, TLS handshake, remote-protocol errors | `NETWORK_TRANSPORT_RUNTIME_UNREACHABLE` | `True` | +| Decoding failures, generic transport fallback | `NETWORK_TRANSPORT_RUNTIME_UNMAPPED` | `True` | +| Redirect-loop exhaustion | `NETWORK_TRANSPORT_RUNTIME_UNMAPPED` | `False` | + +**Tool-authoring bugs / local environment misconfiguration → `FatalToolError`:** + +Use this class for exceptions that will never succeed on retry — the tool's +code or environment needs to change: + +- Invalid URL, unsupported scheme, missing scheme, bad headers, malformed local + HTTP protocol state +- TLS / certificate / trust configuration failures (`ssl.SSLError` and siblings) + +Do **not** dress these up as `UpstreamError` — an UpstreamError implies the +upstream service actually said "no". Miscategorizing pollutes telemetry and +sends the wrong retry signal. + +### 5) Optional dependency handling + +For SDK-specific adapters, lazy-import the SDK module inside `from_exception` if that dependency may be optional. + +- If import fails, log and return `None`. +- Do not raise import errors from adapter code paths. + +## Registration Pattern + +For `httpx` and `requests`, automatic adaptation is typically available. + +For SDK-specific adapters, register explicitly on tools. + +```python +from arcade_mcp_server import tool +from arcade_tdk.error_adapters import GoogleErrorAdapter + +@tool( + # Depending on Arcade version, this may be `adapters=` or `error_adapters=`. + adapters=[GoogleErrorAdapter()], +) +def my_tool(...) -> ...: + ... +``` + +If your project uses a different parameter name, follow your installed API docs/signature. + +## Required Test Matrix + +Create or extend tests in your project test suite: + +- recognized typed exception -> expected `ToolRuntimeError` subclass +- expected `status_code`, `kind`, `can_retry` +- expected `extra` keys (`service`, `error_type`, endpoint/method when applicable) +- unknown exception returns `None` +- optional dependency missing path returns `None` +- privacy split is verified: + - `message` stays safe for uncertain/raw exceptions + - `developer_message` carries deep diagnostics + +## Done Checklist + +- Adapter returns `ToolRuntimeError | None` +- Safe agent-facing messages +- Uncertain exception content defaults to safe templates +- Typed exception coverage added +- Tests added/updated and passing +- Any required package versioning updated for your repo rules +- No noisy stdout/stderr output in MCP tool runtime paths diff --git a/libs/arcade-tdk/arcade_tdk/providers/google/error_adapter.py b/libs/arcade-tdk/arcade_tdk/providers/google/error_adapter.py index c187191d..1bb31dba 100644 --- a/libs/arcade-tdk/arcade_tdk/providers/google/error_adapter.py +++ b/libs/arcade-tdk/arcade_tdk/providers/google/error_adapter.py @@ -222,7 +222,8 @@ class GoogleErrorAdapter: exc_info=True, ) return UpstreamError( - message=f"Upstream Google API error: {exc}", + message=f"Upstream Google API error: unhandled {exc.__class__.__name__}.", + developer_message=str(exc), status_code=500, extra={ "service": self.slug, diff --git a/libs/arcade-tdk/arcade_tdk/providers/graphql/error_adapter.py b/libs/arcade-tdk/arcade_tdk/providers/graphql/error_adapter.py index 808cc3ae..de14d91c 100644 --- a/libs/arcade-tdk/arcade_tdk/providers/graphql/error_adapter.py +++ b/libs/arcade-tdk/arcade_tdk/providers/graphql/error_adapter.py @@ -4,7 +4,12 @@ from functools import lru_cache from http import HTTPStatus from typing import Any -from arcade_core.errors import ToolRuntimeError, UpstreamError +from arcade_core.errors import ( + ErrorKind, + NetworkTransportError, + ToolRuntimeError, + UpstreamError, +) from arcade_tdk.providers.http.error_adapter import BaseHTTPErrorMapper @@ -81,12 +86,14 @@ class GraphQLErrorAdapter(BaseHTTPErrorMapper): if isinstance(exc, TransportServerError): return self._handle_transport_error(exc) - # Network/protocol errors - simple 502 + # Network/protocol errors — the upstream was never reached or never + # produced a complete response. No HTTP status is available. if isinstance(exc, (TransportConnectionFailed, TransportProtocolError)): - return UpstreamError( - message=f"Upstream GraphQL error: {type(exc).__name__}", - status_code=HTTPStatus.BAD_GATEWAY.value, - developer_message=str(exc), + return NetworkTransportError( + message=("GraphQL request failed before a complete response was received."), + developer_message=f"{type(exc).__name__}: {exc}", + kind=ErrorKind.NETWORK_TRANSPORT_RUNTIME_UNREACHABLE, + can_retry=True, extra={"service": self.slug, "error_type": type(exc).__name__}, ) @@ -147,7 +154,8 @@ class GraphQLErrorAdapter(BaseHTTPErrorMapper): return self._map_status_to_error( status=status, headers=headers or {}, - msg=f"Upstream GraphQL error: {_extract_error_message(str(exc))}", + msg=f"Upstream GraphQL request failed with status code {status}.", + developer_message=str(exc), request_url=url, request_method=method, ) diff --git a/libs/arcade-tdk/arcade_tdk/providers/http/error_adapter.py b/libs/arcade-tdk/arcade_tdk/providers/http/error_adapter.py index fed5d4a1..1c1cc1f9 100644 --- a/libs/arcade-tdk/arcade_tdk/providers/http/error_adapter.py +++ b/libs/arcade-tdk/arcade_tdk/providers/http/error_adapter.py @@ -1,10 +1,15 @@ import logging import re from datetime import datetime, timezone +from http import HTTPStatus from typing import Any from urllib.parse import urlparse from arcade_core.errors import ( + ErrorKind, + FatalToolError, + NetworkTransportError, + ToolRuntimeError, UpstreamError, UpstreamRateLimitError, ) @@ -19,6 +24,37 @@ RATE_HEADERS = ("retry-after", "x-ratelimit-reset", "x-ratelimit-reset-ms") class BaseHTTPErrorMapper: """Base class for HTTP error mapping functionality.""" + def _status_class_label(self, status: int) -> str: + if 400 <= status < 500: + return "client error" + if 500 <= status < 600: + return "server error" + if 300 <= status < 400: + return "redirection" + if 100 <= status < 200: + return "informational" + return "response" + + def _status_phrase(self, status: int) -> str: + try: + return HTTPStatus(status).phrase + except ValueError: + return "Unknown Status" + + def _build_safe_status_message(self, status: int, headers: dict[str, str]) -> str: + phrase = self._status_phrase(status) + status_class = self._status_class_label(status) + base_message = f"Upstream HTTP request failed ({phrase}, {status_class})." + + if status == 429 or (status == 403 and self._is_rate_limit_403(headers, base_message)): + retry_after_ms = self._parse_retry_ms(headers) + retry_after_seconds = retry_after_ms // 1000 + if retry_after_seconds > 0: + return f"{base_message} Retry after {retry_after_seconds} second(s)." + return f"{base_message} Rate limit encountered." + + return base_message + def _parse_numeric_header(self, value: str | None) -> float | None: """Convert numeric header values to float without relying on exceptions.""" @@ -91,6 +127,7 @@ class BaseHTTPErrorMapper: status: int, headers: dict[str, str], msg: str, + developer_message: str | None = None, request_url: str | None = None, request_method: str | None = None, ) -> UpstreamError: @@ -102,6 +139,7 @@ class BaseHTTPErrorMapper: return UpstreamRateLimitError( retry_after_ms=self._parse_retry_ms(headers), message=msg, + developer_message=developer_message, extra=extra, ) @@ -109,10 +147,104 @@ class BaseHTTPErrorMapper: return UpstreamRateLimitError( retry_after_ms=self._parse_retry_ms(headers), message=msg, + developer_message=developer_message, extra=extra, ) - return UpstreamError(message=msg, status_code=status, extra=extra) + return UpstreamError( + message=msg, + status_code=status, + developer_message=developer_message, + extra=extra, + ) + + def _build_network_transport_error( + self, + *, + exc: Exception, + kind: ErrorKind, + can_retry: bool, + message: str, + request_url: str | None, + request_method: str | None, + ) -> NetworkTransportError: + """Build a NetworkTransportError for no-response HTTP failures. + + Used for transport-level failures (timeouts, connection errors, decoding + failures, redirect-loop exhaustion) where no complete HTTP response was + received from the upstream service. + """ + return NetworkTransportError( + message=message, + developer_message=str(exc), + kind=kind, + can_retry=can_retry, + extra={ + **self._build_extra_metadata(request_url, request_method), + "error_type": type(exc).__name__, + }, + ) + + def _build_construction_error( + self, + *, + exc: Exception, + message: str, + request_url: str | None, + request_method: str | None, + ) -> FatalToolError: + """Build a FatalToolError for client-side HTTP construction bugs. + + Used for exceptions that indicate the tool built an invalid request + (bad URL, unsupported scheme, malformed headers) or local trust + configuration prevents the request from being sent (TLS/SSL). + Retrying will not help — the tool's code or environment must change. + """ + return FatalToolError( + message=message, + developer_message=str(exc), + extra={ + **self._build_extra_metadata(request_url, request_method), + "error_type": type(exc).__name__, + }, + ) + + @staticmethod + def _extract_request_info(exc: Any) -> tuple[str | None, str | None]: + """Pull ``(url, method)`` from an exception, trying in order: + + 1. ``exc.request.{url,method}`` — present on requests and httpx + exceptions when a Request was built and attached. + 2. ``exc.response.request.{url,method}`` — set on response-bearing + exceptions like ``requests.HTTPError``. + 3. ``exc.response.url`` — final fallback for URL only (no method). + + Guards each access because ``httpx.RequestError.request`` raises + ``RuntimeError`` when no request is attached, and arbitrary mocks + may omit attributes entirely. + """ + + def _safe_get(obj: Any, name: str) -> Any: + try: + return getattr(obj, name, None) + except RuntimeError: + return None + + def _as_str(value: Any) -> str | None: + return str(value) if value is not None else None + + url: str | None = None + method: str | None = None + for source in (_safe_get(exc, "request"), _safe_get(_safe_get(exc, "response"), "request")): + if source is None: + continue + url = url or _as_str(_safe_get(source, "url")) + method = method or _as_str(_safe_get(source, "method")) + if url and method: + break + if url is None: + url = _as_str(_safe_get(_safe_get(exc, "response"), "url")) + return url, method def _is_rate_limit_403(self, headers: dict[str, str], msg: str) -> bool: """ @@ -170,11 +302,11 @@ class BaseHTTPErrorMapper: class _HTTPXExceptionHandler: """Handler for httpx-specific exceptions.""" - def handle_exception(self, exc: Any, mapper: BaseHTTPErrorMapper) -> UpstreamError | None: - """Handle httpx HTTPStatusError exceptions. + def handle_exception(self, exc: Any, mapper: BaseHTTPErrorMapper) -> ToolRuntimeError | None: + """Handle typed httpx exceptions. Args: - exc: An httpx.HTTPStatusError exception + exc: An httpx exception instance mapper: The BaseHTTPErrorMapper instance to use for mapping Returns: @@ -186,33 +318,114 @@ class _HTTPXExceptionHandler: except ImportError: return None - if not isinstance(exc, httpx.HTTPStatusError): - return None + request_url, request_method = mapper._extract_request_info(exc) - response = exc.response - request_url = None - request_method = None - if hasattr(exc, "request") and exc.request: - request_url = str(exc.request.url) - request_method = exc.request.method + if isinstance(exc, httpx.HTTPStatusError): + response = exc.response + safe_message = mapper._build_safe_status_message( + response.status_code, dict(response.headers) + ) + return mapper._map_status_to_error( + response.status_code, + dict(response.headers), + safe_message, + developer_message=str(exc), + request_url=request_url, + request_method=request_method, + ) - return mapper._map_status_to_error( - response.status_code, - dict(response.headers), - str(exc), - request_url=request_url, - request_method=request_method, - ) + # Construction bugs — per-exception messages so the agent can tell + # the failures apart without reading developer_message. Checked before + # transport base classes, and before the RequestError guard because + # ``httpx.InvalidURL`` is a bare ``Exception`` (not a RequestError + # subclass in current httpx). + if isinstance(exc, httpx.InvalidURL): + return mapper._build_construction_error( + exc=exc, + message="HTTP request URL is invalid or malformed.", + request_url=request_url, + request_method=request_method, + ) + if isinstance(exc, httpx.UnsupportedProtocol): + return mapper._build_construction_error( + exc=exc, + message="HTTP request URL uses an unsupported scheme (expected http or https).", + request_url=request_url, + request_method=request_method, + ) + if isinstance(exc, httpx.LocalProtocolError): + return mapper._build_construction_error( + exc=exc, + message=( + "HTTP request violated the HTTP protocol before it was sent " + "(malformed headers or body)." + ), + request_url=request_url, + request_method=request_method, + ) + + # Order is intentional: specific subclasses before broad base classes. + if isinstance(exc, httpx.TimeoutException): + return mapper._build_network_transport_error( + exc=exc, + kind=ErrorKind.NETWORK_TRANSPORT_RUNTIME_TIMEOUT, + can_retry=True, + message="HTTP request timed out before a complete response was received.", + request_url=request_url, + request_method=request_method, + ) + + if isinstance(exc, httpx.TooManyRedirects): + return mapper._build_network_transport_error( + exc=exc, + kind=ErrorKind.NETWORK_TRANSPORT_RUNTIME_UNMAPPED, + can_retry=False, + message="HTTP redirect limit exceeded before a final response was received.", + request_url=request_url, + request_method=request_method, + ) + + if isinstance(exc, httpx.DecodingError): + return mapper._build_network_transport_error( + exc=exc, + kind=ErrorKind.NETWORK_TRANSPORT_RUNTIME_UNMAPPED, + can_retry=True, + message="HTTP response from upstream could not be decoded.", + request_url=request_url, + request_method=request_method, + ) + + if isinstance(exc, httpx.TransportError): + return mapper._build_network_transport_error( + exc=exc, + kind=ErrorKind.NETWORK_TRANSPORT_RUNTIME_UNREACHABLE, + can_retry=True, + message="HTTP request failed before reaching the upstream service.", + request_url=request_url, + request_method=request_method, + ) + + if isinstance(exc, httpx.RequestError): + return mapper._build_network_transport_error( + exc=exc, + kind=ErrorKind.NETWORK_TRANSPORT_RUNTIME_UNMAPPED, + can_retry=True, + message="HTTP request failed before a complete response was received.", + request_url=request_url, + request_method=request_method, + ) + + return None class _RequestsExceptionHandler: """Handler for requests-specific exceptions.""" - def handle_exception(self, exc: Any, mapper: BaseHTTPErrorMapper) -> UpstreamError | None: - """Handle requests library exceptions. + def handle_exception(self, exc: Any, mapper: BaseHTTPErrorMapper) -> ToolRuntimeError | None: + """Handle requests exceptions with HTTP responses. Args: - exc: A requests.exceptions.HTTPError exception + exc: A requests exception candidate mapper: The BaseHTTPErrorMapper instance to use for mapping Returns: @@ -220,33 +433,175 @@ class _RequestsExceptionHandler: """ # Lazy import requests types locally to avoid import errors for toolkits that don't use requests try: - from requests.exceptions import HTTPError # type: ignore[import-untyped] + from requests.exceptions import ( # type: ignore[import-untyped] + ConnectionError, + ContentDecodingError, + HTTPError, + InvalidSchema, + InvalidURL, + MissingSchema, + RequestException, + SSLError, + Timeout, + TooManyRedirects, + URLRequired, + ) except ImportError: return None - if not isinstance(exc, HTTPError): - return None + # Resolve version-gated exception classes separately so an older + # ``requests`` install that is missing one of them doesn't silently + # disable the entire requests adapter chain. Missing classes are + # replaced with a sentinel that no real exception is an instance of, + # turning the downstream ``isinstance()`` check into a no-op. + # - ``InvalidProxyURL``: added in requests 2.21.0 (Dec 2018). + # - ``InvalidHeader``: added in requests 2.12.0 (Nov 2016). + class _UnavailableRequestsException(Exception): + """Placeholder for a requests.exceptions class missing on this install.""" - response = getattr(exc, "response", None) - if response is None: - return None + try: + from requests.exceptions import InvalidProxyURL + except ImportError: + InvalidProxyURL = _UnavailableRequestsException - # Extract request information - request_url = None - request_method = None - if hasattr(response, "request") and response.request: - request_url = response.request.url - request_method = response.request.method - elif hasattr(response, "url"): - request_url = response.url + try: + from requests.exceptions import InvalidHeader + except ImportError: + InvalidHeader = _UnavailableRequestsException - return mapper._map_status_to_error( - response.status_code, - dict(response.headers), - str(exc), - request_url=request_url, - request_method=request_method, - ) + request_url, request_method = mapper._extract_request_info(exc) + + if isinstance(exc, HTTPError): + response = getattr(exc, "response", None) + if response is None: + return None + + safe_message = mapper._build_safe_status_message( + response.status_code, dict(response.headers) + ) + return mapper._map_status_to_error( + response.status_code, + dict(response.headers), + safe_message, + developer_message=str(exc), + request_url=request_url, + request_method=request_method, + ) + + # Construction bugs — per-exception messages so each failure mode is + # distinguishable in the agent-facing message without reading + # developer_message. + if isinstance(exc, MissingSchema): + return mapper._build_construction_error( + exc=exc, + message="HTTP request URL is missing a scheme (expected http:// or https://).", + request_url=request_url, + request_method=request_method, + ) + if isinstance(exc, InvalidSchema): + return mapper._build_construction_error( + exc=exc, + message="HTTP request URL uses an unsupported scheme (expected http or https).", + request_url=request_url, + request_method=request_method, + ) + # InvalidProxyURL is a subclass of InvalidURL — check proxy first. + if isinstance(exc, InvalidProxyURL): + return mapper._build_construction_error( + exc=exc, + message="HTTP proxy URL is invalid or malformed.", + request_url=request_url, + request_method=request_method, + ) + if isinstance(exc, InvalidURL): + return mapper._build_construction_error( + exc=exc, + message="HTTP request URL is invalid or malformed.", + request_url=request_url, + request_method=request_method, + ) + if isinstance(exc, InvalidHeader): + return mapper._build_construction_error( + exc=exc, + message="HTTP request contains an invalid header name or value.", + request_url=request_url, + request_method=request_method, + ) + if isinstance(exc, URLRequired): + return mapper._build_construction_error( + exc=exc, + message="HTTP request requires a URL but none was provided.", + request_url=request_url, + request_method=request_method, + ) + + # TLS / cert / trust failures — typically a local configuration issue. + # (SSLError is a ConnectionError subclass, so it must be checked first.) + if isinstance(exc, SSLError): + return mapper._build_construction_error( + exc=exc, + message=( + "TLS handshake failed — likely a local certificate or trust " + "configuration issue." + ), + request_url=request_url, + request_method=request_method, + ) + + # Order is intentional: specific subclasses before broad base classes. + # ``ConnectTimeout`` inherits from BOTH ``Timeout`` and ``ConnectionError`` — + # check ``Timeout`` first so it's classified as a timeout. + if isinstance(exc, Timeout): + return mapper._build_network_transport_error( + exc=exc, + kind=ErrorKind.NETWORK_TRANSPORT_RUNTIME_TIMEOUT, + can_retry=True, + message="HTTP request timed out before a complete response was received.", + request_url=request_url, + request_method=request_method, + ) + + if isinstance(exc, ConnectionError): + return mapper._build_network_transport_error( + exc=exc, + kind=ErrorKind.NETWORK_TRANSPORT_RUNTIME_UNREACHABLE, + can_retry=True, + message="HTTP request failed before reaching the upstream service.", + request_url=request_url, + request_method=request_method, + ) + + if isinstance(exc, ContentDecodingError): + return mapper._build_network_transport_error( + exc=exc, + kind=ErrorKind.NETWORK_TRANSPORT_RUNTIME_UNMAPPED, + can_retry=True, + message="HTTP response from upstream could not be decoded.", + request_url=request_url, + request_method=request_method, + ) + + if isinstance(exc, TooManyRedirects): + return mapper._build_network_transport_error( + exc=exc, + kind=ErrorKind.NETWORK_TRANSPORT_RUNTIME_UNMAPPED, + can_retry=False, + message="HTTP redirect limit exceeded before a final response was received.", + request_url=request_url, + request_method=request_method, + ) + + if isinstance(exc, RequestException): + return mapper._build_network_transport_error( + exc=exc, + kind=ErrorKind.NETWORK_TRANSPORT_RUNTIME_UNMAPPED, + can_retry=True, + message="HTTP request failed before a complete response was received.", + request_url=request_url, + request_method=request_method, + ) + + return None class HTTPErrorAdapter(BaseHTTPErrorMapper): @@ -258,7 +613,7 @@ class HTTPErrorAdapter(BaseHTTPErrorMapper): self._httpx_handler = _HTTPXExceptionHandler() self._requests_handler = _RequestsExceptionHandler() - def from_exception(self, exc: Exception) -> UpstreamError | None: + def from_exception(self, exc: Exception) -> ToolRuntimeError | None: """Convert HTTP library exceptions into Arcade errors.""" httpx_result = self._httpx_handler.handle_exception(exc, self) diff --git a/libs/arcade-tdk/arcade_tdk/providers/microsoft/error_adapter.py b/libs/arcade-tdk/arcade_tdk/providers/microsoft/error_adapter.py index 7cb7c2f9..a1dfd244 100644 --- a/libs/arcade-tdk/arcade_tdk/providers/microsoft/error_adapter.py +++ b/libs/arcade-tdk/arcade_tdk/providers/microsoft/error_adapter.py @@ -49,8 +49,9 @@ class MicrosoftGraphErrorAdapter: exc_info=True, ) return UpstreamError( - message=f"Upstream Microsoft Graph error: {exc}", + message=f"Upstream Microsoft Graph error: unhandled {exc.__class__.__name__}.", status_code=500, + developer_message=str(exc), extra={ "service": self.slug, "error_type": exc.__class__.__name__, diff --git a/libs/arcade-tdk/arcade_tdk/providers/slack/error_adapter.py b/libs/arcade-tdk/arcade_tdk/providers/slack/error_adapter.py index e54b4f14..59e53760 100644 --- a/libs/arcade-tdk/arcade_tdk/providers/slack/error_adapter.py +++ b/libs/arcade-tdk/arcade_tdk/providers/slack/error_adapter.py @@ -47,8 +47,9 @@ class SlackErrorAdapter: exc_info=True, ) return UpstreamError( - message=f"Upstream Slack SDK error: {exc}", + message=f"Upstream Slack SDK error: unhandled {exc.__class__.__name__}.", status_code=500, + developer_message=str(exc), extra={ "service": self.slug, "error_type": exc.__class__.__name__, diff --git a/libs/arcade-tdk/pyproject.toml b/libs/arcade-tdk/pyproject.toml index 53c62f64..6ae8078c 100644 --- a/libs/arcade-tdk/pyproject.toml +++ b/libs/arcade-tdk/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "arcade-tdk" -version = "3.7.0" +version = "3.8.0" description = "Arcade TDK - Toolkit Development Kit for building Arcade tools" readme = "README.md" license = { text = "MIT" } diff --git a/libs/tests/sdk/test_google_adapter.py b/libs/tests/sdk/test_google_adapter.py index 33beed89..0ae8c064 100644 --- a/libs/tests/sdk/test_google_adapter.py +++ b/libs/tests/sdk/test_google_adapter.py @@ -485,7 +485,8 @@ class TestGoogleErrorAdapter: assert isinstance(result, UpstreamError) assert result.status_code == 500 - assert result.message == "Upstream Google API error: Some unhandled Google error" + assert result.message == "Upstream Google API error: unhandled MockUnhandledError." + assert result.developer_message == "Some unhandled Google error" assert result.extra["service"] == "_google_api_client" assert result.extra["error_type"] == "MockUnhandledError" diff --git a/libs/tests/sdk/test_graphql_adapter.py b/libs/tests/sdk/test_graphql_adapter.py index de6e44dd..d5031f0f 100644 --- a/libs/tests/sdk/test_graphql_adapter.py +++ b/libs/tests/sdk/test_graphql_adapter.py @@ -8,7 +8,12 @@ from typing import Any from unittest.mock import patch import pytest -from arcade_core.errors import UpstreamError, UpstreamRateLimitError +from arcade_core.errors import ( + ErrorKind, + NetworkTransportError, + UpstreamError, + UpstreamRateLimitError, +) LIBS_DIR = Path(__file__).resolve().parents[2] TDK_SRC = LIBS_DIR / "arcade-tdk" @@ -170,6 +175,8 @@ class TestGraphQLErrorAdapter: assert isinstance(result, UpstreamError) assert result.status_code == HTTPStatus.INTERNAL_SERVER_ERROR + assert result.message == "Upstream GraphQL request failed with status code 500." + assert result.developer_message == "Server error" def test_server_error_extracts_headers_from_cause(self) -> None: """Should extract headers from __cause__ if not on exception.""" @@ -232,26 +239,30 @@ class TestGraphQLErrorAdapter: # --- Connection/Protocol error tests --- - def test_connection_failed_returns_502(self) -> None: - """Connection failures should map to 502.""" + def test_connection_failed_maps_to_network_transport_unreachable(self) -> None: + """Connection failures never reached upstream — NetworkTransportError.""" exc = DummyTransportConnectionFailed("Connection refused") with _patch_loader(): result = gql_adapter.GraphQLErrorAdapter().from_exception(exc) - assert isinstance(result, UpstreamError) - assert result.status_code == HTTPStatus.BAD_GATEWAY + assert isinstance(result, NetworkTransportError) + assert result.kind == ErrorKind.NETWORK_TRANSPORT_RUNTIME_UNREACHABLE + assert result.can_retry is True + assert result.status_code is None assert result.extra["error_type"] == "DummyTransportConnectionFailed" - def test_protocol_error_returns_502(self) -> None: - """Protocol errors should map to 502.""" + def test_protocol_error_maps_to_network_transport_unreachable(self) -> None: + """Protocol errors (incomplete / malformed exchange) → NetworkTransportError.""" exc = DummyTransportProtocolError("Invalid response") with _patch_loader(): result = gql_adapter.GraphQLErrorAdapter().from_exception(exc) - assert isinstance(result, UpstreamError) - assert result.status_code == HTTPStatus.BAD_GATEWAY + assert isinstance(result, NetworkTransportError) + assert result.kind == ErrorKind.NETWORK_TRANSPORT_RUNTIME_UNREACHABLE + assert result.can_retry is True + assert result.status_code is None assert result.extra["error_type"] == "DummyTransportProtocolError" # --- Generic TransportError catch-all --- @@ -265,6 +276,8 @@ class TestGraphQLErrorAdapter: assert isinstance(result, UpstreamError) assert result.status_code == 503 + assert result.message == "Upstream GraphQL request failed with status code 503." + assert result.developer_message == "Unknown error" # --- Edge cases --- diff --git a/libs/tests/sdk/test_httpx_adapter.py b/libs/tests/sdk/test_httpx_adapter.py index 3dd642ef..b361fafe 100644 --- a/libs/tests/sdk/test_httpx_adapter.py +++ b/libs/tests/sdk/test_httpx_adapter.py @@ -2,7 +2,16 @@ import logging from datetime import datetime, timezone from unittest.mock import Mock, patch -from arcade_core.errors import UpstreamError, UpstreamRateLimitError +import httpx +import requests + +from arcade_core.errors import ( + ErrorKind, + FatalToolError, + NetworkTransportError, + UpstreamError, + UpstreamRateLimitError, +) from arcade_tdk.providers.http.error_adapter import BaseHTTPErrorMapper, HTTPErrorAdapter @@ -186,7 +195,8 @@ class TestHTTPErrorAdapter: assert isinstance(result, UpstreamError) assert result.status_code == 404 - assert result.message == "404 Client Error: Not Found" + assert result.message == "Upstream HTTP request failed (Not Found, client error)." + assert result.developer_message == "404 Client Error: Not Found" assert result.extra["service"] == "_http" assert result.extra["endpoint"] == "https://api.example.com/users/123" assert result.extra["http_method"] == "GET" @@ -215,7 +225,11 @@ class TestHTTPErrorAdapter: assert isinstance(result, UpstreamRateLimitError) assert result.retry_after_ms == 60_000 - assert result.message == "429 Too Many Requests" + assert result.message == ( + "Upstream HTTP request failed (Too Many Requests, client error). " + "Retry after 60 second(s)." + ) + assert result.developer_message == "429 Too Many Requests" assert result.extra["service"] == "_http" assert result.extra["endpoint"] == "https://api.example.com/upload" assert result.extra["http_method"] == "POST" @@ -245,7 +259,8 @@ class TestHTTPErrorAdapter: assert isinstance(result, UpstreamError) assert result.status_code == 403 - assert result.message == "403 Forbidden" + assert result.message == "Upstream HTTP request failed (Forbidden, client error)." + assert result.developer_message == "403 Forbidden" assert result.extra["service"] == "_http" assert result.extra["endpoint"] == "https://api.example.com/protected" assert result.extra["http_method"] == "GET" @@ -271,7 +286,8 @@ class TestHTTPErrorAdapter: assert isinstance(result, UpstreamError) assert result.status_code == 500 - assert result.message == "500 Internal Server Error" + assert result.message == "Upstream HTTP request failed (Internal Server Error, server error)." + assert result.developer_message == "500 Internal Server Error" assert result.extra["service"] == "_http" assert result.extra["endpoint"] == "https://api.example.com/server-error" assert "http_method" not in result.extra # No method available @@ -291,6 +307,223 @@ class TestHTTPErrorAdapter: assert result is None + def test_requests_timeout_exception_handling(self): + """Timeout exceptions should map to retryable NetworkTransportError (TIMEOUT).""" + request = requests.Request("GET", "https://api.example.com/slow?token=secret").prepare() + exc = requests.exceptions.ReadTimeout("Request timed out", request=request) + + result = self.adapter.from_exception(exc) + + assert isinstance(result, NetworkTransportError) + assert result.status_code is None + assert result.can_retry is True + assert result.kind == ErrorKind.NETWORK_TRANSPORT_RUNTIME_TIMEOUT + assert result.extra["service"] == "_http" + assert result.extra["error_type"] == "ReadTimeout" + assert result.extra["endpoint"] == "https://api.example.com/slow" + assert result.extra["http_method"] == "GET" + + def test_requests_transport_exception_handling(self): + """Connection errors should map to NetworkTransportError (UNREACHABLE).""" + request = requests.Request("POST", "https://api.example.com/ping").prepare() + exc = requests.exceptions.ConnectionError("Connection failed", request=request) + + result = self.adapter.from_exception(exc) + + assert isinstance(result, NetworkTransportError) + assert result.status_code is None + assert result.can_retry is True + assert result.kind == ErrorKind.NETWORK_TRANSPORT_RUNTIME_UNREACHABLE + assert result.extra["service"] == "_http" + assert result.extra["error_type"] == "ConnectionError" + assert result.extra["endpoint"] == "https://api.example.com/ping" + assert result.extra["http_method"] == "POST" + + def test_requests_invalid_url_routes_to_fatal_tool_error(self): + """Invalid URL is a client construction bug — FatalToolError, not retryable.""" + request = requests.Request("GET", "https://api.example.com/bad").prepare() + exc = requests.exceptions.InvalidURL("Invalid URL", request=request) + + result = self.adapter.from_exception(exc) + + assert isinstance(result, FatalToolError) + assert result.can_retry is False + assert result.kind == ErrorKind.TOOL_RUNTIME_FATAL + assert result.message == "HTTP request URL is invalid or malformed." + assert result.extra["service"] == "_http" + assert result.extra["error_type"] == "InvalidURL" + assert result.extra["endpoint"] == "https://api.example.com/bad" + assert result.extra["http_method"] == "GET" + + def test_requests_missing_schema_routes_to_fatal_tool_error(self): + """MissingSchema is a construction bug — FatalToolError with specific message.""" + exc = requests.exceptions.MissingSchema("No scheme") + + result = self.adapter.from_exception(exc) + + assert isinstance(result, FatalToolError) + assert result.can_retry is False + assert ( + result.message + == "HTTP request URL is missing a scheme (expected http:// or https://)." + ) + assert result.extra["error_type"] == "MissingSchema" + + def test_requests_invalid_schema_routes_to_fatal_tool_error(self): + """InvalidSchema (unsupported scheme like ftp://) → FatalToolError.""" + exc = requests.exceptions.InvalidSchema("Bad scheme") + + result = self.adapter.from_exception(exc) + + assert isinstance(result, FatalToolError) + assert result.can_retry is False + assert ( + result.message + == "HTTP request URL uses an unsupported scheme (expected http or https)." + ) + assert result.extra["error_type"] == "InvalidSchema" + + def test_requests_invalid_proxy_url_routes_to_fatal_tool_error(self): + """InvalidProxyURL is a subclass of InvalidURL — proxy-specific message.""" + exc = requests.exceptions.InvalidProxyURL("bad proxy") + + result = self.adapter.from_exception(exc) + + assert isinstance(result, FatalToolError) + assert result.can_retry is False + assert result.message == "HTTP proxy URL is invalid or malformed." + assert result.extra["error_type"] == "InvalidProxyURL" + + def test_requests_invalid_header_routes_to_fatal_tool_error(self): + """InvalidHeader is a construction bug — FatalToolError.""" + exc = requests.exceptions.InvalidHeader("Bad header") + + result = self.adapter.from_exception(exc) + + assert isinstance(result, FatalToolError) + assert result.can_retry is False + assert result.message == "HTTP request contains an invalid header name or value." + assert result.extra["error_type"] == "InvalidHeader" + + def test_requests_url_required_routes_to_fatal_tool_error(self): + """URLRequired (no URL provided) → FatalToolError.""" + exc = requests.exceptions.URLRequired("No URL") + + result = self.adapter.from_exception(exc) + + assert isinstance(result, FatalToolError) + assert result.can_retry is False + assert result.message == "HTTP request requires a URL but none was provided." + assert result.extra["error_type"] == "URLRequired" + + def test_requests_ssl_error_routes_to_fatal_tool_error(self): + """SSLError is typically a local cert/trust config issue — FatalToolError.""" + exc = requests.exceptions.SSLError("bad cert") + + result = self.adapter.from_exception(exc) + + assert isinstance(result, FatalToolError) + assert result.can_retry is False + assert ( + result.message + == "TLS handshake failed — likely a local certificate or trust " + "configuration issue." + ) + assert result.extra["error_type"] == "SSLError" + + def test_requests_content_decoding_error_handling(self): + """Decode failures should map to NetworkTransportError (UNMAPPED, retryable).""" + request = requests.Request("GET", "https://api.example.com/json").prepare() + exc = requests.exceptions.ContentDecodingError("Bad payload", request=request) + + result = self.adapter.from_exception(exc) + + assert isinstance(result, NetworkTransportError) + assert result.status_code is None + assert result.can_retry is True + assert result.kind == ErrorKind.NETWORK_TRANSPORT_RUNTIME_UNMAPPED + assert result.extra["service"] == "_http" + assert result.extra["error_type"] == "ContentDecodingError" + assert result.extra["endpoint"] == "https://api.example.com/json" + assert result.extra["http_method"] == "GET" + + def test_requests_too_many_redirects_is_non_retryable(self): + """Redirect loops → NetworkTransportError (UNMAPPED, not retryable).""" + request = requests.Request("GET", "https://api.example.com/redirect-loop").prepare() + exc = requests.exceptions.TooManyRedirects("Exceeded redirect limit", request=request) + + result = self.adapter.from_exception(exc) + + assert isinstance(result, NetworkTransportError) + assert result.status_code is None + assert result.can_retry is False + assert result.kind == ErrorKind.NETWORK_TRANSPORT_RUNTIME_UNMAPPED + assert result.extra["service"] == "_http" + assert result.extra["error_type"] == "TooManyRedirects" + assert result.extra["endpoint"] == "https://api.example.com/redirect-loop" + assert result.extra["http_method"] == "GET" + + def test_requests_request_exception_fallback(self): + """Unhandled requests base exceptions → NetworkTransportError (UNMAPPED).""" + request = requests.Request("DELETE", "https://api.example.com/resource/123").prepare() + exc = requests.exceptions.RequestException("Request failed", request=request) + + result = self.adapter.from_exception(exc) + + assert isinstance(result, NetworkTransportError) + assert result.status_code is None + assert result.can_retry is True + assert result.kind == ErrorKind.NETWORK_TRANSPORT_RUNTIME_UNMAPPED + assert result.extra["service"] == "_http" + assert result.extra["error_type"] == "RequestException" + assert result.extra["endpoint"] == "https://api.example.com/resource/123" + assert result.extra["http_method"] == "DELETE" + + def test_requests_handler_degrades_gracefully_without_invalid_proxy_url( + self, monkeypatch + ): + """Older ``requests`` (<2.21.0) predates ``InvalidProxyURL``. + + In those versions, a bad proxy URL raises plain ``InvalidURL`` instead, + so the adapter should fall through to the ``InvalidURL`` handler and + still produce a ``FatalToolError`` (regression for the bulk-import bug + that used to silently disable the whole requests chain). + """ + import requests.exceptions as rex + + monkeypatch.delattr(rex, "InvalidProxyURL", raising=False) + + exc = requests.exceptions.InvalidURL("bad proxy url") + result = self.adapter.from_exception(exc) + + assert isinstance(result, FatalToolError) + assert result.can_retry is False + assert result.message == "HTTP request URL is invalid or malformed." + assert result.extra["error_type"] == "InvalidURL" + + def test_requests_handler_degrades_gracefully_without_invalid_header( + self, monkeypatch + ): + """Older ``requests`` (<2.12.0) predates ``InvalidHeader`` — same guard. + + Here we only need to prove the handler still returns a classified error + rather than ``None`` for *any* requests exception when ``InvalidHeader`` + is missing. A ``Timeout`` is the cleanest witness because it's + unambiguously a ``NetworkTransportError`` regardless of the header + routing block. + """ + import requests.exceptions as rex + + monkeypatch.delattr(rex, "InvalidHeader", raising=False) + + request = requests.Request("GET", "https://api.example.com/x").prepare() + exc = requests.exceptions.Timeout("timed out", request=request) + + result = self.adapter.from_exception(exc) + + assert isinstance(result, NetworkTransportError) + assert result.kind == ErrorKind.NETWORK_TRANSPORT_RUNTIME_TIMEOUT + def test_unhandled_exception_logs_warning(self, caplog): """Test that unhandled exceptions log a warning.""" with caplog.at_level(logging.INFO): @@ -313,6 +546,9 @@ class TestHTTPErrorAdapter: mock_response = Mock() mock_response.status_code = 400 mock_response.headers = {} + # Fully detached: neither the exception nor the response carries a Request. + mock_response.request = None + mock_response.url = None mock_exc = MockHTTPStatusError("400 Bad Request") mock_exc.response = mock_response @@ -323,11 +559,170 @@ class TestHTTPErrorAdapter: assert isinstance(result, UpstreamError) assert result.status_code == 400 - assert result.message == "400 Bad Request" + assert result.message == "Upstream HTTP request failed (Bad Request, client error)." + assert result.developer_message == "400 Bad Request" assert result.extra["service"] == "_http" assert "endpoint" not in result.extra assert "http_method" not in result.extra + def test_httpx_timeout_exception_handling(self): + """Timeout exceptions → NetworkTransportError (TIMEOUT, retryable).""" + request = httpx.Request("GET", "https://api.example.com/slow?token=secret") + exc = httpx.ReadTimeout("Read timed out", request=request) + + result = self.adapter.from_exception(exc) + + assert isinstance(result, NetworkTransportError) + assert result.status_code is None + assert result.can_retry is True + assert result.kind == ErrorKind.NETWORK_TRANSPORT_RUNTIME_TIMEOUT + assert result.extra["service"] == "_http" + assert result.extra["error_type"] == "ReadTimeout" + assert result.extra["endpoint"] == "https://api.example.com/slow" + assert result.extra["http_method"] == "GET" + + def test_httpx_pool_timeout_routes_to_timeout(self): + """PoolTimeout (local pool exhaustion) → NetworkTransportError (TIMEOUT).""" + exc = httpx.PoolTimeout("pool exhausted") + + result = self.adapter.from_exception(exc) + + assert isinstance(result, NetworkTransportError) + assert result.kind == ErrorKind.NETWORK_TRANSPORT_RUNTIME_TIMEOUT + assert result.can_retry is True + assert result.status_code is None + assert result.extra["error_type"] == "PoolTimeout" + + def test_httpx_transport_exception_handling(self): + """Transport exceptions → NetworkTransportError (UNREACHABLE, retryable).""" + request = httpx.Request("POST", "https://api.example.com/ping") + exc = httpx.ConnectError("Connection failed", request=request) + + result = self.adapter.from_exception(exc) + + assert isinstance(result, NetworkTransportError) + assert result.status_code is None + assert result.can_retry is True + assert result.kind == ErrorKind.NETWORK_TRANSPORT_RUNTIME_UNREACHABLE + assert result.extra["service"] == "_http" + assert result.extra["error_type"] == "ConnectError" + assert result.extra["endpoint"] == "https://api.example.com/ping" + assert result.extra["http_method"] == "POST" + + def test_httpx_unsupported_protocol_routes_to_fatal_tool_error(self): + """Unsupported scheme is a construction bug — FatalToolError with specific msg.""" + request = httpx.Request("GET", "ftp://api.example.com/resource") + exc = httpx.UnsupportedProtocol("Unsupported protocol", request=request) + + result = self.adapter.from_exception(exc) + + assert isinstance(result, FatalToolError) + assert result.can_retry is False + assert result.kind == ErrorKind.TOOL_RUNTIME_FATAL + assert ( + result.message + == "HTTP request URL uses an unsupported scheme (expected http or https)." + ) + assert result.extra["service"] == "_http" + assert result.extra["error_type"] == "UnsupportedProtocol" + assert result.extra["endpoint"] == "ftp://api.example.com/resource" + assert result.extra["http_method"] == "GET" + + def test_httpx_invalid_url_routes_to_fatal_tool_error(self): + """httpx.InvalidURL is a bare Exception (not RequestError); still → FatalToolError.""" + exc = httpx.InvalidURL("bad url") + + result = self.adapter.from_exception(exc) + + assert isinstance(result, FatalToolError) + assert result.can_retry is False + assert result.message == "HTTP request URL is invalid or malformed." + assert result.extra["error_type"] == "InvalidURL" + + def test_httpx_request_error_fallback(self): + """Unhandled httpx RequestError subclasses → NetworkTransportError (UNMAPPED).""" + request = httpx.Request("DELETE", "https://api.example.com/resource/123") + exc = httpx.RequestError("Request failed", request=request) + + result = self.adapter.from_exception(exc) + + assert isinstance(result, NetworkTransportError) + assert result.status_code is None + assert result.can_retry is True + assert result.kind == ErrorKind.NETWORK_TRANSPORT_RUNTIME_UNMAPPED + assert result.extra["service"] == "_http" + assert result.extra["error_type"] == "RequestError" + assert result.extra["endpoint"] == "https://api.example.com/resource/123" + assert result.extra["http_method"] == "DELETE" + + def test_httpx_decoding_error_handling(self): + """Decoding errors → NetworkTransportError (UNMAPPED, retryable).""" + request = httpx.Request("GET", "https://api.example.com/json") + exc = httpx.DecodingError("Unable to decode response body", request=request) + + result = self.adapter.from_exception(exc) + + assert isinstance(result, NetworkTransportError) + assert result.status_code is None + assert result.can_retry is True + assert result.kind == ErrorKind.NETWORK_TRANSPORT_RUNTIME_UNMAPPED + assert result.extra["service"] == "_http" + assert result.extra["error_type"] == "DecodingError" + assert result.extra["endpoint"] == "https://api.example.com/json" + assert result.extra["http_method"] == "GET" + + def test_httpx_local_protocol_error_routes_to_fatal_tool_error(self): + """LocalProtocolError = our HTTP framing was invalid (construction bug).""" + request = httpx.Request("GET", "https://api.example.com/broken") + exc = httpx.LocalProtocolError("Malformed local protocol state", request=request) + + result = self.adapter.from_exception(exc) + + assert isinstance(result, FatalToolError) + assert result.can_retry is False + assert result.kind == ErrorKind.TOOL_RUNTIME_FATAL + assert ( + result.message + == "HTTP request violated the HTTP protocol before it was sent " + "(malformed headers or body)." + ) + assert result.extra["service"] == "_http" + assert result.extra["error_type"] == "LocalProtocolError" + assert result.extra["endpoint"] == "https://api.example.com/broken" + assert result.extra["http_method"] == "GET" + + def test_httpx_remote_protocol_error_is_retryable_transport_error(self): + """RemoteProtocolError (upstream sent malformed HTTP) → UNREACHABLE, retryable.""" + request = httpx.Request("GET", "https://api.example.com/protocol") + exc = httpx.RemoteProtocolError("Malformed upstream protocol response", request=request) + + result = self.adapter.from_exception(exc) + + assert isinstance(result, NetworkTransportError) + assert result.status_code is None + assert result.can_retry is True + assert result.kind == ErrorKind.NETWORK_TRANSPORT_RUNTIME_UNREACHABLE + assert result.extra["service"] == "_http" + assert result.extra["error_type"] == "RemoteProtocolError" + assert result.extra["endpoint"] == "https://api.example.com/protocol" + assert result.extra["http_method"] == "GET" + + def test_httpx_too_many_redirects_is_non_retryable(self): + """Redirect loops → NetworkTransportError (UNMAPPED, not retryable).""" + request = httpx.Request("GET", "https://api.example.com/redirect-loop") + exc = httpx.TooManyRedirects("Exceeded redirect limit", request=request) + + result = self.adapter.from_exception(exc) + + assert isinstance(result, NetworkTransportError) + assert result.status_code is None + assert result.can_retry is False + assert result.kind == ErrorKind.NETWORK_TRANSPORT_RUNTIME_UNMAPPED + assert result.extra["service"] == "_http" + assert result.extra["error_type"] == "TooManyRedirects" + assert result.extra["endpoint"] == "https://api.example.com/redirect-loop" + assert result.extra["http_method"] == "GET" + def test_adapter_slug(self): """Test that the adapter has the correct slug.""" assert HTTPErrorAdapter.slug == "_http" @@ -503,7 +898,11 @@ class TestHTTPErrorAdapter: assert isinstance(result, UpstreamRateLimitError) assert result.retry_after_ms == 120_000 - assert result.message == "403 Forbidden" + assert result.message == ( + "Upstream HTTP request failed (Forbidden, client error). " + "Retry after 120 second(s)." + ) + assert result.developer_message == "403 Forbidden" def test_requests_403_rate_limit_handling(self): """Test handling requests 403 rate limit with exhausted quota.""" @@ -533,4 +932,33 @@ class TestHTTPErrorAdapter: assert isinstance(result, UpstreamRateLimitError) assert result.retry_after_ms == 30_000 - assert result.message == "403 Forbidden" + assert result.message == ( + "Upstream HTTP request failed (Forbidden, client error). " + "Retry after 30 second(s)." + ) + assert result.developer_message == "403 Forbidden" + + def test_http_status_message_keeps_sensitive_data_in_developer_message_only(self): + """Status messages should remain descriptive while avoiding sensitive payload leaks.""" + request = httpx.Request("GET", "https://api.example.com/users?token=secret-token") + response = httpx.Response( + 401, + request=request, + headers={"authorization": "Bearer super-secret"}, + json={"error": "token secret-token is invalid"}, + ) + exc = httpx.HTTPStatusError( + "401 Client Error: Unauthorized for url: " + "https://api.example.com/users?token=secret-token payload=secret-token", + request=request, + response=response, + ) + + result = self.adapter.from_exception(exc) + + assert isinstance(result, UpstreamError) + assert result.message == "Upstream HTTP request failed (Unauthorized, client error)." + assert "secret-token" not in result.message + assert "Bearer" not in result.message + assert "payload" not in result.message + assert "secret-token" in (result.developer_message or "") diff --git a/libs/tests/sdk/test_microsoft_adapter.py b/libs/tests/sdk/test_microsoft_adapter.py index 7222193a..e24563df 100644 --- a/libs/tests/sdk/test_microsoft_adapter.py +++ b/libs/tests/sdk/test_microsoft_adapter.py @@ -516,9 +516,8 @@ class TestMicrosoftGraphErrorAdapter: assert isinstance(result, UpstreamError) assert result.status_code == 500 - assert ( - result.message == "Upstream Microsoft Graph error: Some unhandled Microsoft Graph error" - ) + assert result.message == "Upstream Microsoft Graph error: unhandled MockUnhandledError." + assert result.developer_message == "Some unhandled Microsoft Graph error" assert result.extra["service"] == "_microsoft_graph" assert result.extra["error_type"] == "MockUnhandledError" @@ -555,7 +554,8 @@ class TestMicrosoftGraphErrorAdapter: assert isinstance(result, UpstreamError) assert result.status_code == 500 - assert result.message == "Upstream Microsoft Graph error: Core error" + assert result.message == "Upstream Microsoft Graph error: unhandled MockCoreError." + assert result.developer_message == "Core error" def test_from_exception_non_msgraph_error(self): """Test handling non-Microsoft Graph errors returns None.""" diff --git a/libs/tests/sdk/test_slack_adapter.py b/libs/tests/sdk/test_slack_adapter.py index 96797f97..d3d971e8 100644 --- a/libs/tests/sdk/test_slack_adapter.py +++ b/libs/tests/sdk/test_slack_adapter.py @@ -470,38 +470,28 @@ class TestSlackErrorAdapter: def test_from_exception_fallback_for_unhandled_slack_error(self): """Test from_exception fallback for unhandled Slack SDK errors.""" - mock_error = Mock() - mock_error.__class__.__name__ = "UnhandledSlackError" + class MockUnhandledSlackError(Exception): + pass + + mock_error = MockUnhandledSlackError("Bearer xoxb-super-secret-token") mock_error.__module__ = "slack_sdk.some_module" errors_module = self._create_mock_errors_module() - # Test that unhandled errors don't match any isinstance checks - api_result = self.adapter._handle_api_errors(mock_error, errors_module) - other_result = self.adapter._handle_other_errors(mock_error, errors_module) + mock_slack_sdk = Mock() + mock_slack_sdk.errors = errors_module - # Both should return None since the error doesn't match any known types - assert api_result is None - assert other_result is None - - # Test the failsafe logic directly - if ( - hasattr(mock_error, "__module__") - and mock_error.__module__ - and "slack_sdk" in mock_error.__module__ + with patch.dict( + "sys.modules", + {"slack_sdk": mock_slack_sdk, "slack_sdk.errors": errors_module}, ): - result = UpstreamError( - message=f"Upstream Slack SDK error: {mock_error}", - status_code=500, - extra={ - "service": self.adapter.slug, - "error_type": mock_error.__class__.__name__, - }, - ) + result = self.adapter.from_exception(mock_error) assert isinstance(result, UpstreamError) assert result.status_code == 500 + assert result.message == "Upstream Slack SDK error: unhandled MockUnhandledSlackError." + assert result.developer_message == "Bearer xoxb-super-secret-token" assert result.extra["service"] == "_slack_sdk" - assert result.extra["error_type"] == "UnhandledSlackError" + assert result.extra["error_type"] == "MockUnhandledSlackError" def test_from_exception_non_slack_error(self): """Test from_exception with non-Slack error."""