From 02a43129c9532c645822fdabc6456fa4e84cc60d Mon Sep 17 00:00:00 2001 From: Andrew Miller Date: Sat, 13 Jun 2026 12:43:43 -0500 Subject: [PATCH 1/3] Step 18: transport robustness, typing cleanup, and coverage polish - Wrap 2xx malformed JSON in VisorTransportError instead of leaking a raw JSONDecodeError from the success path - Replace bare `dict` return annotations with `dict[str, Any]` in _transport.py; drop type: ignore[type-arg] and type: ignore[no-any-return] - Add Retry-After HTTP-date and invalid-value tests (async + sync); _transport.py now at 100% coverage - Add snapshot_date serialization, include serialization, and empty-list omission tests in test_filter_model.py - Document unreachable sold_within_days+snapshot_date mutual-exclusion branch with an explanatory comment Co-Authored-By: Claude Sonnet 4.6 --- src/visor/_transport.py | 14 +++-- src/visor/models/_base.py | 3 + tests/test_filter_model.py | 32 ++++++++++ tests/test_transport.py | 122 +++++++++++++++++++++++++++++++++++++ 4 files changed, 167 insertions(+), 4 deletions(-) diff --git a/src/visor/_transport.py b/src/visor/_transport.py index 20d5828..bda1c30 100644 --- a/src/visor/_transport.py +++ b/src/visor/_transport.py @@ -1,5 +1,6 @@ from datetime import datetime from email.utils import parsedate_to_datetime +from typing import Any import httpx @@ -30,9 +31,12 @@ def _parse_retry_after(value: str | None) -> int | None: return max(0, int((retry_at - datetime.now(retry_at.tzinfo)).total_seconds())) -def _handle_response(response: httpx.Response) -> dict: # type: ignore[type-arg] +def _handle_response(response: httpx.Response) -> dict[str, Any]: if response.is_success: - return response.json() # type: ignore[no-any-return] + try: + return response.json() # type: ignore[no-any-return] + except ValueError as e: + raise VisorTransportError(f"Received malformed JSON from API: {e}") from e try: body = response.json() @@ -86,7 +90,9 @@ def __init__( timeout=timeout, ) - async def get(self, path: str, params: dict[str, str] | None = None) -> dict: # type: ignore[type-arg] + async def get( + self, path: str, params: dict[str, str] | None = None + ) -> dict[str, Any]: try: response = await self._client.get(path, params=params or {}) except httpx.RequestError as e: @@ -110,7 +116,7 @@ def __init__( timeout=timeout, ) - def get(self, path: str, params: dict[str, str] | None = None) -> dict: # type: ignore[type-arg] + def get(self, path: str, params: dict[str, str] | None = None) -> dict[str, Any]: try: response = self._client.get(path, params=params or {}) except httpx.RequestError as e: diff --git a/src/visor/models/_base.py b/src/visor/models/_base.py index 25c8f12..97ee7bb 100644 --- a/src/visor/models/_base.py +++ b/src/visor/models/_base.py @@ -225,6 +225,9 @@ def _validate_geo_and_inventory(self) -> "ListingsFilterBase": and self.inventory_status != InventoryMode.ACTIVE ): raise ValueError("snapshot_date requires inventory_status='active'") + # NOTE: this branch is unreachable in practice — earlier checks already + # enforce sold_within_days→SOLD and snapshot_date→ACTIVE, making both + # non-None simultaneously impossible. Kept as a logical guard. if self.sold_within_days is not None and self.snapshot_date is not None: raise ValueError( "sold_within_days and snapshot_date are mutually exclusive" diff --git a/tests/test_filter_model.py b/tests/test_filter_model.py index 68bb6f6..6442da8 100644 --- a/tests/test_filter_model.py +++ b/tests/test_filter_model.py @@ -142,6 +142,38 @@ def test_valid_facets_accepted() -> None: assert params["facets"] == "make,model,price" +# --------------------------------------------------------------------------- +# snapshot_date serialization +# --------------------------------------------------------------------------- + + +def test_snapshot_date_serializes() -> None: + f = ListingsFilter(snapshot_date=date(2025, 6, 15)) + assert f.to_params()["snapshot_date"] == "2025-06-15" + + +# --------------------------------------------------------------------------- +# include serialization +# --------------------------------------------------------------------------- + + +def test_include_serializes() -> None: + f = ListingsFilter(include=["price_history", "options"]) + assert f.to_params()["include"] == "price_history,options" + + +# --------------------------------------------------------------------------- +# Empty list filters are omitted from query params +# --------------------------------------------------------------------------- + + +def test_empty_list_omitted_from_params() -> None: + # Empty lists (not None) are falsy and skipped by the comma/pipe helpers, + # so they are omitted from the serialized query string. + f = ListingsFilter(make=[]) + assert "make" not in f.to_params() + + # --------------------------------------------------------------------------- # ListingsFilterBase is accessible directly (used by downstream callers) # --------------------------------------------------------------------------- diff --git a/tests/test_transport.py b/tests/test_transport.py index 7abe671..dfae23a 100644 --- a/tests/test_transport.py +++ b/tests/test_transport.py @@ -1,3 +1,6 @@ +from datetime import datetime, timedelta, timezone +from email.utils import format_datetime as email_format_datetime + import httpx import pytest import respx @@ -327,3 +330,122 @@ async def test_async_empty_body_fallback_message( await transport.aclose() assert exc_info.value.message + + +# --------------------------------------------------------------------------- +# Malformed JSON on 2xx raises VisorTransportError +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_async_malformed_success_json_raises_transport_error(): + with respx.mock(base_url=DEFAULT_BASE_URL) as mock: + mock.get("/listings").mock( + return_value=httpx.Response( + 200, + content=b"not-valid-json{{{", + headers={"Content-Type": "application/json"}, + ) + ) + transport = AsyncVisorTransport(api_key=API_KEY) + with pytest.raises(VisorTransportError): + await transport.get("/listings") + await transport.aclose() + + +def test_sync_malformed_success_json_raises_transport_error(): + with respx.mock(base_url=DEFAULT_BASE_URL) as mock: + mock.get("/listings").mock( + return_value=httpx.Response( + 200, + content=b"not-valid-json{{{", + headers={"Content-Type": "application/json"}, + ) + ) + transport = SyncVisorTransport(api_key=API_KEY) + with pytest.raises(VisorTransportError): + transport.get("/listings") + transport.close() + + +# --------------------------------------------------------------------------- +# Retry-After: HTTP-date and invalid-value coverage +# --------------------------------------------------------------------------- + + +def _future_http_date(seconds_ahead: int = 120) -> str: + """Return a valid RFC 7231 HTTP-date string for a moment in the future.""" + future = datetime.now(timezone.utc) + timedelta(seconds=seconds_ahead) + return email_format_datetime(future, usegmt=True) + + +def test_sync_rate_limit_retry_after_http_date(): + with respx.mock(base_url=DEFAULT_BASE_URL) as mock: + mock.get("/listings").mock( + return_value=httpx.Response( + 429, + headers={"Retry-After": _future_http_date(120)}, + json=ERROR_BODY, + ) + ) + transport = SyncVisorTransport(api_key=API_KEY) + with pytest.raises(RateLimitError) as exc_info: + transport.get("/listings") + transport.close() + + assert isinstance(exc_info.value.retry_after, int) + assert exc_info.value.retry_after >= 0 + + +def test_sync_rate_limit_retry_after_invalid_value(): + with respx.mock(base_url=DEFAULT_BASE_URL) as mock: + mock.get("/listings").mock( + return_value=httpx.Response( + 429, + headers={"Retry-After": "not-a-number-or-date"}, + json=ERROR_BODY, + ) + ) + transport = SyncVisorTransport(api_key=API_KEY) + with pytest.raises(RateLimitError) as exc_info: + transport.get("/listings") + transport.close() + + assert exc_info.value.retry_after is None + + +@pytest.mark.asyncio +async def test_async_rate_limit_retry_after_http_date(): + with respx.mock(base_url=DEFAULT_BASE_URL) as mock: + mock.get("/listings").mock( + return_value=httpx.Response( + 429, + headers={"Retry-After": _future_http_date(120)}, + json=ERROR_BODY, + ) + ) + transport = AsyncVisorTransport(api_key=API_KEY) + with pytest.raises(RateLimitError) as exc_info: + await transport.get("/listings") + await transport.aclose() + + assert isinstance(exc_info.value.retry_after, int) + assert exc_info.value.retry_after >= 0 + + +@pytest.mark.asyncio +async def test_async_rate_limit_retry_after_invalid_value(): + with respx.mock(base_url=DEFAULT_BASE_URL) as mock: + mock.get("/listings").mock( + return_value=httpx.Response( + 429, + headers={"Retry-After": "not-a-number-or-date"}, + json=ERROR_BODY, + ) + ) + transport = AsyncVisorTransport(api_key=API_KEY) + with pytest.raises(RateLimitError) as exc_info: + await transport.get("/listings") + await transport.aclose() + + assert exc_info.value.retry_after is None From 945ddc38618d76518c6ac4c99d1fc2e9d9c6485b Mon Sep 17 00:00:00 2001 From: Andrew Miller Date: Sat, 13 Jun 2026 12:46:30 -0500 Subject: [PATCH 2/3] Remove type: ignore[no-any-return] from transport success path Capture response.json() as Any, then cast to dict[str, Any] before returning, eliminating the suppression comment added in step 18. Co-Authored-By: Claude Sonnet 4.6 --- src/visor/_transport.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/visor/_transport.py b/src/visor/_transport.py index bda1c30..e7fcc5a 100644 --- a/src/visor/_transport.py +++ b/src/visor/_transport.py @@ -1,6 +1,6 @@ from datetime import datetime from email.utils import parsedate_to_datetime -from typing import Any +from typing import Any, cast import httpx @@ -34,9 +34,10 @@ def _parse_retry_after(value: str | None) -> int | None: def _handle_response(response: httpx.Response) -> dict[str, Any]: if response.is_success: try: - return response.json() # type: ignore[no-any-return] + data: Any = response.json() except ValueError as e: raise VisorTransportError(f"Received malformed JSON from API: {e}") from e + return cast(dict[str, Any], data) try: body = response.json() From 766207cbd1292e2cbf865d67e1a2c6127a1f4c58 Mon Sep 17 00:00:00 2001 From: Andrew Miller Date: Sat, 13 Jun 2026 13:07:44 -0500 Subject: [PATCH 3/3] Enforce dict response at runtime instead of cast() Replace cast(dict[str, Any], data) with an isinstance check that raises VisorTransportError for successful responses that are not JSON objects. Removes cast import. Adds sync and async tests for the new guard. Co-Authored-By: Claude Sonnet 4.6 --- src/visor/_transport.py | 6 ++++-- tests/test_transport.py | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/visor/_transport.py b/src/visor/_transport.py index e7fcc5a..ce92218 100644 --- a/src/visor/_transport.py +++ b/src/visor/_transport.py @@ -1,6 +1,6 @@ from datetime import datetime from email.utils import parsedate_to_datetime -from typing import Any, cast +from typing import Any import httpx @@ -37,7 +37,9 @@ def _handle_response(response: httpx.Response) -> dict[str, Any]: data: Any = response.json() except ValueError as e: raise VisorTransportError(f"Received malformed JSON from API: {e}") from e - return cast(dict[str, Any], data) + if not isinstance(data, dict): + raise VisorTransportError("Received non-object JSON from API") + return data try: body = response.json() diff --git a/tests/test_transport.py b/tests/test_transport.py index dfae23a..cceae26 100644 --- a/tests/test_transport.py +++ b/tests/test_transport.py @@ -368,6 +368,25 @@ def test_sync_malformed_success_json_raises_transport_error(): transport.close() +def test_sync_non_object_success_json_raises_transport_error(): + with respx.mock(base_url=DEFAULT_BASE_URL) as mock: + mock.get("/listings").mock(return_value=httpx.Response(200, json=[1, 2, 3])) + transport = SyncVisorTransport(api_key=API_KEY) + with pytest.raises(VisorTransportError): + transport.get("/listings") + transport.close() + + +@pytest.mark.asyncio +async def test_async_non_object_success_json_raises_transport_error(): + with respx.mock(base_url=DEFAULT_BASE_URL) as mock: + mock.get("/listings").mock(return_value=httpx.Response(200, json=[1, 2, 3])) + transport = AsyncVisorTransport(api_key=API_KEY) + with pytest.raises(VisorTransportError): + await transport.get("/listings") + await transport.aclose() + + # --------------------------------------------------------------------------- # Retry-After: HTTP-date and invalid-value coverage # ---------------------------------------------------------------------------