Skip to content

Commit a6a8fe6

Browse files
TexasCodingclaude
andcommitted
fix(retry): honor Retry-After: 0 end-to-end through transport
Per second-pass bot review on #112: the prior commit kept Retry-After: 0 on the error object, but transport's "if error.retry_after:" check dropped 0 as falsy and fell through to backoff. Switched to "is not None" in both sync and async transport. Added regression tests that monkeypatch time.sleep / asyncio.sleep and assert the delay is exactly 0.0 (not the backoff fallback). Also addressed: - config._validate_url docstring trimmed to one line (CLAUDE.md). - Called via KalshiConfig._validate_url(...) rather than self. (staticmethod-through-self hides that no instance state is used). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent f18a768 commit a6a8fe6

4 files changed

Lines changed: 59 additions & 16 deletions

File tree

kalshi/_base_client.py

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -165,8 +165,12 @@ def request(
165165
if not should_retry:
166166
raise error
167167

168-
# Use Retry-After header if available for 429
169-
if isinstance(error, KalshiRateLimitError) and error.retry_after:
168+
# Use Retry-After header if available for 429.
169+
# `is not None` — not truthy — so Retry-After: 0 ("retry immediately") is honored.
170+
if (
171+
isinstance(error, KalshiRateLimitError)
172+
and error.retry_after is not None
173+
):
170174
delay = min(error.retry_after, self._config.retry_max_delay)
171175
else:
172176
delay = _compute_backoff(attempt, self._config)
@@ -279,7 +283,11 @@ async def request(
279283
if not should_retry:
280284
raise error
281285

282-
if isinstance(error, KalshiRateLimitError) and error.retry_after:
286+
# `is not None` so Retry-After: 0 ("retry immediately") is honored.
287+
if (
288+
isinstance(error, KalshiRateLimitError)
289+
and error.retry_after is not None
290+
):
283291
delay = min(error.retry_after, self._config.retry_max_delay)
284292
else:
285293
delay = _compute_backoff(attempt, self._config)

kalshi/config.py

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -51,22 +51,12 @@ def __post_init__(self) -> None:
5151
object.__setattr__(self, "base_url", self.base_url.rstrip("/"))
5252
if self.ws_base_url.endswith("/"):
5353
object.__setattr__(self, "ws_base_url", self.ws_base_url.rstrip("/"))
54-
self._validate_url(self.base_url, "base_url", secure="https", plaintext="http")
55-
self._validate_url(self.ws_base_url, "ws_base_url", secure="wss", plaintext="ws")
54+
KalshiConfig._validate_url(self.base_url, "base_url", secure="https", plaintext="http")
55+
KalshiConfig._validate_url(self.ws_base_url, "ws_base_url", secure="wss", plaintext="ws")
5656

5757
@staticmethod
5858
def _validate_url(url: str, field_name: str, *, secure: str, plaintext: str) -> None:
59-
"""Reject URLs that would leak API credentials.
60-
61-
An attacker who can write to the process environment (``docker run
62-
-e``, CI variable, shell history) can otherwise redirect signed
63-
requests to an arbitrary host. Enforce the secure scheme for remote
64-
hosts; warn when the host isn't a known Kalshi endpoint so misroutes
65-
surface in logs.
66-
67-
The plaintext scheme is permitted only for loopback hosts (localhost,
68-
127.0.0.1, ::1) so local mock servers and tests still work.
69-
"""
59+
"""Reject URLs that would expose credentials (bad scheme or plaintext-to-remote)."""
7060
parsed = urlparse(url)
7161
scheme = parsed.scheme.lower()
7262
host = (parsed.hostname or "").lower()

tests/test_async_client.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,31 @@ async def test_get_retries_on_429(self, transport: AsyncTransport) -> None:
7171
assert resp.status_code == 200
7272
assert route.call_count == 2
7373

74+
@respx.mock
75+
@pytest.mark.asyncio
76+
async def test_429_retry_after_zero_is_honored_end_to_end(
77+
self, transport: AsyncTransport, monkeypatch: pytest.MonkeyPatch
78+
) -> None:
79+
# Async counterpart of the sync regression: `is not None` keeps Retry-After: 0.
80+
sleeps: list[float] = []
81+
82+
async def fake_sleep(d: float) -> None:
83+
sleeps.append(d)
84+
85+
# _base_client imports asyncio inside the method, so patch the module attr directly.
86+
monkeypatch.setattr("asyncio.sleep", fake_sleep)
87+
respx.get("https://test.kalshi.com/trade-api/v2/markets").mock(
88+
side_effect=[
89+
httpx.Response(429, headers={"Retry-After": "0"}, json={"message": "rl"}),
90+
httpx.Response(200, json={"markets": []}),
91+
]
92+
)
93+
resp = await transport.request("GET", "/markets")
94+
assert resp.status_code == 200
95+
assert sleeps == [0.0], (
96+
f"Expected one sleep of 0.0s honoring Retry-After: 0, got {sleeps!r}"
97+
)
98+
7499
@respx.mock
75100
@pytest.mark.asyncio
76101
async def test_get_retries_on_500(self, transport: AsyncTransport) -> None:

tests/test_client.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,26 @@ def test_get_retries_on_429(self, transport: SyncTransport) -> None:
180180
assert resp.status_code == 200
181181
assert route.call_count == 2
182182

183+
@respx.mock
184+
def test_429_retry_after_zero_is_honored_end_to_end(
185+
self, transport: SyncTransport, monkeypatch: pytest.MonkeyPatch
186+
) -> None:
187+
# Regression: `if error.retry_after:` would drop 0 ("retry immediately") as falsy.
188+
# `is not None` keeps it; assert the transport actually sleeps 0, not the backoff fallback.
189+
sleeps: list[float] = []
190+
monkeypatch.setattr("kalshi._base_client.time.sleep", lambda d: sleeps.append(d))
191+
respx.get("https://test.kalshi.com/trade-api/v2/markets").mock(
192+
side_effect=[
193+
httpx.Response(429, headers={"Retry-After": "0"}, json={"message": "rl"}),
194+
httpx.Response(200, json={"markets": []}),
195+
]
196+
)
197+
resp = transport.request("GET", "/markets")
198+
assert resp.status_code == 200
199+
assert sleeps == [0.0], (
200+
f"Expected one sleep of 0.0s honoring Retry-After: 0, got {sleeps!r}"
201+
)
202+
183203
@respx.mock
184204
def test_get_retries_on_500(self, transport: SyncTransport) -> None:
185205
"""500s are transient on GETs; demo routinely returns them mid-paginate."""

0 commit comments

Comments
 (0)