From 5b27d96230efe263bffc4f3c48f0e3aeed1093cf Mon Sep 17 00:00:00 2001 From: Jeff West Date: Sun, 17 May 2026 09:22:13 -0500 Subject: [PATCH 1/2] test(retry): cover Retry-After cap, HTTP-date fallback, and TimeoutException paths Backfills the regression test surface flagged by the Wave 5 coverage audit: - F-Q-01 (HIGH): assert Retry-After: 9999 clamps to retry_max_delay so a hostile/misconfigured server cannot park the SDK. Sync + async. - F-Q-02 (MED): assert HTTP-date Retry-After falls back to computed backoff and the transport still retries to success. Sync + async. - F-Q-03 (HIGH): assert httpx.TimeoutException retries on GET (idempotent) but raises immediately on POST (duplicate-order risk). Sync + async. All paths sleep through monkeypatched time.sleep / asyncio.sleep so the assertions check actual delays, not wall-clock timing. Closes #97 Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/test_async_client.py | 95 ++++++++++++++++++++++++++++++++++++++ tests/test_client.py | 78 +++++++++++++++++++++++++++++++ 2 files changed, 173 insertions(+) diff --git a/tests/test_async_client.py b/tests/test_async_client.py index 65b0c75..a2597d3 100644 --- a/tests/test_async_client.py +++ b/tests/test_async_client.py @@ -19,6 +19,7 @@ from kalshi.errors import ( AuthRequiredError, KalshiAuthError, + KalshiError, KalshiServerError, KalshiValidationError, ) @@ -203,6 +204,100 @@ async def test_successful_request( assert resp.status_code == 200 assert resp.json()["markets"][0]["ticker"] == "TEST" + # --- Regression: issue #97 ------------------------------------------- + # Async counterparts of F-Q-01 (cap), F-Q-02 (HTTP-date), and F-Q-03 + # (TimeoutException retry on GET, no retry on POST). + + @respx.mock + @pytest.mark.asyncio + async def test_429_retry_after_caps_at_retry_max_delay( + self, transport: AsyncTransport, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Async cap: Retry-After: 9999 must clamp to retry_max_delay=0.1.""" + sleeps: list[float] = [] + + async def fake_sleep(d: float) -> None: + sleeps.append(d) + + monkeypatch.setattr("asyncio.sleep", fake_sleep) + respx.get("https://test.kalshi.com/trade-api/v2/markets").mock( + side_effect=[ + httpx.Response(429, headers={"Retry-After": "9999"}, json={"message": "rl"}), + httpx.Response(200, json={"markets": []}), + ] + ) + resp = await transport.request("GET", "/markets") + assert resp.status_code == 200 + assert sleeps == [0.1], ( + f"Expected sleep clamped to retry_max_delay=0.1, got {sleeps!r}" + ) + + @respx.mock + @pytest.mark.asyncio + async def test_429_retry_after_http_date_falls_back_to_backoff( + self, transport: AsyncTransport, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Async: HTTP-date unparseable; transport retries via computed backoff.""" + sleeps: list[float] = [] + + async def fake_sleep(d: float) -> None: + sleeps.append(d) + + monkeypatch.setattr("asyncio.sleep", fake_sleep) + route = respx.get("https://test.kalshi.com/trade-api/v2/markets").mock( + side_effect=[ + httpx.Response( + 429, + headers={"Retry-After": "Wed, 21 Oct 2026 07:28:00 GMT"}, + json={"message": "rl"}, + ), + httpx.Response(200, json={"markets": []}), + ] + ) + resp = await transport.request("GET", "/markets") + assert resp.status_code == 200 + assert route.call_count == 2 + assert len(sleeps) == 1 + assert 0.0 <= sleeps[0] <= 0.01, ( + f"Expected backoff sleep in [0, 0.01], got {sleeps[0]!r}" + ) + + @respx.mock + @pytest.mark.asyncio + async def test_get_retries_on_timeout( + self, transport: AsyncTransport, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Async GET retries on httpx.TimeoutException.""" + + async def fake_sleep(d: float) -> None: + return None + + monkeypatch.setattr("asyncio.sleep", fake_sleep) + route = respx.get("https://test.kalshi.com/trade-api/v2/markets").mock( + side_effect=[ + httpx.TimeoutException("read timed out"), + httpx.Response(200, json={"markets": []}), + ] + ) + resp = await transport.request("GET", "/markets") + assert resp.status_code == 200 + assert route.call_count == 2 + + @respx.mock + @pytest.mark.asyncio + async def test_post_not_retried_on_timeout( + self, transport: AsyncTransport + ) -> None: + """Async POST timeout raises immediately; no retry.""" + route = respx.post( + "https://test.kalshi.com/trade-api/v2/portfolio/orders" + ).mock(side_effect=httpx.TimeoutException("read timed out")) + with pytest.raises(KalshiError, match="timed out"): + await transport.request( + "POST", "/portfolio/orders", json={"ticker": "T"} + ) + assert route.call_count == 1 + class TestAsyncTransportContextManager: @pytest.mark.asyncio diff --git a/tests/test_client.py b/tests/test_client.py index 49f3995..c42f64e 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -277,6 +277,84 @@ def test_successful_request(self, transport: SyncTransport) -> None: assert resp.status_code == 200 assert resp.json()["markets"][0]["ticker"] == "TEST" + # --- Regression: issue #97 ------------------------------------------- + # F-Q-01: Retry-After must be capped at retry_max_delay so a hostile + # server can't park the SDK for 9999s. F-Q-02: HTTP-date Retry-After + # falls back to computed backoff and the transport still retries. + # F-Q-03: httpx.TimeoutException retries only on idempotent methods. + + @respx.mock + def test_429_retry_after_caps_at_retry_max_delay( + self, transport: SyncTransport, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Hostile/misconfigured Retry-After is clamped to retry_max_delay.""" + sleeps: list[float] = [] + monkeypatch.setattr("kalshi._base_client.time.sleep", lambda d: sleeps.append(d)) + respx.get("https://test.kalshi.com/trade-api/v2/markets").mock( + side_effect=[ + httpx.Response(429, headers={"Retry-After": "9999"}, json={"message": "rl"}), + httpx.Response(200, json={"markets": []}), + ] + ) + resp = transport.request("GET", "/markets") + assert resp.status_code == 200 + # config fixture: retry_max_delay=0.1 — must clamp to that, NOT 9999. + assert sleeps == [0.1], ( + f"Expected sleep clamped to retry_max_delay=0.1, got {sleeps!r}" + ) + + @respx.mock + def test_429_retry_after_http_date_falls_back_to_backoff( + self, transport: SyncTransport, monkeypatch: pytest.MonkeyPatch + ) -> None: + """HTTP-date Retry-After unparseable; transport still retries via backoff.""" + sleeps: list[float] = [] + monkeypatch.setattr("kalshi._base_client.time.sleep", lambda d: sleeps.append(d)) + route = respx.get("https://test.kalshi.com/trade-api/v2/markets").mock( + side_effect=[ + httpx.Response( + 429, + headers={"Retry-After": "Wed, 21 Oct 2026 07:28:00 GMT"}, + json={"message": "rl"}, + ), + httpx.Response(200, json={"markets": []}), + ] + ) + resp = transport.request("GET", "/markets") + assert resp.status_code == 200 + assert route.call_count == 2 + # Backoff is Full Jitter: uniform(0, min(base*2**0, max)) = uniform(0, 0.01). + assert len(sleeps) == 1 + assert 0.0 <= sleeps[0] <= 0.01, ( + f"Expected backoff sleep in [0, 0.01], got {sleeps[0]!r}" + ) + + @respx.mock + def test_get_retries_on_timeout( + self, transport: SyncTransport, monkeypatch: pytest.MonkeyPatch + ) -> None: + """GET retries on httpx.TimeoutException (idempotent method).""" + monkeypatch.setattr("kalshi._base_client.time.sleep", lambda d: None) + route = respx.get("https://test.kalshi.com/trade-api/v2/markets").mock( + side_effect=[ + httpx.TimeoutException("read timed out"), + httpx.Response(200, json={"markets": []}), + ] + ) + resp = transport.request("GET", "/markets") + assert resp.status_code == 200 + assert route.call_count == 2 + + @respx.mock + def test_post_not_retried_on_timeout(self, transport: SyncTransport) -> None: + """POST timeout raises immediately; no retry (duplicate-order risk).""" + route = respx.post( + "https://test.kalshi.com/trade-api/v2/portfolio/orders" + ).mock(side_effect=httpx.TimeoutException("read timed out")) + with pytest.raises(KalshiError, match="timed out"): + transport.request("POST", "/portfolio/orders", json={"ticker": "T"}) + assert route.call_count == 1 + class TestSyncTransportContextManager: def test_close(self, test_auth: KalshiAuth, config: KalshiConfig) -> None: From 5aeb539f717c3be616a14e80edab4bb7778b2786 Mon Sep 17 00:00:00 2001 From: Jeff West Date: Sun, 17 May 2026 10:08:04 -0500 Subject: [PATCH 2/2] review(#133): assert sleep called in timeout-retry tests + drop F-Q comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per bot review on #133: - test_get_retries_on_timeout (sync + async) now captures sleeps via monkeypatch and asserts len(sleeps) == 1. Without this, removing the time.sleep(delay) / asyncio.sleep(delay) line in the timeout-retry path would leave the test green. - Dropped block-comment header that referenced issue #97 + F-Q codes (CLAUDE.md §3: don't reference current task in code, that's PR-body territory; F-Q codes aren't defined in the codebase). Skipped: - F-Q-02 magic-number explanation comment (bot called it nice-to-have; the docstring already names the formula). - Redundant "return None" in fake_sleep — replaced by sleeps.append(d) anyway. uv run pytest tests/test_client.py tests/test_async_client.py -k retry: 36 passed. ruff clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/test_async_client.py | 11 ++++++----- tests/test_client.py | 13 ++++++------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/test_async_client.py b/tests/test_async_client.py index a2597d3..6778c66 100644 --- a/tests/test_async_client.py +++ b/tests/test_async_client.py @@ -204,10 +204,6 @@ async def test_successful_request( assert resp.status_code == 200 assert resp.json()["markets"][0]["ticker"] == "TEST" - # --- Regression: issue #97 ------------------------------------------- - # Async counterparts of F-Q-01 (cap), F-Q-02 (HTTP-date), and F-Q-03 - # (TimeoutException retry on GET, no retry on POST). - @respx.mock @pytest.mark.asyncio async def test_429_retry_after_caps_at_retry_max_delay( @@ -268,9 +264,10 @@ async def test_get_retries_on_timeout( self, transport: AsyncTransport, monkeypatch: pytest.MonkeyPatch ) -> None: """Async GET retries on httpx.TimeoutException.""" + sleeps: list[float] = [] async def fake_sleep(d: float) -> None: - return None + sleeps.append(d) monkeypatch.setattr("asyncio.sleep", fake_sleep) route = respx.get("https://test.kalshi.com/trade-api/v2/markets").mock( @@ -282,6 +279,10 @@ async def fake_sleep(d: float) -> None: resp = await transport.request("GET", "/markets") assert resp.status_code == 200 assert route.call_count == 2 + # Confirm a backoff delay happened between attempts — without this + # assertion, removing the asyncio.sleep(delay) line would still pass. + assert len(sleeps) == 1 + assert sleeps[0] >= 0.0 @respx.mock @pytest.mark.asyncio diff --git a/tests/test_client.py b/tests/test_client.py index c42f64e..9657280 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -277,12 +277,6 @@ def test_successful_request(self, transport: SyncTransport) -> None: assert resp.status_code == 200 assert resp.json()["markets"][0]["ticker"] == "TEST" - # --- Regression: issue #97 ------------------------------------------- - # F-Q-01: Retry-After must be capped at retry_max_delay so a hostile - # server can't park the SDK for 9999s. F-Q-02: HTTP-date Retry-After - # falls back to computed backoff and the transport still retries. - # F-Q-03: httpx.TimeoutException retries only on idempotent methods. - @respx.mock def test_429_retry_after_caps_at_retry_max_delay( self, transport: SyncTransport, monkeypatch: pytest.MonkeyPatch @@ -334,7 +328,8 @@ def test_get_retries_on_timeout( self, transport: SyncTransport, monkeypatch: pytest.MonkeyPatch ) -> None: """GET retries on httpx.TimeoutException (idempotent method).""" - monkeypatch.setattr("kalshi._base_client.time.sleep", lambda d: None) + sleeps: list[float] = [] + monkeypatch.setattr("kalshi._base_client.time.sleep", lambda d: sleeps.append(d)) route = respx.get("https://test.kalshi.com/trade-api/v2/markets").mock( side_effect=[ httpx.TimeoutException("read timed out"), @@ -344,6 +339,10 @@ def test_get_retries_on_timeout( resp = transport.request("GET", "/markets") assert resp.status_code == 200 assert route.call_count == 2 + # Confirm a backoff delay happened between attempts — without this + # assertion, removing the time.sleep(delay) line would still pass. + assert len(sleeps) == 1 + assert sleeps[0] >= 0.0 @respx.mock def test_post_not_retried_on_timeout(self, transport: SyncTransport) -> None: