diff --git a/packages/tangle-cli/src/tangle_cli/api_transport.py b/packages/tangle-cli/src/tangle_cli/api_transport.py index b128070..804649e 100644 --- a/packages/tangle-cli/src/tangle_cli/api_transport.py +++ b/packages/tangle-cli/src/tangle_cli/api_transport.py @@ -11,6 +11,7 @@ from typing import Any import httpx +import requests DEFAULT_API_URL = "http://localhost:8000" DEFAULT_TIMEOUT_SECONDS = 30.0 @@ -40,6 +41,85 @@ def tangle_verbose_enabled() -> bool: return value.strip().lower() in {"1", "true", "yes", "on"} +def sanitize_destination(url: str | None) -> str | None: + """Return ``scheme://host[:port]/path`` with userinfo, query, and fragment dropped. + + Userinfo can embed proxy credentials and query strings can carry signed-URL + tokens, so only the safe origin and path are kept for user-facing errors. + """ + + if not url: + return None + try: + parts = urllib.parse.urlsplit(url) + except ValueError: + return None + if not parts.scheme and not parts.netloc: + # A bare reference with no origin may itself be a signed URL; keep only + # the path portion and discard any query/fragment. + return url.split("?", 1)[0].split("#", 1)[0] or None + host = parts.hostname or "" + if ":" in host: + # ``hostname`` strips the brackets around an IPv6 literal; restore them so + # the origin stays a valid URL and any port remains unambiguous. + host = f"[{host}]" + if parts.port is not None: + host = f"{host}:{parts.port}" + return urllib.parse.urlunsplit((parts.scheme, host, parts.path, "", "")) or None + + +def transport_error_reason(exc: BaseException) -> str: + """Classify a requests transport failure into a short, safe reason phrase. + + The mapping references ``requests.exceptions`` lazily so importing this module + stays cheap and does not depend on the full requests package being present. + """ + + exceptions = requests.exceptions + # Ordered most-specific first: several transport errors share a base + # (SSLError/ProxyError subclass ConnectionError; ConnectTimeout subclasses both + # ConnectionError and Timeout), so the first match wins. + reasons: tuple[tuple[type[BaseException], str], ...] = ( + (exceptions.SSLError, "TLS verification failed"), + (exceptions.ProxyError, "proxy connection failed"), + (exceptions.ConnectTimeout, "connection timed out"), + (exceptions.ReadTimeout, "read timed out"), + (exceptions.Timeout, "request timed out"), + (exceptions.ChunkedEncodingError, "connection closed mid-response"), + (exceptions.ConnectionError, "could not connect"), + ) + for exc_type, reason in reasons: + if isinstance(exc, exc_type): + return reason + return "request failed" + + +def format_transport_error( + exc: BaseException, + *, + method: str | None = None, + url: str | None = None, +) -> str: + """Build a one-line, credential-safe message for a transport failure. + + The raw exception text is never echoed because it can contain proxy URLs or + request paths with signed-URL query secrets; the reason is derived from the + exception type and the destination is reduced to a sanitized origin+path. + """ + + request = getattr(exc, "request", None) + if url is None and request is not None: + url = getattr(request, "url", None) + if method is None and request is not None: + method = getattr(request, "method", None) + reason = transport_error_reason(exc) + destination = sanitize_destination(url) + if destination: + prefix = f"{method} " if method else "" + return f"Could not reach Tangle API ({prefix}{destination}): {reason}" + return f"Could not reach Tangle API: {reason}" + + def _redact_headers(headers: dict[str, Any] | None) -> dict[str, Any]: redacted: dict[str, Any] = {} for name, value in (headers or {}).items(): diff --git a/packages/tangle-cli/src/tangle_cli/cli.py b/packages/tangle-cli/src/tangle_cli/cli.py index e9c1fd4..f7bfb02 100644 --- a/packages/tangle-cli/src/tangle_cli/cli.py +++ b/packages/tangle-cli/src/tangle_cli/cli.py @@ -1,3 +1,8 @@ +from __future__ import annotations + +import sys + +import requests from cyclopts import App from . import ( @@ -11,6 +16,7 @@ quickstart, secrets_cli, ) +from .api_transport import format_transport_error def version() -> None: @@ -49,8 +55,39 @@ def build_app() -> App: return app +def run(tokens: list[str] | None = None) -> int: + """Dispatch the CLI, rendering transport failures as one clean stderr line. + + The static requests client raises transport failures with no HTTP response; + they are printed without a traceback and mapped to a nonzero exit. HTTP status + errors carry a response and stay the command layer's responsibility, so they + are re-raised unchanged. + """ + + try: + build_app()(tokens) + except requests.exceptions.RequestException as exc: + if getattr(exc, "response", None) is not None: + raise + print(_transport_error_line(exc), file=sys.stderr) + return 1 + return 0 + + +def _transport_error_line(exc: requests.exceptions.RequestException) -> str: + # The static client already formats its failures into a clean line, so only raw + # requests exceptions that bypassed it need formatting here. The client module is + # only inspected if it is already imported (it must be, to have raised the domain + # error), so local-only commands never load the generated API bindings. + client_module = sys.modules.get(f"{__package__}.client") + domain_error = getattr(client_module, "TangleApiTransportError", None) + if domain_error is not None and isinstance(exc, domain_error): + return str(exc) + return format_transport_error(exc) + + def main() -> None: - build_app()() + raise SystemExit(run()) if __name__ == "__main__": diff --git a/packages/tangle-cli/src/tangle_cli/client.py b/packages/tangle-cli/src/tangle_cli/client.py index 060c960..a1685a8 100644 --- a/packages/tangle-cli/src/tangle_cli/client.py +++ b/packages/tangle-cli/src/tangle_cli/client.py @@ -23,6 +23,7 @@ _normalize_base_url, _request_headers, default_base_url, + format_transport_error, log_http_exchange, tangle_verbose_enabled, ) @@ -40,6 +41,16 @@ ) +class TangleApiTransportError(requests.exceptions.RequestException): + """A connection/timeout/TLS/proxy/stream failure carrying no HTTP response. + + The message is a single, credential-safe line. Subclassing + ``requests.exceptions.RequestException`` keeps callers that already handle + requests errors working unchanged, while the originating exception is chained + as ``__cause__`` so programmatic hooks can still inspect the low-level cause. + """ + + class TangleApiClient(GeneratedTangleApiOperations): """Single public API wrapper for Tangle backends. @@ -144,6 +155,35 @@ def _make_request( ) return response + def _send_request( + self, + method: str, + path: str, + params: Mapping[str, Any] | None = None, + json_data: Any = None, + **kwargs: Any, + ) -> requests.Response: + """Issue a request, converting an unhandled transport failure to a clean error. + + ``_make_request`` and every retry/redirect layer beneath it re-raise the + original ``requests`` exception subtypes, so callers that manage their own + retries -- and the transient-retry layer -- can classify and retry them. + This public boundary is where a failure that survived all of those layers + becomes a credential-safe :class:`TangleApiTransportError`. HTTP status + errors carry a response and stay the caller's responsibility, so they + propagate unchanged. + """ + + try: + return self._make_request(method, path, params=params, json_data=json_data, **kwargs) + except requests.exceptions.RequestException as exc: + if getattr(exc, "response", None) is not None: + raise + raise TangleApiTransportError( + format_transport_error(exc, method=method.upper(), url=self._url(path)), + request=getattr(exc, "request", None), + ) from exc + def _request_with_rate_limit_retries( self, method: str, @@ -224,6 +264,9 @@ def _request_with_same_origin_redirects( for _ in range(self._MAX_REDIRECTS + 1): request_headers = self._headers(extra_headers) + # Transport failures raised here propagate untouched so the enclosing + # retry/rate-limit layers can classify them; conversion to a clean + # TangleApiTransportError happens once, at the _send_request boundary. response = self.session.request( current_method, current_url, @@ -296,7 +339,7 @@ def _request_json( response_model: Any = None, ) -> Any: formatted_path = self._format_path(path, path_params) - response = self._make_request(method, formatted_path, params=params, json_data=json_data) + response = self._send_request(method, formatted_path, params=params, json_data=json_data) response.raise_for_status() data = self._decode_response(response) if response_model is not None and isinstance(data, dict): @@ -358,7 +401,7 @@ def get_execution_details(self, execution_id: str) -> GetExecutionInfoResponse: return details def stream_execution_container_log(self, execution_id: str) -> requests.Response: - response = self._make_request( + response = self._send_request( "GET", self._format_path( "/api/executions/{id}/stream_container_log", diff --git a/packages/tangle-cli/src/tangle_cli/component_inspector.py b/packages/tangle-cli/src/tangle_cli/component_inspector.py index 92f16c5..b2a9328 100644 --- a/packages/tangle-cli/src/tangle_cli/component_inspector.py +++ b/packages/tangle-cli/src/tangle_cli/component_inspector.py @@ -33,7 +33,7 @@ def _request_path(client: TangleApiClient, path: str) -> Any: if callable(custom_request_path): response = custom_request_path(path) else: - response = client._make_request("GET", path) + response = client._send_request("GET", path) response.raise_for_status() return response diff --git a/tests/test_packaging.py b/tests/test_packaging.py index dd56d23..651de32 100644 --- a/tests/test_packaging.py +++ b/tests/test_packaging.py @@ -77,7 +77,16 @@ def _write_runtime_stubs(path: Path) -> None: " raise RuntimeError('request stub should not be called')\n" "\n" "class Response:\n" - " pass\n", + " pass\n" + "\n" + "class RequestException(Exception):\n" + " def __init__(self, *args, **kwargs):\n" + " self.response = kwargs.pop('response', None)\n" + " self.request = kwargs.pop('request', None)\n" + " super().__init__(*args)\n" + "\n" + "class exceptions:\n" + " RequestException = RequestException\n", encoding="utf-8", ) diff --git a/tests/test_transport_errors.py b/tests/test_transport_errors.py new file mode 100644 index 0000000..3a02334 --- /dev/null +++ b/tests/test_transport_errors.py @@ -0,0 +1,365 @@ +"""Transport-failure handling for the static requests client and CLI boundary.""" + +from __future__ import annotations + +import socket +from typing import Any + +import pytest +import requests +from cyclopts import App + +from tangle_cli import cli +from tangle_cli.api_transport import ( + format_transport_error, + sanitize_destination, + transport_error_reason, +) +from tangle_cli.client import TangleApiClient, TangleApiTransportError + + +class RaisingSession: + """A requests-like session whose every request raises a transport error.""" + + def __init__(self, exc: BaseException) -> None: + self.exc = exc + self.calls = 0 + + def request(self, *args: Any, **kwargs: Any) -> requests.Response: + self.calls += 1 + raise self.exc + + +def _client(exc: BaseException) -> TangleApiClient: + return TangleApiClient("https://api.test", session=RaisingSession(exc)) + + +def _prepared(method: str | None = "GET", url: str = "https://api.test/api/x") -> requests.PreparedRequest: + request = requests.PreparedRequest() + request.method = method + request.url = url + return request + + +# --- URL sanitization ------------------------------------------------------- + + +def test_sanitize_destination_strips_userinfo_and_query() -> None: + sanitized = sanitize_destination( + "https://user:pass@proxy.internal:8080/api/x?token=SEKRET&sig=abc#frag" + ) + assert sanitized == "https://proxy.internal:8080/api/x" + assert "pass" not in sanitized + assert "SEKRET" not in sanitized + + +def test_sanitize_destination_drops_signed_url_query() -> None: + sanitized = sanitize_destination( + "https://bucket.example.com/artifact?X-Amz-Signature=deadbeef&X-Amz-Credential=k" + ) + assert sanitized == "https://bucket.example.com/artifact" + + +def test_sanitize_destination_bare_path_drops_query() -> None: + assert sanitize_destination("/api/pipeline_runs?token=abc") == "/api/pipeline_runs" + + +def test_sanitize_destination_rebrackets_ipv6_host() -> None: + # ``hostname`` drops the brackets; without restoring them the origin renders as + # an ambiguous ``::1:8080`` and stops being a valid URL. + assert ( + sanitize_destination("https://user:pw@[::1]:8080/api/x?token=SEKRET") + == "https://[::1]:8080/api/x" + ) + assert sanitize_destination("http://[2001:db8::1]/api/y") == "http://[2001:db8::1]/api/y" + + +@pytest.mark.parametrize("value", [None, ""]) +def test_sanitize_destination_handles_empty(value: str | None) -> None: + assert sanitize_destination(value) is None + + +# --- reason classification -------------------------------------------------- + + +@pytest.mark.parametrize( + ("exc", "reason"), + [ + (requests.exceptions.SSLError("x"), "TLS verification failed"), + (requests.exceptions.ProxyError("x"), "proxy connection failed"), + (requests.exceptions.ConnectTimeout("x"), "connection timed out"), + (requests.exceptions.ReadTimeout("x"), "read timed out"), + (requests.exceptions.Timeout("x"), "request timed out"), + (requests.exceptions.ChunkedEncodingError("x"), "connection closed mid-response"), + (requests.exceptions.ConnectionError("x"), "could not connect"), + (requests.exceptions.RequestException("x"), "request failed"), + ], +) +def test_transport_error_reason(exc: BaseException, reason: str) -> None: + assert transport_error_reason(exc) == reason + + +def test_format_transport_error_uses_request_metadata() -> None: + exc = requests.exceptions.ConnectionError("boom", request=_prepared("POST")) + assert format_transport_error(exc) == ( + "Could not reach Tangle API (POST https://api.test/api/x): could not connect" + ) + + +def test_format_transport_error_without_destination() -> None: + assert ( + format_transport_error(requests.exceptions.Timeout("slow")) + == "Could not reach Tangle API: request timed out" + ) + + +def test_format_transport_error_never_echoes_raw_text() -> None: + # A raw requests message can embed the request path with query secrets. + exc = requests.exceptions.ConnectionError( + "HTTPConnectionPool: Max retries exceeded with url: /x?token=SEKRET", + request=_prepared(url="https://api.test/x?token=SEKRET"), + ) + message = format_transport_error(exc) + assert "SEKRET" not in message + assert "token" not in message + + +# --- client boundary -------------------------------------------------------- + + +def test_client_wraps_connection_error_as_domain_error() -> None: + cause = requests.exceptions.ConnectionError("refused") + client = _client(cause) + with pytest.raises(TangleApiTransportError) as excinfo: + client.pipeline_runs_get("run-1") + message = str(excinfo.value) + assert message == ( + "Could not reach Tangle API (GET https://api.test/api/pipeline_runs/run-1): " + "could not connect" + ) + assert "Traceback" not in message + # Cause preserved for programmatic hooks. + assert excinfo.value.__cause__ is cause + + +def test_client_wraps_timeout() -> None: + client = _client(requests.exceptions.ReadTimeout("read timed out")) + with pytest.raises(TangleApiTransportError) as excinfo: + client.pipeline_runs_get("run-1") + assert str(excinfo.value).endswith("read timed out") + + +def test_client_wraps_ssl_error() -> None: + client = _client(requests.exceptions.SSLError("certificate verify failed")) + with pytest.raises(TangleApiTransportError) as excinfo: + client.pipeline_runs_get("run-1") + assert "TLS verification failed" in str(excinfo.value) + + +def test_client_wraps_chunked_encoding_error() -> None: + client = _client(requests.exceptions.ChunkedEncodingError("peer closed")) + with pytest.raises(TangleApiTransportError) as excinfo: + client.pipeline_runs_get("run-1") + assert "connection closed mid-response" in str(excinfo.value) + + +def test_client_proxy_error_does_not_leak_credentials() -> None: + # Proxy credentials can appear in the destination URL; they must not surface. + request = _prepared(url="https://user:s3cr3t@api.test/api/pipeline_runs/run-1") + cause = requests.exceptions.ProxyError("Cannot connect to proxy", request=request) + client = _client(cause) + with pytest.raises(TangleApiTransportError) as excinfo: + client.pipeline_runs_get("run-1") + message = str(excinfo.value) + assert "s3cr3t" not in message + assert "user:" not in message + assert "proxy connection failed" in message + + +def test_client_does_not_intercept_errors_carrying_a_response() -> None: + response = requests.Response() + response.status_code = 500 + cause = requests.exceptions.RequestException("server error", response=response) + client = _client(cause) + with pytest.raises(requests.exceptions.RequestException) as excinfo: + client.pipeline_runs_get("run-1") + # Re-raised unchanged, not wrapped, so status handling stays intact. + assert excinfo.value is cause + assert not isinstance(excinfo.value, TangleApiTransportError) + + +def test_client_does_not_swallow_programmer_errors() -> None: + client = _client(ValueError("bad code")) + with pytest.raises(ValueError): + client.pipeline_runs_get("run-1") + + +def test_client_real_refused_port() -> None: + # No mocking: a genuinely closed localhost port must yield a clean domain error. + sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + sock.bind(("127.0.0.1", 0)) + port = sock.getsockname()[1] + sock.close() + + client = TangleApiClient(f"http://127.0.0.1:{port}", timeout=2.0) + with pytest.raises(TangleApiTransportError) as excinfo: + client.pipeline_runs_get("run-1") + message = str(excinfo.value) + assert message.startswith( + f"Could not reach Tangle API (GET http://127.0.0.1:{port}/api/pipeline_runs/run-1)" + ) + assert "Traceback" not in message + + +# --- wrapping boundary ------------------------------------------------------- +# +# ``_make_request`` and every retry/rate-limit/redirect layer beneath it re-raise +# the original ``requests`` exception subtypes; conversion to a clean +# TangleApiTransportError happens only at the ``_send_request`` public boundary. +# Keeping the low-level method raw is what lets a transient-retry layer inserted +# below ``_make_request`` classify and retry connect/timeout failures. These tests +# pin that split so it cannot silently regress back into an inner layer. + + +def test_inner_request_strategy_reraises_raw_transport_exception() -> None: + # The redirect layer that issues session.request must let the original requests + # exception through unconverted, so an enclosing retry layer can classify it. + cause = requests.exceptions.ConnectionError("refused") + client = _client(cause) + with pytest.raises(requests.exceptions.ConnectionError) as excinfo: + client._request_with_same_origin_redirects( + "GET", + "https://api.test/api/x", + params=None, + json_data=None, + extra_headers=None, + timeout=1.0, + request_kwargs={}, + ) + assert excinfo.value is cause + assert not isinstance(excinfo.value, TangleApiTransportError) + + +def test_make_request_surfaces_raw_transport_exception() -> None: + # _make_request stays raw so a transient-retry layer wrapping it sees the + # original subtype; only _send_request converts. This guards the composition + # with a later retry change that retries connect/timeout failures. + cause = requests.exceptions.ConnectionError("refused") + client = _client(cause) + with pytest.raises(requests.exceptions.ConnectionError) as excinfo: + client._make_request("GET", "/api/x") + assert excinfo.value is cause + assert not isinstance(excinfo.value, TangleApiTransportError) + + +def test_send_request_converts_at_public_boundary() -> None: + cause = requests.exceptions.ConnectionError("refused") + client = _client(cause) + with pytest.raises(TangleApiTransportError) as excinfo: + client._send_request("GET", "/api/pipeline_runs/run-1") + assert str(excinfo.value) == ( + "Could not reach Tangle API (GET https://api.test/api/pipeline_runs/run-1): " + "could not connect" + ) + assert excinfo.value.__cause__ is cause + assert "Traceback" not in str(excinfo.value) + + +def test_send_request_preserves_ssl_reason() -> None: + cause = requests.exceptions.SSLError("certificate verify failed") + client = _client(cause) + with pytest.raises(TangleApiTransportError) as excinfo: + client._send_request("GET", "/api/x") + assert "TLS verification failed" in str(excinfo.value) + assert excinfo.value.__cause__ is cause + + +def test_send_request_propagates_http_error_with_response_unchanged() -> None: + # Status errors carry a response and stay the caller's responsibility; the + # boundary must not swallow them into a transport error. + response = requests.Response() + response.status_code = 502 + cause = requests.exceptions.HTTPError("bad gateway", response=response) + client = _client(cause) + with pytest.raises(requests.exceptions.HTTPError) as excinfo: + client._send_request("GET", "/api/x") + assert excinfo.value is cause + assert not isinstance(excinfo.value, TangleApiTransportError) + + +# --- CLI boundary ----------------------------------------------------------- + + +def _app_raising(exc: BaseException) -> App: + app = App(name="probe") + + @app.command(name="call") + def _call() -> None: + raise exc + + return app + + +def test_run_renders_domain_error_one_line(monkeypatch, capsys) -> None: + exc = TangleApiTransportError( + "Could not reach Tangle API (GET https://api.test/api/x): could not connect", + request=_prepared(), + ) + monkeypatch.setattr(cli, "build_app", lambda: _app_raising(exc)) + + exit_code = cli.run(["call"]) + + captured = capsys.readouterr() + assert exit_code == 1 + assert captured.err.strip() == ( + "Could not reach Tangle API (GET https://api.test/api/x): could not connect" + ) + assert "Traceback" not in captured.err + + +def test_run_formats_raw_requests_exception_safety_net(monkeypatch, capsys) -> None: + # A raw requests error that bypassed the client is formatted (and redacted) here. + exc = requests.exceptions.ConnectionError( + "boom", request=_prepared(url="https://api.test/x?token=SEKRET") + ) + monkeypatch.setattr(cli, "build_app", lambda: _app_raising(exc)) + + exit_code = cli.run(["call"]) + + captured = capsys.readouterr() + assert exit_code == 1 + assert captured.err.strip() == ( + "Could not reach Tangle API (GET https://api.test/x): could not connect" + ) + assert "SEKRET" not in captured.err + + +def test_run_reraises_errors_with_response(monkeypatch) -> None: + response = requests.Response() + response.status_code = 500 + exc = requests.exceptions.HTTPError("server error", response=response) + monkeypatch.setattr(cli, "build_app", lambda: _app_raising(exc)) + + with pytest.raises(requests.exceptions.HTTPError): + cli.run(["call"]) + + +def test_run_propagates_normal_exit(monkeypatch) -> None: + app = App(name="probe") + + @app.command(name="ok") + def _ok() -> None: + return None + + monkeypatch.setattr(cli, "build_app", lambda: app) + + with pytest.raises(SystemExit) as excinfo: + cli.run(["ok"]) + assert excinfo.value.code == 0 + + +def test_run_does_not_swallow_programmer_errors(monkeypatch) -> None: + monkeypatch.setattr(cli, "build_app", lambda: _app_raising(ValueError("bug"))) + + with pytest.raises(ValueError): + cli.run(["call"])