From 9d967852968f6394f22c0a863ec0d8902b008065 Mon Sep 17 00:00:00 2001 From: Jeff West Date: Thu, 21 May 2026 07:04:22 -0500 Subject: [PATCH 1/3] fix(errors): preserve typed exception class when body is suppressed When a response advertises >MAX_ERROR_BODY_BYTES via Content-Length, _map_error previously returned a generic KalshiError, bypassing the status dispatch. This silently broke: - The retry loop's isinstance(err, KalshiRateLimitError) Retry-After branch (a hostile 429 with a verbose body fell back to computed backoff, ignoring the server's hint). - Caller code structured as except KalshiRateLimitError / KalshiAuthError / KalshiConflictError, which silently missed these responses. Fix: when the body is suppressed, set body={} and build a suppressed-body message, then fall through to the existing status dispatch so the typed class is preserved. The 429 Retry-After header is read from response.headers (not the body), so it still populates correctly. Closes #252 --- kalshi/_base_client.py | 28 ++++++++++++++++------------ tests/test_client.py | 31 ++++++++++++++++++++++++++++++- 2 files changed, 46 insertions(+), 13 deletions(-) diff --git a/kalshi/_base_client.py b/kalshi/_base_client.py index 38b254a9..d51bdc93 100644 --- a/kalshi/_base_client.py +++ b/kalshi/_base_client.py @@ -49,26 +49,30 @@ def _map_error(response: httpx.Response) -> KalshiError: status = response.status_code # #203: cap body buffering. If the server advertises an oversized body, - # short-circuit before httpx materialises it into memory. + # short-circuit before httpx materialises it into memory. We still fall + # through to status dispatch so the typed exception class is preserved + # (#252) — only the message changes. content_length = response.headers.get("content-length") + suppressed: bool = False if content_length: try: if int(content_length) > MAX_ERROR_BODY_BYTES: - return KalshiError( - message=f"HTTP {status} (body {content_length} bytes, suppressed)", - status_code=status, - ) + suppressed = True except ValueError: pass # malformed Content-Length — fall through to normal parse - try: - body = response.json() - except Exception: + body: dict[str, Any] + if suppressed: body = {} - - raw_message = body.get("message") or body.get("error") or response.text or f"HTTP {status}" - # Truncate to bound log-line and exception-message size. - message = str(raw_message)[:MAX_ERROR_MESSAGE_CHARS] + message = f"HTTP {status} (body {content_length} bytes, suppressed)" + else: + try: + body = response.json() + except Exception: + body = {} + raw_message = body.get("message") or body.get("error") or response.text or f"HTTP {status}" + # Truncate to bound log-line and exception-message size. + message = str(raw_message)[:MAX_ERROR_MESSAGE_CHARS] if status in (400, 422): details = body.get("details") or body.get("errors") diff --git a/tests/test_client.py b/tests/test_client.py index 17982d07..cfcfb5fd 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -987,7 +987,6 @@ def test_from_env_typed_kwargs_forwarded(self, monkeypatch: pytest.MonkeyPatch) assert client._config.max_retries == 11 client.close() - class TestWidenedRetrySet: """#192: Cloudflare 5xx (520-524) + 408 are retryable on safe methods only.""" @@ -1062,6 +1061,36 @@ def test_oversized_error_body_truncated_by_content_length_header(self) -> None: err = _map_error(resp) assert "suppressed" in str(err) assert "50000" in str(err) + # #252: typed exception class preserved even when body is suppressed. + assert isinstance(err, KalshiServerError) + + def test_oversized_body_429_preserves_rate_limit_with_retry_after(self) -> None: + # #252: a hostile 429 with verbose body must still surface as + # KalshiRateLimitError and the Retry-After header (read separately + # from the body) must still populate. + resp = httpx.Response( + 429, + headers={"content-length": "50000", "Retry-After": "2.5"}, + text="x", + ) + err = _map_error(resp) + assert isinstance(err, KalshiRateLimitError) + assert err.retry_after == 2.5 + assert "suppressed" in str(err) + + def test_oversized_body_401_preserves_auth_error(self) -> None: + # #252 + resp = httpx.Response(401, headers={"content-length": "20000"}, text="x") + err = _map_error(resp) + assert isinstance(err, KalshiAuthError) + assert "suppressed" in str(err) + + def test_oversized_body_409_preserves_conflict_error(self) -> None: + # #252 + resp = httpx.Response(409, headers={"content-length": "20000"}, text="x") + err = _map_error(resp) + assert isinstance(err, KalshiConflictError) + assert "suppressed" in str(err) def test_error_message_truncated_to_1024_chars(self) -> None: long_msg = "A" * 5000 From 49012681f0008144c95c41687175fe3d6983d0f1 Mon Sep 17 00:00:00 2001 From: Jeff West Date: Thu, 21 May 2026 07:05:04 -0500 Subject: [PATCH 2/3] fix(errors): map 408 and 504 to KalshiTimeoutError KalshiTimeoutError (#226) exposes the 'may have committed' semantic so callers can branch on it (e.g., query by client_order_id before retrying an order create). 408 Request Timeout from an intermediary/CDN and 504 Gateway Timeout from upstream carry the same semantic, but routed to generic KalshiError and KalshiServerError respectively. Callers writing 'except KalshiTimeoutError' silently missed the HTTP-status variant. Fix: route 408 and 504 to KalshiTimeoutError before the >=500 catch-all. 500/502/503 continue to route to KalshiServerError. Closes #251 --- kalshi/_base_client.py | 6 ++++++ tests/test_client.py | 24 ++++++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/kalshi/_base_client.py b/kalshi/_base_client.py index d51bdc93..5f31ba6d 100644 --- a/kalshi/_base_client.py +++ b/kalshi/_base_client.py @@ -120,6 +120,12 @@ def _map_error(response: httpx.Response) -> KalshiError: return KalshiRateLimitError( message=message, status_code=status, retry_after=retry_after_val ) + # #251: 408 Request Timeout and 504 Gateway Timeout carry the same + # "may have committed" semantic as a transport-level timeout. Route them + # to KalshiTimeoutError so callers can branch on it (e.g., reconcile via + # client_order_id before retrying an order create). + if status in (408, 504): + return KalshiTimeoutError(message=message, status_code=status) if status >= 500: return KalshiServerError(message=message, status_code=status) diff --git a/tests/test_client.py b/tests/test_client.py index cfcfb5fd..b19d7fed 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -97,6 +97,30 @@ def test_503_server_error(self) -> None: err = _map_error(resp) assert isinstance(err, KalshiServerError) + def test_408_request_timeout_maps_to_timeout(self) -> None: + # #251: 408 carries "may have committed" semantic — route to + # KalshiTimeoutError, not generic KalshiError. + resp = httpx.Response(408, json={"message": "request timeout"}) + err = _map_error(resp) + assert isinstance(err, KalshiTimeoutError) + assert err.status_code == 408 + + def test_504_gateway_timeout_maps_to_timeout(self) -> None: + # #251: 504 carries same semantic as transport timeout — must route to + # KalshiTimeoutError, not KalshiServerError. + resp = httpx.Response(504, json={"message": "gateway timeout"}) + err = _map_error(resp) + assert isinstance(err, KalshiTimeoutError) + assert err.status_code == 504 + + def test_500_still_server_error(self) -> None: + # #251: regression — 500/502/503 must continue to route to KalshiServerError. + for status in (500, 502, 503): + resp = httpx.Response(status, json={"message": "boom"}) + err = _map_error(resp) + assert isinstance(err, KalshiServerError), f"status={status}" + assert not isinstance(err, KalshiTimeoutError), f"status={status}" + def test_validation_error_with_details(self) -> None: resp = httpx.Response( 400, json={"message": "validation failed", "details": {"ticker": "required"}} From 72dba04f644c249b43a0152ae10bd58bad1e9a00 Mon Sep 17 00:00:00 2001 From: Jeff West Date: Thu, 21 May 2026 07:06:29 -0500 Subject: [PATCH 3/3] perf(transport): hoist invariant header merge out of retry loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The merge of config_extra / per_call_extra / body_headers is loop-invariant — only auth_headers changes per attempt (fresh timestamp). Previously a fresh 4-way dict was allocated on every attempt, including the common no-retry hot path of every resource method. Build the invariant base once before the loop; the per-attempt merge is now a 2-way dict literal over a single fresh auth_headers map. Applied to both SyncTransport.request and AsyncTransport.request. Behavior unchanged: layer order (config < per-call < body-helper < auth) and final-key wins semantics are preserved. Existing transport tests pass. Closes #262 --- kalshi/_base_client.py | 41 +++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/kalshi/_base_client.py b/kalshi/_base_client.py index 5f31ba6d..57b91211 100644 --- a/kalshi/_base_client.py +++ b/kalshi/_base_client.py @@ -213,20 +213,20 @@ def request( # #193: wall-clock budget across the whole request including retries. start = time.monotonic() total_timeout = self._config.total_timeout - config_extra = self._config.extra_headers or {} - per_call_extra = extra_headers or {} - body_headers = headers or {} + # #262: header layers config_extra/per_call_extra/body_headers are + # loop-invariant; only auth_headers changes per attempt (fresh + # timestamp). Merge the invariant base once outside the loop. + # Order matters: config defaults < per-request overrides + # < body-helper headers < signed auth. + base_headers: dict[str, str] = { + **(self._config.extra_headers or {}), + **(extra_headers or {}), + **(headers or {}), + } for attempt in range(self._config.max_retries + 1): auth_headers = self._auth.sign_request(method.upper(), sign_path) if self._auth else {} - # Order matters: config defaults < per-request overrides - # < body-helper headers < signed auth. - merged_headers = { - **config_extra, - **per_call_extra, - **body_headers, - **auth_headers, - } + merged_headers = {**base_headers, **auth_headers} logger.debug( "Request: %s %s (attempt %d/%d)", @@ -413,21 +413,22 @@ async def request( # #193: wall-clock budget across the whole request including retries. start = time.monotonic() total_timeout = self._config.total_timeout - config_extra = self._config.extra_headers or {} - per_call_extra = extra_headers or {} - body_headers = headers or {} + # #262: see SyncTransport.request — hoist invariant header layers + # out of the retry loop; only auth_headers changes per attempt. + # Order matters: config defaults < per-request overrides + # < body-helper headers < signed auth. + base_headers: dict[str, str] = { + **(self._config.extra_headers or {}), + **(extra_headers or {}), + **(headers or {}), + } for attempt in range(self._config.max_retries + 1): if self._auth: auth_headers = await self._auth.sign_request_async(method.upper(), sign_path) else: auth_headers = {} - merged_headers = { - **config_extra, - **per_call_extra, - **body_headers, - **auth_headers, - } + merged_headers = {**base_headers, **auth_headers} logger.debug( "Async request: %s %s (attempt %d/%d)",