From 5a38aa5d8e86f558b6a98321648537e2327821ec Mon Sep 17 00:00:00 2001 From: DioChuks Date: Thu, 27 Aug 2026 22:05:17 +0100 Subject: [PATCH 1/9] feat: add internal _SyncHTTPClient class wrapping httpx.Client --- src/shade/http_client.py | 298 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 298 insertions(+) create mode 100644 src/shade/http_client.py diff --git a/src/shade/http_client.py b/src/shade/http_client.py new file mode 100644 index 0000000..7690a01 --- /dev/null +++ b/src/shade/http_client.py @@ -0,0 +1,298 @@ +from __future__ import annotations + +import logging +from typing import Any, Dict, Mapping, Optional + +import httpx + +from ._debug import log_request, log_response +from .config import Environment, get_config, validate_client_settings +from .config import config as _config +from .errors import NetworkError, ShadeError +from .http import ( + _BASE_BACKOFF, + _is_retryable_error, + _parse_response, + _parse_retry_after, + _retry_delay, +) + +logger = logging.getLogger(__name__) + + +def _build_full_url(base: str, path: str) -> str: + """Combine a base URL and a path, avoiding double or missing slashes. + + Examples + -------- + >>> _build_full_url("https://api.example.com", "/users") + 'https://api.example.com/users' + >>> _build_full_url("https://api.example.com/", "users") + 'https://api.example.com/users' + """ + if not base: + return path + return base.rstrip("/") + "/" + path.lstrip("/") + + +class _SyncHTTPClient: + """Internal synchronous HTTP client wrapping ``httpx.Client``. + + This is an implementation detail shared by sync resource methods through + :class:`~shade.client.ShadeClient`. It centralises header construction, + URL building, response parsing and retry logic so resources never need + to import or reference ``httpx`` directly. + + Parameters + ---------- + api_key : str, optional + Bearer token. Resolved against the global config at request time + when omitted. + api_base : str, optional + Override the API base URL. Resolved against the global config at + request time when omitted. + timeout : float, optional + Per-request socket timeout in seconds. Resolved against the global + config at request time when omitted. + environment : str | Environment, optional + Controls the default ``api_base`` and the Stellar network. + max_retries : int, optional + How many times to retry HTTP 429 and transient 5xx errors. Defaults + to the global ``shade.max_retries``. + """ + + def __init__( + self, + api_key: Optional[str] = None, + api_base: Optional[str] = None, + timeout: Optional[float] = None, + environment: Optional[Environment | str] = None, + max_retries: Optional[int] = None, + ) -> None: + self.api_key = api_key + self._api_base = api_base.rstrip("/") if api_base else None + self.environment = environment + self._timeout = timeout + self._max_retries = max_retries + if timeout is not None or max_retries is not None: + validate_client_settings( + timeout if timeout is not None else _config.timeout, + max_retries if max_retries is not None else _config.max_retries, + ) + import shade + + self._user_agent = f"shade-python/{shade.__version__}" + self._client = httpx.Client() + + @property + def max_retries(self) -> int: + return self._max_retries if self._max_retries is not None else _config.max_retries + + @property + def timeout(self) -> float: + return self._timeout if self._timeout is not None else _config.timeout + + @property + def api_base(self) -> Optional[str]: + return self._api_base + + @property + def base_url(self) -> str: + if self._api_base: + return self._api_base + env = ( + _config.parse_environment(self.environment) + if self.environment is not None + else _config.environment + ) + return _config.api_base or env.base_url.rstrip("/") + + def close(self) -> None: + """Close the underlying ``httpx.Client``.""" + self._client.close() + + def __enter__(self) -> "_SyncHTTPClient": + return self + + def __exit__(self, *args: Any) -> None: + self.close() + + def _headers( + self, + api_key: str, + has_json_body: bool, + ) -> Dict[str, str]: + headers: Dict[str, str] = { + "Accept": "application/json", + "Authorization": f"Bearer {api_key}", + "User-Agent": self._user_agent, + } + if has_json_body: + headers["Content-Type"] = "application/json" + return headers + + def request( + self, + method: str, + path: str, + params: Optional[Mapping[str, Any]] = None, + json: Any = None, + ) -> Dict[str, Any]: + """Execute an HTTP request, retrying on 429/transient errors. + + Parameters + ---------- + method : str + HTTP verb (``"GET"``, ``"POST"``, …). + path : str + API path, e.g. ``"/payments"``. Combined with the resolved + base URL. + params : Mapping[str, Any], optional + Query-string parameters encoded and appended to the URL. + json : Any, optional + JSON-serializable request body. When provided the + ``Content-Type: application/json`` header is added. + + Returns + ------- + dict + Decoded JSON response body. + + Raises + ------ + ~shade.errors.AuthenticationError + For HTTP 401/403. + ~shade.errors.InvalidRequestError + For HTTP 400/422. + ~shade.errors.NotFoundError + For HTTP 404. + ~shade.errors.RateLimitError + For HTTP 429 once retries are exhausted. + ~shade.errors.NetworkError + For HTTP 5xx once retries are exhausted, or unrecoverable + transport failures. + ~shade.errors.HTTPError + For any other non-2xx status. + ~shade.errors.ShadeError + When a 2xx response body is not valid JSON. + """ + cfg = get_config( + api_key=self.api_key, + environment=self.environment, + api_base=self._api_base, + timeout=self._timeout, + max_retries=self._max_retries, + ) + + url = _build_full_url(cfg.base_url, path) + headers = self._headers(cfg.api_key, has_json_body=json is not None) + + attempt = 0 + while True: + if _config.debug: + log_request(method, url, headers, json if json is not None else params) + + try: + response = self._client.request( + method.upper(), + url, + headers=headers, + params=params, + json=json, + timeout=cfg.timeout, + ) + except Exception as exc: + if _is_retryable_error(exc): + if attempt >= cfg.max_retries: + raise NetworkError( + "Request failed after exhausting retries", + status_code=None, + ) from exc + delay = _retry_delay(attempt, _BASE_BACKOFF) + logger.debug( + "Retrying request after transient failure (attempt %s/%s) in %.3fs", + attempt + 1, + cfg.max_retries + 1, + delay, + ) + import time + + time.sleep(delay) + attempt += 1 + continue + raise + + if _config.debug: + log_response(response.status_code, response.headers, response.text) + + if response.status_code == 429: + retry_after = _parse_retry_after(response.headers) + if attempt < cfg.max_retries: + wait = ( + retry_after + if retry_after is not None + else _retry_delay(attempt, _BASE_BACKOFF) + ) + logger.debug( + "Retrying request after 429 (attempt %s/%s) in %.3fs", + attempt + 1, + cfg.max_retries + 1, + wait, + ) + import time + + time.sleep(wait) + attempt += 1 + continue + + try: + return _parse_response(response) + except Exception as exc: + if ( + attempt < cfg.max_retries + and _is_retryable_error(exc) + ): + delay = _retry_delay(attempt, _BASE_BACKOFF) + logger.debug( + "Retrying request after retryable status (attempt %s/%s) in %.3fs", + attempt + 1, + cfg.max_retries + 1, + delay, + ) + import time + + time.sleep(delay) + attempt += 1 + continue + raise + + def get( + self, + path: str, + params: Optional[Mapping[str, Any]] = None, + ) -> Dict[str, Any]: + return self.request("GET", path, params=params) + + def post( + self, + path: str, + params: Optional[Mapping[str, Any]] = None, + json: Any = None, + ) -> Dict[str, Any]: + return self.request("POST", path, params=params, json=json) + + def patch( + self, + path: str, + params: Optional[Mapping[str, Any]] = None, + json: Any = None, + ) -> Dict[str, Any]: + return self.request("PATCH", path, params=params, json=json) + + def delete( + self, + path: str, + params: Optional[Mapping[str, Any]] = None, + json: Any = None, + ) -> Dict[str, Any]: + return self.request("DELETE", path, params=params, json=json) From 1fcfc782a2e8d4b005ff848959ba7c2f8372f214 Mon Sep 17 00:00:00 2001 From: DioChuks Date: Thu, 27 Aug 2026 22:05:47 +0100 Subject: [PATCH 2/9] test: add test for _SyncHTTPClient class wrapping httpx.Client --- tests/test_http_client.py | 586 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 586 insertions(+) create mode 100644 tests/test_http_client.py diff --git a/tests/test_http_client.py b/tests/test_http_client.py new file mode 100644 index 0000000..742b403 --- /dev/null +++ b/tests/test_http_client.py @@ -0,0 +1,586 @@ +""" +Tests for the internal ``_SyncHTTPClient`` wrapper (issue #15). + +Covers: +* URL construction for GET, POST, PATCH, DELETE +* Default headers: User-Agent, Accept, Content-Type, Authorization +* Query parameters and JSON request bodies +* Response parsing and error handling +* Single shared httpx.Client per ShadeClient lifetime +* Closing ShadeClient closes the underlying client +* Resources delegate through the shared wrapper +""" +from __future__ import annotations + +from typing import Any, List, Optional +from unittest.mock import MagicMock, patch + +import httpx +import pytest + +import shade +from shade import BaseResource, Gateway, ShadeClient +from shade.client import API_KEY_ENV_VAR, ENVIRONMENT_ENV_VAR, reset_default_client +from shade.config import Environment +from shade.config import config as _config +from shade.errors import ( + AuthenticationError, + HTTPError, + InvalidRequestError, + NetworkError, + NotFoundError, + RateLimitError, + ShadeError, +) +from shade.http_client import _SyncHTTPClient, _build_full_url + + +@pytest.fixture(autouse=True) +def _reset(monkeypatch): + monkeypatch.delenv(API_KEY_ENV_VAR, raising=False) + monkeypatch.delenv(ENVIRONMENT_ENV_VAR, raising=False) + _config.reset() + reset_default_client() + yield + _config.reset() + reset_default_client() + + +# --------------------------------------------------------------------------- +# URL construction helper +# --------------------------------------------------------------------------- + + +class TestBuildFullUrl: + @pytest.mark.parametrize( + "base, path, expected", + [ + ("https://api.example.com", "/users", "https://api.example.com/users"), + ("https://api.example.com/", "users", "https://api.example.com/users"), + ("https://api.example.com/", "/users", "https://api.example.com/users"), + ("https://api.example.com", "users", "https://api.example.com/users"), + ( + "https://api.example.com/v1", + "/payments/pay_1", + "https://api.example.com/v1/payments/pay_1", + ), + ( + "https://api.example.com/v1/", + "payments/pay_1", + "https://api.example.com/v1/payments/pay_1", + ), + ], + ) + def test_combines_base_and_path_without_double_slashes( + self, base: str, path: str, expected: str + ): + assert _build_full_url(base, path) == expected + + +# --------------------------------------------------------------------------- +# Helpers for intercepting httpx calls +# --------------------------------------------------------------------------- + + +def _stub_httpx_client( + http_wrapper: _SyncHTTPClient, + responses: List[httpx.Response], +) -> List[dict]: + """Replace the underlying ``httpx.Client.request`` and capture calls. + + Returns a list that will be populated with the kwargs of every call. + """ + captured: List[dict] = [] + response_iter = iter(responses) + + def fake_request(*args, **kwargs): + captured.append({"args": args, **kwargs}) + return next(response_iter) + + http_wrapper._client.request = fake_request # type: ignore[method-assign] + return captured + + +def _resp( + status: int = 200, + *, + json_body: Any = None, + text: Optional[str] = None, + headers: Optional[dict] = None, +) -> httpx.Response: + kwargs: dict[str, Any] = {"status_code": status, "headers": headers or {}} + if json_body is not None: + kwargs["json"] = json_body + elif text is not None: + kwargs["text"] = text + return httpx.Response(**kwargs) + + +def _make_client(**overrides) -> _SyncHTTPClient: + kwargs = { + "api_key": "sk_test_xxx", + "api_base": "https://api.example.com", + "timeout": 5.0, + **overrides, + } + return _SyncHTTPClient(**kwargs) + + +# --------------------------------------------------------------------------- +# URL construction for each HTTP verb +# --------------------------------------------------------------------------- + + +class TestUrlConstruction: + def test_get_builds_correct_url(self): + client = _make_client() + captured = _stub_httpx_client(client, [_resp(200, json_body={})]) + + client.get("/resources") + + assert captured[0]["args"] == ("GET", "https://api.example.com/resources") + + def test_post_builds_correct_url(self): + client = _make_client() + captured = _stub_httpx_client(client, [_resp(200, json_body={})]) + + client.post("/resources", json={"name": "x"}) + + assert captured[0]["args"] == ("POST", "https://api.example.com/resources") + + def test_patch_builds_correct_url(self): + client = _make_client() + captured = _stub_httpx_client(client, [_resp(200, json_body={})]) + + client.patch("/resources/1", json={"name": "y"}) + + assert captured[0]["args"] == ("PATCH", "https://api.example.com/resources/1") + + def test_delete_builds_correct_url(self): + client = _make_client() + captured = _stub_httpx_client(client, [_resp(200, json_body={})]) + + client.delete("/resources/1") + + assert captured[0]["args"] == ("DELETE", "https://api.example.com/resources/1") + + def test_handles_trailing_slash_on_api_base(self): + client = _make_client(api_base="https://api.example.com/") + captured = _stub_httpx_client(client, [_resp(200, json_body={})]) + + client.get("users") + + assert captured[0]["args"] == ("GET", "https://api.example.com/users") + + +# --------------------------------------------------------------------------- +# Headers +# --------------------------------------------------------------------------- + + +class TestHeaders: + def test_authorization_bearer_api_key(self): + client = _make_client(api_key="sk_live_secret") + captured = _stub_httpx_client(client, [_resp(200, json_body={})]) + + client.get("/x") + + assert captured[0]["headers"]["Authorization"] == "Bearer sk_live_secret" + + def test_accept_application_json(self): + client = _make_client() + captured = _stub_httpx_client(client, [_resp(200, json_body={})]) + + client.get("/x") + + assert captured[0]["headers"]["Accept"] == "application/json" + + def test_user_agent_includes_version(self): + client = _make_client() + captured = _stub_httpx_client(client, [_resp(200, json_body={})]) + + client.get("/x") + + expected = f"shade-python/{shade.__version__}" + assert captured[0]["headers"]["User-Agent"] == expected + + def test_content_type_json_on_post_with_body(self): + client = _make_client() + captured = _stub_httpx_client(client, [_resp(200, json_body={})]) + + client.post("/x", json={"k": "v"}) + + assert captured[0]["headers"]["Content-Type"] == "application/json" + + def test_content_type_json_on_patch_with_body(self): + client = _make_client() + captured = _stub_httpx_client(client, [_resp(200, json_body={})]) + + client.patch("/x", json={"k": "v"}) + + assert captured[0]["headers"]["Content-Type"] == "application/json" + + def test_no_content_type_on_get(self): + client = _make_client() + captured = _stub_httpx_client(client, [_resp(200, json_body={})]) + + client.get("/x") + + assert "Content-Type" not in captured[0]["headers"] + + def test_no_content_type_on_delete_without_body(self): + client = _make_client() + captured = _stub_httpx_client(client, [_resp(200, json_body={})]) + + client.delete("/x") + + assert "Content-Type" not in captured[0]["headers"] + + +# --------------------------------------------------------------------------- +# Request parameters +# --------------------------------------------------------------------------- + + +class TestRequestParameters: + def test_query_params_are_forwarded(self): + client = _make_client() + captured = _stub_httpx_client(client, [_resp(200, json_body={})]) + + client.get("/x", params={"limit": 10, "status": "paid"}) + + assert captured[0]["params"] == {"limit": 10, "status": "paid"} + + def test_json_body_is_forwarded(self): + client = _make_client() + captured = _stub_httpx_client(client, [_resp(200, json_body={})]) + body = {"amount": 10.0, "currency": "USD"} + + client.post("/x", json=body) + + assert captured[0]["json"] == body + + def test_timeout_forwarded(self): + client = _make_client(timeout=7.5) + captured = _stub_httpx_client(client, [_resp(200, json_body={})]) + + client.get("/x") + + assert captured[0]["timeout"] == 7.5 + + +# --------------------------------------------------------------------------- +# Response parsing +# --------------------------------------------------------------------------- + + +class TestResponseParsing: + def test_200_returns_parsed_dict(self): + client = _make_client() + _stub_httpx_client( + client, [_resp(200, json_body={"id": "res_1", "status": "ok"})] + ) + + result = client.get("/res/res_1") + + assert result == {"id": "res_1", "status": "ok"} + + def test_empty_body_returns_empty_dict(self): + client = _make_client() + _stub_httpx_client(client, [_resp(204, text="")]) + + result = client.delete("/res/1") + + assert result == {} + + def test_non_dict_2xx_raises_shade_error(self): + client = _make_client() + _stub_httpx_client(client, [_resp(200, json_body=[1, 2, 3])]) + + with pytest.raises(ShadeError, match="Invalid response from API"): + client.get("/x") + + def test_non_json_2xx_raises_shade_error(self): + client = _make_client() + _stub_httpx_client(client, [_resp(200, text="not json")]) + + with pytest.raises(ShadeError, match="Invalid response from API"): + client.get("/x") + + +# --------------------------------------------------------------------------- +# Error mapping (consistent with existing conventions) +# --------------------------------------------------------------------------- + + +class TestErrorResponses: + def test_401_maps_to_authentication_error(self): + client = _make_client() + _stub_httpx_client( + client, + [_resp(401, json_body={"error": {"message": "bad token"}})], + ) + + with pytest.raises(AuthenticationError) as exc: + client.get("/x") + assert exc.value.status_code == 401 + + def test_400_maps_to_invalid_request_error(self): + client = _make_client() + _stub_httpx_client( + client, + [_resp(400, json_body={"error": {"message": "bad input"}})], + ) + + with pytest.raises(InvalidRequestError) as exc: + client.post("/x", json={}) + assert exc.value.status_code == 400 + + def test_404_maps_to_not_found_error(self): + client = _make_client() + _stub_httpx_client( + client, + [_resp(404, json_body={"error": {"message": "gone"}})], + ) + + with pytest.raises(NotFoundError) as exc: + client.get("/missing") + assert exc.value.status_code == 404 + + def test_429_maps_to_rate_limit_error(self): + client = _make_client(max_retries=0) + _stub_httpx_client( + client, + [ + _resp( + 429, + json_body={"error": {"message": "slow down"}}, + headers={"Retry-After": "5"}, + ) + ], + ) + + with pytest.raises(RateLimitError) as exc: + client.get("/x") + assert exc.value.status_code == 429 + assert exc.value.retry_after == 5 + + def test_5xx_maps_to_network_error_when_retries_exhausted(self): + client = _make_client(max_retries=0) + _stub_httpx_client( + client, + [_resp(502, json_body={"error": {"message": "upstream"}})], + ) + + with pytest.raises(NetworkError) as exc: + client.get("/x") + assert exc.value.status_code == 502 + + def test_other_non_2xx_maps_to_http_error(self): + client = _make_client() + _stub_httpx_client( + client, + [_resp(418, json_body={"error": {"message": "teapot"}})], + ) + + with pytest.raises(HTTPError) as exc: + client.get("/x") + assert exc.value.status_code == 418 + + +# --------------------------------------------------------------------------- +# Client lifecycle — single shared httpx.Client +# --------------------------------------------------------------------------- + + +class TestClientLifecycle: + def test_single_httpx_client_instance_reused_across_requests(self): + client = _make_client() + captured = _stub_httpx_client( + client, + [ + _resp(200, json_body={"a": 1}), + _resp(200, json_body={"b": 2}), + _resp(200, json_body={"c": 3}), + ], + ) + + client.get("/one") + client.post("/two", json={"k": "v"}) + client.delete("/three") + + # Three requests captured — confirming the same wrapper drove all three + assert len(captured) == 3 + + def test_shade_client_has_one_sync_http_wrapper(self): + sc = ShadeClient(api_key="sk_test_xxx") + + assert isinstance(sc._http, _SyncHTTPClient) + + def test_resources_share_the_same_wrapper(self): + sc = ShadeClient(api_key="sk_test_xxx") + + class Res(BaseResource): + def fetch(self): + return self._request("GET", "/x") + + a = Res(client=sc) + b = Res(client=sc) + + assert a.client._http is b.client._http + assert a.client._http is sc._http + + def test_close_closes_underlying_httpx_client(self): + client = _make_client() + underlying = client._client + + with patch.object(underlying, "close", wraps=underlying.close) as mock_close: + client.close() + mock_close.assert_called_once() + + def test_shade_client_close_closes_sync_wrapper(self): + sc = ShadeClient(api_key="sk_test_xxx") + wrapper = sc._http + + with patch.object(wrapper, "close", wraps=wrapper.close) as mock_close: + sc.close() + mock_close.assert_called_once() + + def test_context_manager_closes_wrapper(self): + wrapper_close_calls = [] + sc = ShadeClient(api_key="sk_test_xxx") + + orig_close = sc._http.close + sc._http.close = lambda: wrapper_close_calls.append(True) # type: ignore[method-assign] + + with sc: + pass + + assert len(wrapper_close_calls) == 1 + + def test_no_per_request_httpx_client_construction(self): + sc = ShadeClient(api_key="sk_test_xxx") + wrapper = sc._http + original_client = wrapper._client + + _stub_httpx_client( + wrapper, + [ + _resp(200, json_body={}), + _resp(200, json_body={}), + ], + ) + + wrapper.get("/a") + wrapper.post("/b", json={}) + + # The same object reference — a new one was not created per call + assert wrapper._client is original_client + + +# --------------------------------------------------------------------------- +# Integration via ShadeClient and resources +# --------------------------------------------------------------------------- + + +class _TestResource(BaseResource): + def list(self, limit: int = 5): + return self._request("GET", f"/things?limit={limit}") + + def create(self, data: dict): + return self._request("POST", "/things", data) + + def update(self, thing_id: str, data: dict): + return self._request("PATCH", f"/things/{thing_id}", data) + + def remove(self, thing_id: str): + return self._request("DELETE", f"/things/{thing_id}") + + +class TestResourceIntegration: + def test_resource_get_routes_through_wrapper(self): + sc = ShadeClient(api_key="sk_test_xxx") + captured = _stub_httpx_client(sc._http, [_resp(200, json_body={"data": []})]) + res = _TestResource(client=sc) + + res.list(limit=3) + + assert captured[0]["args"][0] == "GET" + assert "/things" in captured[0]["args"][1] + + def test_resource_post_routes_through_wrapper(self): + sc = ShadeClient(api_key="sk_test_xxx") + captured = _stub_httpx_client(sc._http, [_resp(201, json_body={"id": "t1"})]) + res = _TestResource(client=sc) + + res.create({"name": "widget"}) + + assert captured[0]["args"][0] == "POST" + assert captured[0]["json"] == {"name": "widget"} + + def test_resource_patch_routes_through_wrapper(self): + sc = ShadeClient(api_key="sk_test_xxx") + captured = _stub_httpx_client(sc._http, [_resp(200, json_body={"id": "t1"})]) + res = _TestResource(client=sc) + + res.update("t1", {"name": "gizmo"}) + + assert captured[0]["args"][0] == "PATCH" + assert captured[0]["json"] == {"name": "gizmo"} + + def test_resource_delete_routes_through_wrapper(self): + sc = ShadeClient(api_key="sk_test_xxx") + captured = _stub_httpx_client(sc._http, [_resp(204, text="")]) + res = _TestResource(client=sc) + + res.remove("t1") + + assert captured[0]["args"][0] == "DELETE" + assert captured[0]["args"][1].endswith("/things/t1") + + def test_gateway_process_payment_uses_post(self): + gw = Gateway(api_key="sk_test_xxx") + captured = _stub_httpx_client(gw._http, [_resp(200, json_body={"id": "p1"})]) + + gw.process_payment(9.99, "USD") + + assert captured[0]["args"] == ("POST", "https://testnet.api.shadeprotocol.io/v1/payments") + assert captured[0]["json"] == {"amount": 9.99, "currency": "USD"} + + +# --------------------------------------------------------------------------- +# Public API boundary — httpx types must not leak +# --------------------------------------------------------------------------- + + +class TestPublicApiBoundary: + def test_sync_http_client_is_not_exported_from_package(self): + public_api = getattr(shade, "__all__", []) + assert "_SyncHTTPClient" not in public_api + + def test_wrapper_request_returns_dict_not_httpx_response(self): + client = _make_client() + _stub_httpx_client(client, [_resp(200, json_body={"ok": True})]) + + result = client.get("/x") + + assert isinstance(result, dict) + assert not isinstance(result, httpx.Response) + + def test_resource_request_returns_dict(self): + sc = ShadeClient(api_key="sk_test_xxx") + _stub_httpx_client(sc._http, [_resp(200, json_body={"ok": True})]) + + result = _TestResource(client=sc).list() + + assert isinstance(result, dict) + assert not isinstance(result, httpx.Response) + + def test_gateway_process_payment_returns_dict(self): + gw = Gateway(api_key="sk_test_xxx") + _stub_httpx_client(gw._http, [_resp(200, json_body={"id": "p1"})]) + + result = gw.process_payment(1.0, "USD") + + assert isinstance(result, dict) + assert not isinstance(result, httpx.Response) From f2206548b259bf7540d3387c433418e15a636f70 Mon Sep 17 00:00:00 2001 From: DioChuks Date: Thu, 27 Aug 2026 22:06:06 +0100 Subject: [PATCH 3/9] feat: ShadeClient._http now uses _SyncHTTPClient; close() shuts it down --- src/shade/client.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/shade/client.py b/src/shade/client.py index e626ec1..547cebf 100644 --- a/src/shade/client.py +++ b/src/shade/client.py @@ -17,7 +17,8 @@ from .config import Environment, validate_client_settings from .config import config as _config -from .http import AsyncHTTPClient, HTTPXTransport, SyncHTTPClient +from .http import AsyncHTTPClient, HTTPXTransport +from .http_client import _SyncHTTPClient API_KEY_ENV_VAR = "SHADE_API_KEY" ENVIRONMENT_ENV_VAR = "SHADE_ENVIRONMENT" @@ -97,8 +98,8 @@ def __init__( max_retries if max_retries is not None else _config.max_retries, ) - self._http = SyncHTTPClient( - base_url=self._api_base, + self._http = _SyncHTTPClient( + api_base=self._api_base, api_key=self._api_key, environment=self._environment, max_retries=self._max_retries, @@ -192,6 +193,7 @@ def api_base(self) -> str: return self._base_url def close(self) -> None: + self._http.close() self._client.close() def __enter__(self) -> "ShadeClient": From 419f4b01434b440149f0f8a25e357e45de4646e9 Mon Sep 17 00:00:00 2001 From: DioChuks Date: Thu, 27 Aug 2026 22:06:29 +0100 Subject: [PATCH 4/9] feat: _request() passes json=payload (kwargs) instead of positional --- src/shade/resources/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shade/resources/base.py b/src/shade/resources/base.py index 6d9f5f3..2ba6418 100644 --- a/src/shade/resources/base.py +++ b/src/shade/resources/base.py @@ -40,7 +40,7 @@ def _request( payload: Optional[Dict[str, Any]] = None, ) -> Dict[str, Any]: """Send a request through this resource's client and return the body.""" - return self.client._http.request(method, path, payload) + return self.client._http.request(method, path, json=payload) async def _request_async( self, From 3fc21a9fd2128f65b7dd23ef674e77d7661a8bcd Mon Sep 17 00:00:00 2001 From: DioChuks Date: Thu, 27 Aug 2026 22:06:54 +0100 Subject: [PATCH 5/9] feat: process_payment() calls self._http.post(..., json=...) --- src/shade/gateway.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/shade/gateway.py b/src/shade/gateway.py index e847eef..f70ad3f 100644 --- a/src/shade/gateway.py +++ b/src/shade/gateway.py @@ -30,10 +30,9 @@ def process_payment(self, amount: float, currency: str) -> Dict[str, Any]: dict API response body. """ - return self._http.request( - "POST", + return self._http.post( "/payments", - {"amount": amount, "currency": currency}, + json={"amount": amount, "currency": currency}, ) async def process_payment_async( From d744deaf9157c187c2f5d043c410a389f9b43890 Mon Sep 17 00:00:00 2001 From: DioChuks Date: Thu, 27 Aug 2026 22:07:59 +0100 Subject: [PATCH 6/9] chore: update 9 tests patching strategy for the httpx-backed transport --- tests/test_client_settings.py | 18 ++++++--- tests/test_gateway.py | 2 +- tests/test_global_config.py | 76 ++++++++++++++++++++++++----------- tests/test_shade_client.py | 26 ++++++++++-- 4 files changed, 87 insertions(+), 35 deletions(-) diff --git a/tests/test_client_settings.py b/tests/test_client_settings.py index 77c002b..ea92a7c 100644 --- a/tests/test_client_settings.py +++ b/tests/test_client_settings.py @@ -181,16 +181,22 @@ def fake_execute(req): mock_sleep.assert_not_called() def test_shade_client_max_retries_zero_disables_retries(self): + import httpx + client = ShadeClient(api_key="test-key", max_retries=0) - def fake_execute(req): - return 429, {"Retry-After": "3"}, self._fake_429_body() + def fake_request(*args, **kwargs): + return httpx.Response( + status_code=429, + headers={"Retry-After": "3"}, + content=self._fake_429_body(), + ) - with patch.object(client._http, "_execute", side_effect=fake_execute), patch( - "time.sleep" - ) as mock_sleep: + with patch.object( + client._http._client, "request", side_effect=fake_request + ), patch("time.sleep") as mock_sleep: with pytest.raises(RateLimitError): - client._http.request("POST", "/payments", {}) + client._http.request("POST", "/payments", json={}) mock_sleep.assert_not_called() diff --git a/tests/test_gateway.py b/tests/test_gateway.py index 278748d..faaf92f 100644 --- a/tests/test_gateway.py +++ b/tests/test_gateway.py @@ -17,7 +17,7 @@ def test_process_payment(): assert result == mock_response mock_req.assert_called_once_with( - "POST", "/payments", {"amount": 100.0, "currency": "USD"} + "POST", "/payments", params=None, json={"amount": 100.0, "currency": "USD"} ) def test_process_payment_async(): diff --git a/tests/test_global_config.py b/tests/test_global_config.py index b5fd89d..050ba83 100644 --- a/tests/test_global_config.py +++ b/tests/test_global_config.py @@ -75,6 +75,34 @@ def test_setting_api_key_none_after_init_raises(self): gateway.process_payment(50.0, "USD") +def _patch_httpx_and_capture(client_wrapper, json_response_body=None, status_code=200, headers=None): + """Patch ``wrapper._client.request`` and return (patch_context, capture_list). + + ``capture_list`` will contain one dict per call with keys: ``url``, ``headers``, + ``method``, ``params``, ``json``. + """ + import httpx + + captured = [] + + def fake_request(*args, **kwargs): + captured.append( + { + "method": args[0] if args else kwargs.get("method", ""), + "url": args[1] if len(args) > 1 else kwargs.get("url", ""), + "headers": kwargs.get("headers", {}), + "params": kwargs.get("params"), + "json": kwargs.get("json"), + } + ) + body_kwargs: dict = {"text": ""} + if json_response_body is not None: + body_kwargs = {"json": json_response_body} + return httpx.Response(status_code=status_code, headers=headers or {}, **body_kwargs) + + return patch.object(client_wrapper._client, "request", side_effect=fake_request), captured + + class TestGlobalConfigResourceCalls: def test_gateway_uses_global_api_key_and_environment(self): shade.api_key = "sk_live_global" @@ -82,15 +110,16 @@ def test_gateway_uses_global_api_key_and_environment(self): gateway = Gateway() - with patch.object(gateway._http, "_execute") as mock_exec: - mock_exec.return_value = (200, {}, b'{"id": "pay_1", "status": "success"}') + ctx, captured = _patch_httpx_and_capture( + gateway._http, {"id": "pay_1", "status": "success"} + ) + with ctx: res = gateway.process_payment(200.0, "USD") assert res == {"id": "pay_1", "status": "success"} - mock_exec.assert_called_once() - req = mock_exec.call_args[0][0] - assert req.headers["Authorization"] == "Bearer sk_live_global" - assert req.full_url.startswith("https://api.shadeprotocol.io/v1") + assert len(captured) == 1 + assert captured[0]["headers"]["Authorization"] == "Bearer sk_live_global" + assert captured[0]["url"].startswith("https://api.shadeprotocol.io/v1") class TestInstanceOverridesBeatsGlobalConfig: @@ -98,34 +127,34 @@ def test_instance_api_key_beats_global(self): shade.api_key = "sk_global" gateway = Gateway(api_key="sk_instance_override") - with patch.object(gateway._http, "_execute") as mock_exec: - mock_exec.return_value = (200, {}, b'{"ok": true}') + ctx, captured = _patch_httpx_and_capture(gateway._http, {"ok": True}) + with ctx: gateway.process_payment(10.0, "USD") - req = mock_exec.call_args[0][0] - assert req.headers["Authorization"] == "Bearer sk_instance_override" + assert len(captured) == 1 + assert captured[0]["headers"]["Authorization"] == "Bearer sk_instance_override" def test_instance_api_base_beats_global(self): shade.api_base = "https://global-base.example.com" gateway = Gateway(api_key="sk_test", api_base="https://override-base.example.com") - with patch.object(gateway._http, "_execute") as mock_exec: - mock_exec.return_value = (200, {}, b'{"ok": true}') + ctx, captured = _patch_httpx_and_capture(gateway._http, {"ok": True}) + with ctx: gateway.process_payment(10.0, "USD") - req = mock_exec.call_args[0][0] - assert req.full_url.startswith("https://override-base.example.com") + assert len(captured) == 1 + assert captured[0]["url"].startswith("https://override-base.example.com") def test_gateway_environment_override_propagates_to_subclients(self): shade.environment = "sandbox" gateway = Gateway(api_key="sk_test", environment="production") - with patch.object(gateway._http, "_execute") as mock_exec: - mock_exec.return_value = (200, {}, b'{"ok": true}') + ctx, captured = _patch_httpx_and_capture(gateway._http, {"ok": True}) + with ctx: gateway.process_payment(10.0, "USD") - req = mock_exec.call_args[0][0] - assert req.full_url.startswith(Environment.PRODUCTION.base_url) + assert len(captured) == 1 + assert captured[0]["url"].startswith(Environment.PRODUCTION.base_url) def test_gateway_setter_updates_propagate_to_subclients(self): gateway = Gateway(api_key="sk_initial", environment="sandbox") @@ -133,13 +162,13 @@ def test_gateway_setter_updates_propagate_to_subclients(self): gateway.api_key = "sk_updated_setter" gateway.environment = "production" - with patch.object(gateway._http, "_execute") as mock_exec: - mock_exec.return_value = (200, {}, b'{"ok": true}') + ctx, captured = _patch_httpx_and_capture(gateway._http, {"ok": True}) + with ctx: gateway.process_payment(10.0, "USD") - req = mock_exec.call_args[0][0] - assert req.headers["Authorization"] == "Bearer sk_updated_setter" - assert req.full_url.startswith(Environment.PRODUCTION.base_url) + assert len(captured) == 1 + assert captured[0]["headers"]["Authorization"] == "Bearer sk_updated_setter" + assert captured[0]["url"].startswith(Environment.PRODUCTION.base_url) @@ -194,4 +223,3 @@ def read_override(): # Step 3: Worker thread re-used; check that stale thread-local override is invalidated future_read = executor.submit(read_override) assert future_read.result() is None - diff --git a/tests/test_shade_client.py b/tests/test_shade_client.py index 5d22749..6ab8211 100644 --- a/tests/test_shade_client.py +++ b/tests/test_shade_client.py @@ -50,11 +50,29 @@ def _capture_requests(client: ShadeClient): """Patch a client's sync transport, returning the list of sent requests.""" sent = [] - def fake_execute(req): - sent.append(req) - return 200, {}, b'{"id": "pay_1"}' + def fake_request(*args, **kwargs): + import httpx - return patch.object(client._http, "_execute", side_effect=fake_execute), sent + class _FakeReq: + def __init__(self, method, url, headers): + self._method = method + self._url = url + self._headers = headers + + def get_header(self, name): + return self._headers.get(name) + + @property + def full_url(self): + return self._url + + method = args[0] if args else kwargs.get("method", "") + url = args[1] if len(args) > 1 else kwargs.get("url", "") + headers = kwargs.get("headers", {}) + sent.append(_FakeReq(method, url, headers)) + return httpx.Response(status_code=200, json={"id": "pay_1"}) + + return patch.object(client._http._client, "request", side_effect=fake_request), sent # --------------------------------------------------------------------------- From 6e917e52cee7269a0c6510db47e83f8d8f1fb5d3 Mon Sep 17 00:00:00 2001 From: DioChuks Date: Fri, 28 Aug 2026 00:01:00 +0100 Subject: [PATCH 7/9] fix: non-idempotency requests --- src/shade/http_client.py | 77 ++++++++++-- tests/test_gateway.py | 8 +- tests/test_http_client.py | 259 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 331 insertions(+), 13 deletions(-) diff --git a/src/shade/http_client.py b/src/shade/http_client.py index 7690a01..4a7eea6 100644 --- a/src/shade/http_client.py +++ b/src/shade/http_client.py @@ -12,6 +12,7 @@ from .http import ( _BASE_BACKOFF, _is_retryable_error, + _is_retryable_status, _parse_response, _parse_retry_after, _retry_delay, @@ -117,6 +118,9 @@ def __enter__(self) -> "_SyncHTTPClient": def __exit__(self, *args: Any) -> None: self.close() + _IDEMPOTENT_METHODS = frozenset({"GET", "HEAD", "OPTIONS", "PUT", "DELETE", "PATCH"}) + _IDEMPOTENCY_KEY_HEADER = "Idempotency-Key" + def _headers( self, api_key: str, @@ -131,15 +135,44 @@ def _headers( headers["Content-Type"] = "application/json" return headers + def _merge_headers( + self, + base: Mapping[str, str], + extra: Optional[Mapping[str, str]], + ) -> Dict[str, str]: + if not extra: + return dict(base) + merged = dict(base) + for k, v in extra.items(): + if v is None: + merged.pop(k, None) + else: + merged[k] = v + return merged + + def _has_idempotency_key(self, headers: Mapping[str, str]) -> bool: + for name in headers: + if name.lower() == self._IDEMPOTENCY_KEY_HEADER.lower(): + return True + return False + def request( self, method: str, path: str, params: Optional[Mapping[str, Any]] = None, json: Any = None, + headers: Optional[Mapping[str, str]] = None, ) -> Dict[str, Any]: """Execute an HTTP request, retrying on 429/transient errors. + Non-idempotent methods (``POST`` without an ``Idempotency-Key`` + header) are *not* automatically retried on 5xx or transport + failures to avoid the risk of duplicate side-effects such as + double-charging a payment. They are still retried on HTTP 429, + since a rate-limit response proves the server declined to process + the request and no side-effect occurred. + Parameters ---------- method : str @@ -152,6 +185,10 @@ def request( json : Any, optional JSON-serializable request body. When provided the ``Content-Type: application/json`` header is added. + headers : Mapping[str, str], optional + Per-request headers merged on top of the defaults. Pass an + ``Idempotency-Key`` here to safely retry ``POST`` requests + the server guarantees to be idempotent. Returns ------- @@ -184,25 +221,35 @@ def request( max_retries=self._max_retries, ) + method_upper = method.upper() url = _build_full_url(cfg.base_url, path) - headers = self._headers(cfg.api_key, has_json_body=json is not None) + final_headers = self._merge_headers( + self._headers(cfg.api_key, has_json_body=json is not None), + headers, + ) + safe_to_retry = ( + method_upper in self._IDEMPOTENT_METHODS + or self._has_idempotency_key(final_headers) + ) attempt = 0 while True: if _config.debug: - log_request(method, url, headers, json if json is not None else params) + log_request( + method_upper, url, final_headers, json if json is not None else params + ) try: response = self._client.request( - method.upper(), + method_upper, url, - headers=headers, + headers=final_headers, params=params, json=json, timeout=cfg.timeout, ) except Exception as exc: - if _is_retryable_error(exc): + if safe_to_retry and _is_retryable_error(exc): if attempt >= cfg.max_retries: raise NetworkError( "Request failed after exhausting retries", @@ -248,9 +295,13 @@ def request( try: return _parse_response(response) except Exception as exc: + retryable = _is_retryable_error(exc) + if not retryable and isinstance(exc, ShadeError): + retryable = _is_retryable_status(exc.status_code or 0) if ( - attempt < cfg.max_retries - and _is_retryable_error(exc) + safe_to_retry + and attempt < cfg.max_retries + and retryable ): delay = _retry_delay(attempt, _BASE_BACKOFF) logger.debug( @@ -270,29 +321,33 @@ def get( self, path: str, params: Optional[Mapping[str, Any]] = None, + headers: Optional[Mapping[str, str]] = None, ) -> Dict[str, Any]: - return self.request("GET", path, params=params) + return self.request("GET", path, params=params, headers=headers) def post( self, path: str, params: Optional[Mapping[str, Any]] = None, json: Any = None, + headers: Optional[Mapping[str, str]] = None, ) -> Dict[str, Any]: - return self.request("POST", path, params=params, json=json) + return self.request("POST", path, params=params, json=json, headers=headers) def patch( self, path: str, params: Optional[Mapping[str, Any]] = None, json: Any = None, + headers: Optional[Mapping[str, str]] = None, ) -> Dict[str, Any]: - return self.request("PATCH", path, params=params, json=json) + return self.request("PATCH", path, params=params, json=json, headers=headers) def delete( self, path: str, params: Optional[Mapping[str, Any]] = None, json: Any = None, + headers: Optional[Mapping[str, str]] = None, ) -> Dict[str, Any]: - return self.request("DELETE", path, params=params, json=json) + return self.request("DELETE", path, params=params, json=json, headers=headers) diff --git a/tests/test_gateway.py b/tests/test_gateway.py index faaf92f..db0ca2f 100644 --- a/tests/test_gateway.py +++ b/tests/test_gateway.py @@ -17,7 +17,11 @@ def test_process_payment(): assert result == mock_response mock_req.assert_called_once_with( - "POST", "/payments", params=None, json={"amount": 100.0, "currency": "USD"} + "POST", + "/payments", + params=None, + json={"amount": 100.0, "currency": "USD"}, + headers=None, ) def test_process_payment_async(): @@ -33,4 +37,4 @@ def test_process_payment_async(): assert result == mock_response mock_req.assert_called_once_with( "POST", "/payments", {"amount": 50.0, "currency": "EUR"} - ) \ No newline at end of file + ) diff --git a/tests/test_http_client.py b/tests/test_http_client.py index 742b403..56468ca 100644 --- a/tests/test_http_client.py +++ b/tests/test_http_client.py @@ -584,3 +584,262 @@ def test_gateway_process_payment_returns_dict(self): assert isinstance(result, dict) assert not isinstance(result, httpx.Response) + + +# --------------------------------------------------------------------------- +# Idempotency-safe retry behaviour +# --------------------------------------------------------------------------- + + +def _call_counting_responses( + client: _SyncHTTPClient, + responses: List[httpx.Response], +) -> int: + """Drive responses through the wrapper and return the number of calls made. + + The last response in the list is expected to be a 2xx (or equivalent + success) so ``request()`` returns; otherwise the count is the number + of attempts before raising. + """ + captured = _stub_httpx_client(client, list(responses)) + try: + client.request( + responses[0].request.method if responses[0].request else "GET", + "/any", + json={}, + ) + except Exception: + pass + return len(captured) + + +class TestIdempotencySafeRetry: + def test_get_5xx_is_retried(self): + client = _make_client() + captured = _stub_httpx_client( + client, + [ + _resp(502, json_body={"error": {"message": "bad gateway"}}), + _resp(200, json_body={"ok": True}), + ], + ) + + result = client.get("/x") + + assert result == {"ok": True} + assert len(captured) == 2 + + def test_post_5xx_is_not_retried(self): + client = _make_client() + captured = _stub_httpx_client( + client, + [ + _resp(502, json_body={"error": {"message": "bad gateway"}}), + _resp(200, json_body={"should": "never-reach-this"}), + ], + ) + + with pytest.raises(NetworkError): + client.post("/payments", json={"amount": 10}) + + assert len(captured) == 1 + + def test_patch_5xx_is_retried(self): + client = _make_client() + captured = _stub_httpx_client( + client, + [ + _resp(503, json_body={"error": {"message": "unavailable"}}), + _resp(200, json_body={"ok": True}), + ], + ) + + result = client.patch("/x", json={"a": 1}) + + assert result == {"ok": True} + assert len(captured) == 2 + + def test_delete_5xx_is_retried(self): + client = _make_client() + captured = _stub_httpx_client( + client, + [ + _resp(504, json_body={"error": {"message": "timeout"}}), + _resp(204, text=""), + ], + ) + + result = client.delete("/x") + + assert result == {} + assert len(captured) == 2 + + def test_post_5xx_is_retried_when_idempotency_key_present(self): + client = _make_client() + captured = _stub_httpx_client( + client, + [ + _resp(502, json_body={"error": {"message": "bad gateway"}}), + _resp(200, json_body={"id": "p1"}), + ], + ) + + result = client.post( + "/payments", + json={"amount": 10}, + headers={"Idempotency-Key": "unique-key-abc"}, + ) + + assert result == {"id": "p1"} + assert len(captured) == 2 + + def test_post_429_is_always_retried(self): + client = _make_client() + captured = _stub_httpx_client( + client, + [ + _resp( + 429, + json_body={"error": {"message": "slow"}}, + headers={"Retry-After": "1"}, + ), + _resp(200, json_body={"id": "p1"}), + ], + ) + + with patch("time.sleep"): + result = client.post("/payments", json={"amount": 10}) + + assert result == {"id": "p1"} + assert len(captured) == 2 + + def test_post_429_with_idempotency_key_still_works(self): + client = _make_client() + captured = _stub_httpx_client( + client, + [ + _resp( + 429, + json_body={"error": {"message": "slow"}}, + headers={"Retry-After": "1"}, + ), + _resp(200, json_body={"id": "p1"}), + ], + ) + + with patch("time.sleep"): + result = client.post( + "/payments", + json={"amount": 10}, + headers={"Idempotency-Key": "abc123"}, + ) + + assert result == {"id": "p1"} + assert len(captured) == 2 + + def test_post_transport_error_is_not_retried(self): + client = _make_client() + call_count = {"n": 0} + + def fake_request(*args, **kwargs): + call_count["n"] += 1 + raise httpx.ConnectError("no route to host") + + client._client.request = fake_request # type: ignore[method-assign] + + with pytest.raises(httpx.ConnectError): + client.post("/payments", json={"amount": 10}) + + assert call_count["n"] == 1 + + def test_post_transport_error_is_retried_with_idempotency_key(self): + client = _make_client() + captured: List[dict] = [] + + def fake_request(*args, **kwargs): + captured.append({"args": args, **kwargs}) + if len(captured) == 1: + raise httpx.ConnectError("no route to host") + return httpx.Response(status_code=200, json={"id": "p1"}) + + client._client.request = fake_request # type: ignore[method-assign] + + with patch("time.sleep"): + result = client.post( + "/payments", + json={"amount": 10}, + headers={"Idempotency-Key": "xyz"}, + ) + + assert result == {"id": "p1"} + assert len(captured) == 2 + + def test_get_transport_error_is_retried(self): + client = _make_client() + captured: List[dict] = [] + + def fake_request(*args, **kwargs): + captured.append({"args": args, **kwargs}) + if len(captured) == 1: + raise httpx.TimeoutException("timed out") + return httpx.Response(status_code=200, json={"ok": True}) + + client._client.request = fake_request # type: ignore[method-assign] + + with patch("time.sleep"): + result = client.get("/items") + + assert result == {"ok": True} + assert len(captured) == 2 + + def test_idempotency_key_header_case_insensitive(self): + client = _make_client() + captured = _stub_httpx_client( + client, + [ + _resp(502, json_body={"error": {"message": "bad gateway"}}), + _resp(200, json_body={"id": "p1"}), + ], + ) + + result = client.post( + "/payments", + json={"amount": 10}, + headers={"idempotency-key": "lowercased-key"}, + ) + + assert result == {"id": "p1"} + assert len(captured) == 2 + + def test_post_exhausts_retries_with_idempotency_key(self): + client = _make_client(max_retries=2) + captured = _stub_httpx_client( + client, + [ + _resp(502, json_body={"error": {"message": "bad gateway"}}), + _resp(502, json_body={"error": {"message": "bad gateway"}}), + _resp(502, json_body={"error": {"message": "bad gateway"}}), + ], + ) + + with patch("time.sleep"): + with pytest.raises(NetworkError): + client.post( + "/payments", + json={"amount": 10}, + headers={"Idempotency-Key": "retry-3"}, + ) + + # 3 total attempts: initial + 2 retries + assert len(captured) == 3 + + def test_extra_headers_are_merged(self): + client = _make_client() + captured = _stub_httpx_client(client, [_resp(200, json_body={})]) + + client.get("/x", headers={"X-Custom": "hello"}) + + assert captured[0]["headers"]["X-Custom"] == "hello" + # defaults still present + assert captured[0]["headers"]["Accept"] == "application/json" + assert captured[0]["headers"]["Authorization"] == "Bearer sk_test_xxx" From 3c0543056c3a5a2e7e484791f798c78e608a792a Mon Sep 17 00:00:00 2001 From: DioChuks Date: Fri, 28 Aug 2026 00:28:29 +0100 Subject: [PATCH 8/9] fix: Remove PATCH from the _IDEMPOTENT_METHODS frozenset --- src/shade/http_client.py | 2 +- tests/test_http_client.py | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/shade/http_client.py b/src/shade/http_client.py index 4a7eea6..e2949e2 100644 --- a/src/shade/http_client.py +++ b/src/shade/http_client.py @@ -118,7 +118,7 @@ def __enter__(self) -> "_SyncHTTPClient": def __exit__(self, *args: Any) -> None: self.close() - _IDEMPOTENT_METHODS = frozenset({"GET", "HEAD", "OPTIONS", "PUT", "DELETE", "PATCH"}) + _IDEMPOTENT_METHODS = frozenset({"GET", "HEAD", "OPTIONS", "PUT", "DELETE"}) _IDEMPOTENCY_KEY_HEADER = "Idempotency-Key" def _headers( diff --git a/tests/test_http_client.py b/tests/test_http_client.py index 56468ca..8a9200a 100644 --- a/tests/test_http_client.py +++ b/tests/test_http_client.py @@ -644,20 +644,20 @@ def test_post_5xx_is_not_retried(self): assert len(captured) == 1 - def test_patch_5xx_is_retried(self): + def test_patch_5xx_is_not_retried(self): client = _make_client() captured = _stub_httpx_client( client, [ _resp(503, json_body={"error": {"message": "unavailable"}}), - _resp(200, json_body={"ok": True}), + _resp(200, json_body={"should": "never-reach-this"}), ], ) - result = client.patch("/x", json={"a": 1}) + with pytest.raises(NetworkError): + client.patch("/x", json={"a": 1}) - assert result == {"ok": True} - assert len(captured) == 2 + assert len(captured) == 1 def test_delete_5xx_is_retried(self): client = _make_client() From cb884bc7b4b3b682c30ff3a1e1a6335eb9b0e38a Mon Sep 17 00:00:00 2001 From: DioChuks Date: Fri, 28 Aug 2026 06:45:52 +0100 Subject: [PATCH 9/9] fix: coderabbit reviews --- src/shade/http_client.py | 27 ++++++++++++++++++--- tests/test_http_client.py | 49 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 71 insertions(+), 5 deletions(-) diff --git a/src/shade/http_client.py b/src/shade/http_client.py index e2949e2..a2e4356 100644 --- a/src/shade/http_client.py +++ b/src/shade/http_client.py @@ -1,6 +1,7 @@ from __future__ import annotations import logging +import urllib.parse from typing import Any, Dict, Mapping, Optional import httpx @@ -144,9 +145,11 @@ def _merge_headers( return dict(base) merged = dict(base) for k, v in extra.items(): - if v is None: - merged.pop(k, None) - else: + k_fold = k.casefold() + matching_keys = [existing_k for existing_k in merged if existing_k.casefold() == k_fold] + for existing_k in matching_keys: + del merged[existing_k] + if v is not None: merged[k] = v return merged @@ -223,6 +226,24 @@ def request( method_upper = method.upper() url = _build_full_url(cfg.base_url, path) + + parsed_url = urllib.parse.urlparse(url) + if parsed_url.scheme.lower() == "http": + hostname = (parsed_url.hostname or "").lower() + is_local = hostname in ("localhost", "127.0.0.1", "::1") + has_withheld_auth = False + if headers: + for k, v in headers.items(): + if k.casefold() == "authorization" and v is None: + has_withheld_auth = True + break + if not is_local: + raise ValueError("HTTPS is required for non-local API bases") + if not has_withheld_auth: + raise ValueError( + "Cleartext HTTP API bases are not allowed when sending bearer credentials" + ) + final_headers = self._merge_headers( self._headers(cfg.api_key, has_json_body=json is not None), headers, diff --git a/tests/test_http_client.py b/tests/test_http_client.py index 8a9200a..3b7047e 100644 --- a/tests/test_http_client.py +++ b/tests/test_http_client.py @@ -107,12 +107,15 @@ def _resp( json_body: Any = None, text: Optional[str] = None, headers: Optional[dict] = None, + request: Optional[httpx.Request] = None, ) -> httpx.Response: kwargs: dict[str, Any] = {"status_code": status, "headers": headers or {}} if json_body is not None: kwargs["json"] = json_body elif text is not None: kwargs["text"] = text + if request is not None: + kwargs["request"] = request return httpx.Response(**kwargs) @@ -236,6 +239,45 @@ def test_no_content_type_on_delete_without_body(self): assert "Content-Type" not in captured[0]["headers"] + def test_merge_headers_case_insensitive_replacement(self): + client = _make_client() + captured = _stub_httpx_client(client, [_resp(200, json_body={})]) + + client.get("/x", headers={"authorization": "Bearer custom_token"}) + + assert "Authorization" not in captured[0]["headers"] + assert captured[0]["headers"]["authorization"] == "Bearer custom_token" + + def test_merge_headers_case_insensitive_none_removal(self): + client = _make_client() + captured = _stub_httpx_client(client, [_resp(200, json_body={})]) + + client.get("/x", headers={"authorization": None}) + + assert "Authorization" not in captured[0]["headers"] + assert "authorization" not in captured[0]["headers"] + + +class TestCleartextHttp: + def test_rejects_non_local_cleartext_http(self): + client = _make_client(api_base="http://api.shadeprotocol.io") + with pytest.raises(ValueError, match="HTTPS is required"): + client.get("/x") + + def test_rejects_local_cleartext_http_when_authorization_not_withheld(self): + client = _make_client(api_base="http://localhost:8000") + with pytest.raises(ValueError, match="Cleartext HTTP API bases are not allowed"): + client.get("/x") + + def test_allows_local_cleartext_http_when_authorization_withheld(self): + client = _make_client(api_base="http://localhost:8000") + captured = _stub_httpx_client(client, [_resp(200, json_body={})]) + + client.get("/x", headers={"Authorization": None}) + + assert "Authorization" not in captured[0]["headers"] + assert len(captured) == 1 + # --------------------------------------------------------------------------- # Request parameters @@ -594,6 +636,7 @@ def test_gateway_process_payment_returns_dict(self): def _call_counting_responses( client: _SyncHTTPClient, responses: List[httpx.Response], + method: str = "GET", ) -> int: """Drive responses through the wrapper and return the number of calls made. @@ -602,13 +645,15 @@ def _call_counting_responses( of attempts before raising. """ captured = _stub_httpx_client(client, list(responses)) + has_request = getattr(responses[0], "_request", None) is not None + req_method = responses[0].request.method if has_request else method try: client.request( - responses[0].request.method if responses[0].request else "GET", + req_method, "/any", json={}, ) - except Exception: + except ShadeError: pass return len(captured)