From b8028ae8855a3d622a0bcb866b7f535b658b9a7d Mon Sep 17 00:00:00 2001 From: audit-w1a Date: Wed, 20 May 2026 21:11:59 -0500 Subject: [PATCH] feat(transport): widen retry codes; typed timeouts; bounded errors; base_url validation; close ownership - Retry: widen RETRYABLE_STATUS_CODES to include 408/425/520-524 (Cloudflare). POST/DELETE still never retry. - KalshiConfig.total_timeout: wall-clock deadline (None = legacy unbounded). - _map_error: 422 -> KalshiValidationError, 409 -> KalshiConflictError. Body capped at 16KB via Content-Length; message truncated to 1024 chars. - httpx.PoolTimeout -> KalshiPoolExhaustedError (always retryable, even POST); httpx.TimeoutException -> KalshiTimeoutError. - KalshiConfig.__post_init__ validates base_url path == /trade-api/v2 (raises ValueError otherwise). - KalshiClient/AsyncKalshiClient.close() only shuts down KalshiAuth when client owns it (auth was constructed locally, not passed via auth= kwarg). Closes #192, Closes #193, Closes #201, Closes #202, Closes #203, Closes #204, Closes #210 Refs #224 --- kalshi/_base_client.py | 115 ++++++++++++++-- kalshi/async_client.py | 10 +- kalshi/client.py | 10 +- kalshi/config.py | 13 ++ tests/test_async_client.py | 249 ++++++++++++++++++++++++++++++++- tests/test_client.py | 272 ++++++++++++++++++++++++++++++++++++- tests/test_config.py | 34 +++++ 7 files changed, 683 insertions(+), 20 deletions(-) diff --git a/kalshi/_base_client.py b/kalshi/_base_client.py index 106a58d3..030fdb62 100644 --- a/kalshi/_base_client.py +++ b/kalshi/_base_client.py @@ -19,41 +19,67 @@ from kalshi.config import KalshiConfig from kalshi.errors import ( KalshiAuthError, + KalshiConflictError, KalshiError, KalshiNotFoundError, + KalshiPoolExhaustedError, KalshiRateLimitError, KalshiServerError, + KalshiTimeoutError, KalshiValidationError, ) logger = logging.getLogger("kalshi") -RETRYABLE_STATUS_CODES = {429, 500, 502, 503, 504} +RETRYABLE_STATUS_CODES = {408, 425, 429, 500, 502, 503, 504, 520, 521, 522, 523, 524} # DELETE excluded: cancel/batch_cancel are not safely idempotent RETRYABLE_METHODS = {"GET", "HEAD", "OPTIONS"} +# _map_error body-size guards: refuse to parse and never let one error +# message dominate a log line / Sentry payload. +MAX_ERROR_BODY_BYTES = 16 * 1024 +MAX_ERROR_MESSAGE_CHARS = 1024 + def _map_error(response: httpx.Response) -> KalshiError: """Map an HTTP error response to the appropriate SDK exception.""" status = response.status_code + + # #203: cap body buffering. If the server advertises an oversized body, + # short-circuit before httpx materialises it into memory. + content_length = response.headers.get("content-length") + 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, + ) + except ValueError: + pass # malformed Content-Length — fall through to normal parse + try: body = response.json() except Exception: body = {} - message = body.get("message") or body.get("error") or response.text or f"HTTP {status}" + 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 == 400: + if status in (400, 422): details = body.get("details") or body.get("errors") return KalshiValidationError( - message=str(message), + message=message, status_code=status, details=details if isinstance(details, dict) else None, ) if status in (401, 403): - return KalshiAuthError(message=str(message), status_code=status) + return KalshiAuthError(message=message, status_code=status) if status == 404: - return KalshiNotFoundError(message=str(message), status_code=status) + return KalshiNotFoundError(message=message, status_code=status) + if status == 409: + return KalshiConflictError(message=message, status_code=status) if status == 429: retry_after = response.headers.get("Retry-After") retry_after_val: float | None = None @@ -66,12 +92,12 @@ def _map_error(response: httpx.Response) -> KalshiError: except ValueError: retry_after_val = None # HTTP-date format, fall back to computed backoff return KalshiRateLimitError( - message=str(message), status_code=status, retry_after=retry_after_val + message=message, status_code=status, retry_after=retry_after_val ) if status >= 500: - return KalshiServerError(message=str(message), status_code=status) + return KalshiServerError(message=message, status_code=status) - return KalshiError(message=str(message), status_code=status) + return KalshiError(message=message, status_code=status) def _compute_backoff(attempt: int, config: KalshiConfig) -> float: @@ -87,6 +113,13 @@ def _compute_backoff(attempt: int, config: KalshiConfig) -> float: return float(random.uniform(0, capped)) +def _would_exceed_budget(start: float, delay: float, total_timeout: float | None) -> bool: + """#193: True iff sleeping ``delay`` now would push past ``total_timeout``.""" + if total_timeout is None: + return False + return (time.monotonic() - start) + delay > total_timeout + + class SyncTransport: """Synchronous HTTP transport using httpx.Client.""" @@ -129,6 +162,9 @@ def request( # Sign with path-only (not full URL). Kalshi expects: /trade-api/v2/endpoint sign_path = self._base_path + path last_error: KalshiError | None = None + # #193: wall-clock budget across the whole request including retries. + start = time.monotonic() + total_timeout = self._config.total_timeout for attempt in range(self._config.max_retries + 1): auth_headers = self._auth.sign_request(method.upper(), sign_path) if self._auth else {} @@ -149,14 +185,38 @@ def request( json=json, headers=auth_headers, ) + except httpx.PoolTimeout as e: + # #204: pool exhaustion never reached the wire — safe to retry + # even for POST/DELETE. + last_error = KalshiPoolExhaustedError( + f"Connection pool exhausted on {method.upper()} {path}. " + f"Raise KalshiConfig.limits.max_connections." + ) + if attempt < self._config.max_retries: + delay = _compute_backoff(attempt, self._config) + if _would_exceed_budget(start, delay, total_timeout): + raise last_error from None + logger.warning( + "Pool exhausted on %s %s, retrying in %.1fs", + method, + path, + delay, + ) + time.sleep(delay) + continue + raise last_error from e except httpx.TimeoutException as e: # F-O-09: don't interpolate httpx exception strings — they # include the full URL with query string and can leak # token-like values into uncaught-exception sinks. The # underlying exception is preserved via `__cause__`. - last_error = KalshiError(f"Request timed out: {method.upper()} {path}") + last_error = KalshiTimeoutError( + f"Request timed out: {method.upper()} {path}" + ) if method.upper() in RETRYABLE_METHODS and attempt < self._config.max_retries: delay = _compute_backoff(attempt, self._config) + if _would_exceed_budget(start, delay, total_timeout): + raise last_error from None logger.warning("Timeout on %s %s, retrying in %.1fs", method, path, delay) time.sleep(delay) continue @@ -192,6 +252,9 @@ def request( else: delay = _compute_backoff(attempt, self._config) + if _would_exceed_budget(start, delay, total_timeout): + raise error from None + logger.warning( "%s %s returned %d, retrying in %.1fs (attempt %d/%d)", method.upper(), @@ -256,6 +319,9 @@ async def request( # Sign with path-only (not full URL). Kalshi expects: /trade-api/v2/endpoint sign_path = self._base_path + path last_error: KalshiError | None = None + # #193: wall-clock budget across the whole request including retries. + start = time.monotonic() + total_timeout = self._config.total_timeout for attempt in range(self._config.max_retries + 1): if self._auth: @@ -279,11 +345,35 @@ async def request( json=json, headers=auth_headers, ) + except httpx.PoolTimeout as e: + # #204: pool exhaustion never reached the wire — safe to retry + # even for POST/DELETE. + last_error = KalshiPoolExhaustedError( + f"Connection pool exhausted on {method.upper()} {path}. " + f"Raise KalshiConfig.limits.max_connections." + ) + if attempt < self._config.max_retries: + delay = _compute_backoff(attempt, self._config) + if _would_exceed_budget(start, delay, total_timeout): + raise last_error from None + logger.warning( + "Pool exhausted on %s %s, retrying in %.1fs", + method, + path, + delay, + ) + await asyncio.sleep(delay) + continue + raise last_error from e except httpx.TimeoutException as e: # F-O-09: see sync transport above. - last_error = KalshiError(f"Request timed out: {method.upper()} {path}") + last_error = KalshiTimeoutError( + f"Request timed out: {method.upper()} {path}" + ) if method.upper() in RETRYABLE_METHODS and attempt < self._config.max_retries: delay = _compute_backoff(attempt, self._config) + if _would_exceed_budget(start, delay, total_timeout): + raise last_error from None logger.warning("Timeout on %s %s, retrying in %.1fs", method, path, delay) await asyncio.sleep(delay) continue @@ -319,6 +409,9 @@ async def request( else: delay = _compute_backoff(attempt, self._config) + if _would_exceed_budget(start, delay, total_timeout): + raise error from None + logger.warning( "%s %s returned %d, retrying in %.1fs (attempt %d/%d)", method.upper(), diff --git a/kalshi/async_client.py b/kalshi/async_client.py index 976510e8..c9a1ddf8 100644 --- a/kalshi/async_client.py +++ b/kalshi/async_client.py @@ -65,6 +65,9 @@ def __init__( # Reject empty strings that look like misconfigured credentials if key_id is not None and not key_id.strip(): raise ValueError("key_id must not be empty. Omit it for unauthenticated access.") + # #210: only shut down auth on close() when WE built it. If the + # caller passed in their own KalshiAuth, they keep ownership. + self._auth_owned: bool = auth is None self._auth: KalshiAuth | None if auth is not None: self._auth = auth @@ -166,12 +169,15 @@ def from_env(cls, **kwargs: object) -> AsyncKalshiClient: auth = KalshiAuth.try_from_env() demo = os.environ.get("KALSHI_DEMO", "").lower() == "true" base_url = os.environ.get("KALSHI_API_BASE_URL") - return cls(auth=auth, demo=demo, base_url=base_url, **kwargs) # type: ignore[arg-type] + client = cls(auth=auth, demo=demo, base_url=base_url, **kwargs) # type: ignore[arg-type] + # from_env constructs auth locally — caller never owned it. + client._auth_owned = auth is not None + return client async def close(self) -> None: """Close the underlying async HTTP connection pool.""" await self._transport.close() - if self._auth is not None: + if self._auth is not None and self._auth_owned: self._auth.close() async def __aenter__(self) -> AsyncKalshiClient: diff --git a/kalshi/client.py b/kalshi/client.py index ec8ddb68..055a9122 100644 --- a/kalshi/client.py +++ b/kalshi/client.py @@ -67,6 +67,9 @@ def __init__( # Reject empty strings that look like misconfigured credentials if key_id is not None and not key_id.strip(): raise ValueError("key_id must not be empty. Omit it for unauthenticated access.") + # #210: only shut down auth on close() when WE built it. If the + # caller passed in their own KalshiAuth, they keep ownership. + self._auth_owned: bool = auth is None self._auth: KalshiAuth | None if auth is not None: self._auth = auth @@ -135,12 +138,15 @@ def from_env(cls, **kwargs: object) -> KalshiClient: auth = KalshiAuth.try_from_env() demo = os.environ.get("KALSHI_DEMO", "").lower() == "true" base_url = os.environ.get("KALSHI_API_BASE_URL") - return cls(auth=auth, demo=demo, base_url=base_url, **kwargs) # type: ignore[arg-type] + client = cls(auth=auth, demo=demo, base_url=base_url, **kwargs) # type: ignore[arg-type] + # from_env constructs auth locally — caller never owned it. + client._auth_owned = auth is not None + return client def close(self) -> None: """Close the underlying HTTP connection pool.""" self._transport.close() - if self._auth is not None: + if self._auth is not None and self._auth_owned: self._auth.close() def __enter__(self) -> KalshiClient: diff --git a/kalshi/config.py b/kalshi/config.py index abbfd08a..a2f46efa 100644 --- a/kalshi/config.py +++ b/kalshi/config.py @@ -36,6 +36,10 @@ class KalshiConfig: max_retries: Max retry attempts for transient errors. Defaults to 3. retry_base_delay: Base delay in seconds for exponential backoff. Defaults to 0.5. retry_max_delay: Maximum delay in seconds for backoff. Defaults to 30. + total_timeout: Hard cap on cumulative time spent inside a single + request including retries, in seconds. ``None`` disables (legacy + behaviour: retry until ``max_retries`` exhausted regardless of + wall-clock). http2: Enable HTTP/2 for REST requests. Off by default for compat. Requires the ``h2`` package (install ``httpx[http2]`` or ``h2``). limits: Custom ``httpx.Limits`` for connection pool tuning. ``None`` @@ -47,6 +51,7 @@ class KalshiConfig: max_retries: int = DEFAULT_MAX_RETRIES retry_base_delay: float = 0.5 retry_max_delay: float = 30.0 + total_timeout: float | None = None extra_headers: dict[str, str] = field(default_factory=dict) ws_base_url: str = PRODUCTION_WS_URL # trailing slash is stripped automatically ws_max_retries: int = DEFAULT_WS_MAX_RETRIES @@ -61,6 +66,14 @@ def __post_init__(self) -> None: object.__setattr__(self, "ws_base_url", self.ws_base_url.rstrip("/")) KalshiConfig._validate_url(self.base_url, "base_url", secure="https", plaintext="http") KalshiConfig._validate_url(self.ws_base_url, "ws_base_url", secure="wss", plaintext="ws") + # #202: prevent silently calling /trade-api or /v1 by enforcing the + # v2 path component. Trailing slashes are already stripped above. + base_path = urlparse(self.base_url).path + if base_path != "/trade-api/v2": + raise ValueError( + f"KalshiConfig.base_url must include the /trade-api/v2 path " + f"component, got base_url={self.base_url!r}" + ) @staticmethod def _validate_url(url: str, field_name: str, *, secure: str, plaintext: str) -> None: diff --git a/tests/test_async_client.py b/tests/test_async_client.py index ed0e8bdf..549ce33f 100644 --- a/tests/test_async_client.py +++ b/tests/test_async_client.py @@ -19,8 +19,11 @@ from kalshi.errors import ( AuthRequiredError, KalshiAuthError, + KalshiConflictError, KalshiError, + KalshiPoolExhaustedError, KalshiServerError, + KalshiTimeoutError, KalshiValidationError, ) @@ -384,7 +387,7 @@ def test_demo_flag(self, test_auth: KalshiAuth) -> None: assert client._config.base_url == DEMO_BASE_URL def test_base_url_override(self, test_auth: KalshiAuth) -> None: - custom = "https://custom.api.com/v2" + custom = "https://custom.api.com/trade-api/v2" client = AsyncKalshiClient(auth=test_auth, base_url=custom) assert client._config.base_url == custom @@ -437,7 +440,7 @@ def test_from_env_demo_flag( def test_from_env_base_url_override( self, monkeypatch: pytest.MonkeyPatch, pem_string: str ) -> None: - custom = "https://custom.api.com/v2" + custom = "https://custom.api.com/trade-api/v2" monkeypatch.setenv("KALSHI_KEY_ID", "env-key") monkeypatch.setenv("KALSHI_PRIVATE_KEY", pem_string) monkeypatch.delenv("KALSHI_DEMO", raising=False) @@ -523,3 +526,245 @@ def test_ws_raises_without_auth(self) -> None: client = AsyncKalshiClient(demo=True) with pytest.raises(AuthRequiredError): _ = client.ws + +class TestAsyncWidenedRetrySet: + """#192: async mirror of widened retryable status set.""" + + @respx.mock + @pytest.mark.asyncio + async def test_retry_includes_cloudflare_5xx_521_on_get( + self, transport: AsyncTransport + ) -> None: + route = respx.get("https://test.kalshi.com/trade-api/v2/markets").mock( + side_effect=[ + httpx.Response(521, text="Web Server Is Down"), + 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_521_not_retried(self, transport: AsyncTransport) -> None: + route = respx.post( + "https://test.kalshi.com/trade-api/v2/portfolio/orders" + ).mock(return_value=httpx.Response(521, text="down")) + with pytest.raises(KalshiServerError): + await transport.request("POST", "/portfolio/orders", json={"ticker": "T"}) + assert route.call_count == 1 + + @respx.mock + @pytest.mark.asyncio + async def test_408_retried_on_get(self, transport: AsyncTransport) -> None: + route = respx.get("https://test.kalshi.com/trade-api/v2/markets").mock( + side_effect=[ + httpx.Response(408, json={"message": "request timeout"}), + httpx.Response(200, json={"markets": []}), + ] + ) + resp = await transport.request("GET", "/markets") + assert resp.status_code == 200 + assert route.call_count == 2 + + +class TestAsyncStatusToTypedException: + """#201: async hits _map_error too.""" + + @respx.mock + @pytest.mark.asyncio + async def test_422_maps_to_KalshiValidationError( # noqa: N802 + self, transport: AsyncTransport + ) -> None: + respx.get("https://test.kalshi.com/trade-api/v2/markets").mock( + return_value=httpx.Response(422, json={"message": "unprocessable"}) + ) + with pytest.raises(KalshiValidationError): + await transport.request("GET", "/markets") + + @respx.mock + @pytest.mark.asyncio + async def test_409_maps_to_KalshiConflictError( # noqa: N802 + self, transport: AsyncTransport + ) -> None: + respx.post( + "https://test.kalshi.com/trade-api/v2/portfolio/orders" + ).mock( + return_value=httpx.Response(409, json={"message": "duplicate"}) + ) + with pytest.raises(KalshiConflictError): + await transport.request( + "POST", "/portfolio/orders", json={"ticker": "T"} + ) + + +class TestAsyncPoolTimeout: + """#204: async pool exhaustion → KalshiPoolExhaustedError, always retried.""" + + @respx.mock + @pytest.mark.asyncio + async def test_pool_timeout_retried_on_post( + self, transport: AsyncTransport, monkeypatch: pytest.MonkeyPatch + ) -> None: + async def fake_sleep(d: float) -> None: + pass + + monkeypatch.setattr("asyncio.sleep", fake_sleep) + route = respx.post( + "https://test.kalshi.com/trade-api/v2/portfolio/orders" + ).mock( + side_effect=[ + httpx.PoolTimeout("pool full"), + httpx.Response(200, json={"order_id": "abc"}), + ] + ) + resp = await transport.request( + "POST", "/portfolio/orders", json={"ticker": "T"} + ) + assert resp.status_code == 200 + assert route.call_count == 2 + + @respx.mock + @pytest.mark.asyncio + async def test_pool_timeout_raises_KalshiPoolExhaustedError( # noqa: N802 + self, transport: AsyncTransport, monkeypatch: pytest.MonkeyPatch + ) -> None: + async def fake_sleep(d: float) -> None: + pass + + monkeypatch.setattr("asyncio.sleep", fake_sleep) + respx.post( + "https://test.kalshi.com/trade-api/v2/portfolio/orders" + ).mock(side_effect=httpx.PoolTimeout("pool full")) + with pytest.raises(KalshiPoolExhaustedError, match="pool exhausted"): + await transport.request( + "POST", "/portfolio/orders", json={"ticker": "T"} + ) + + +class TestAsyncTypedTimeoutException: + """#204: async POST read-timeout → KalshiTimeoutError, no retry.""" + + @respx.mock + @pytest.mark.asyncio + async def test_read_timeout_on_post_raises_KalshiTimeoutError_no_retry( # noqa: N802 + self, transport: AsyncTransport + ) -> None: + route = respx.post( + "https://test.kalshi.com/trade-api/v2/portfolio/orders" + ).mock(side_effect=httpx.ReadTimeout("read timed out")) + with pytest.raises(KalshiTimeoutError, match="timed out"): + await transport.request( + "POST", "/portfolio/orders", json={"ticker": "T"} + ) + assert route.call_count == 1 + + +class TestAsyncTotalTimeoutBudget: + """#193: async wall-clock budget.""" + + @respx.mock + @pytest.mark.asyncio + async def test_total_timeout_short_circuits_retries( + self, test_auth: KalshiAuth, monkeypatch: pytest.MonkeyPatch + ) -> None: + cfg = KalshiConfig( + base_url="https://test.kalshi.com/trade-api/v2", + timeout=5.0, + max_retries=5, + retry_base_delay=0.01, + retry_max_delay=0.1, + total_timeout=0.05, + ) + transport = AsyncTransport(test_auth, cfg) + sleeps: list[float] = [] + + async def fake_sleep(d: float) -> None: + sleeps.append(d) + + monkeypatch.setattr("asyncio.sleep", fake_sleep) + clock = {"t": 0.0} + + def fake_monotonic() -> float: + t = clock["t"] + clock["t"] += 1.0 + return t + + monkeypatch.setattr( + "kalshi._base_client.time.monotonic", fake_monotonic + ) + route = respx.get("https://test.kalshi.com/trade-api/v2/markets").mock( + return_value=httpx.Response(502, text="bad gateway") + ) + with pytest.raises(KalshiServerError): + await transport.request("GET", "/markets") + assert route.call_count == 1 + assert sleeps == [] + await transport.close() + + @respx.mock + @pytest.mark.asyncio + async def test_total_timeout_None_preserves_legacy_behavior( # noqa: N802 + self, test_auth: KalshiAuth, monkeypatch: pytest.MonkeyPatch + ) -> None: + cfg = KalshiConfig( + base_url="https://test.kalshi.com/trade-api/v2", + timeout=5.0, + max_retries=2, + retry_base_delay=0.001, + retry_max_delay=0.01, + total_timeout=None, + ) + transport = AsyncTransport(test_auth, cfg) + + async def fake_sleep(d: float) -> None: + pass + + monkeypatch.setattr("asyncio.sleep", fake_sleep) + route = respx.get("https://test.kalshi.com/trade-api/v2/markets").mock( + return_value=httpx.Response(502, text="bad gateway") + ) + with pytest.raises(KalshiServerError): + await transport.request("GET", "/markets") + assert route.call_count == 3 + await transport.close() + + +class TestAsyncCloseOwnership: + """#210: async close() must not shut down a caller-owned KalshiAuth.""" + + @pytest.mark.asyncio + async def test_close_does_not_shut_externally_provided_auth( + self, test_auth: KalshiAuth + ) -> None: + client_a = AsyncKalshiClient(auth=test_auth) + client_b = AsyncKalshiClient(auth=test_auth) + assert client_a._auth_owned is False + assert client_b._auth_owned is False + await client_a.close() + headers = test_auth.sign_request("GET", "/trade-api/v2/markets") + assert "KALSHI-ACCESS-KEY" in headers + await client_b.close() + + @pytest.mark.asyncio + async def test_close_shuts_locally_constructed_auth( + self, pem_string: str + ) -> None: + client = AsyncKalshiClient(key_id="test-key", private_key=pem_string) + assert client._auth_owned is True + await client.close() + await client.close() # idempotent + + @pytest.mark.asyncio + async def test_from_env_owns_auth( + self, monkeypatch: pytest.MonkeyPatch, pem_string: str + ) -> None: + monkeypatch.setenv("KALSHI_KEY_ID", "env-key") + monkeypatch.setenv("KALSHI_PRIVATE_KEY", pem_string) + monkeypatch.delenv("KALSHI_PRIVATE_KEY_PATH", raising=False) + monkeypatch.delenv("KALSHI_DEMO", raising=False) + monkeypatch.delenv("KALSHI_API_BASE_URL", raising=False) + client = AsyncKalshiClient.from_env() + assert client._auth_owned is True + await client.close() diff --git a/tests/test_client.py b/tests/test_client.py index e36cb6e1..d0f7cd31 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -19,10 +19,13 @@ from kalshi.errors import ( AuthRequiredError, KalshiAuthError, + KalshiConflictError, KalshiError, KalshiNotFoundError, + KalshiPoolExhaustedError, KalshiRateLimitError, KalshiServerError, + KalshiTimeoutError, KalshiValidationError, ) @@ -457,13 +460,13 @@ def test_demo_flag(self, test_auth: KalshiAuth) -> None: client.close() def test_base_url_override(self, test_auth: KalshiAuth) -> None: - custom = "https://custom.api.com/v2" + custom = "https://custom.api.com/trade-api/v2" client = KalshiClient(auth=test_auth, base_url=custom) assert client._config.base_url == custom client.close() def test_base_url_takes_precedence_over_demo(self, test_auth: KalshiAuth) -> None: - custom = "https://custom.api.com/v2" + custom = "https://custom.api.com/trade-api/v2" client = KalshiClient(auth=test_auth, base_url=custom, demo=True) assert client._config.base_url == custom client.close() @@ -661,7 +664,7 @@ def test_from_env_demo_flag( def test_from_env_base_url_override( self, monkeypatch: pytest.MonkeyPatch, pem_string: str ) -> None: - custom = "https://custom.api.com/v2" + custom = "https://custom.api.com/trade-api/v2" monkeypatch.setenv("KALSHI_KEY_ID", "env-key") monkeypatch.setenv("KALSHI_PRIVATE_KEY", pem_string) monkeypatch.delenv("KALSHI_DEMO", raising=False) @@ -906,3 +909,266 @@ def test_from_env_no_credentials_returns_unauthenticated( client = KalshiClient.from_env() assert client._auth is None client.close() + +class TestWidenedRetrySet: + """#192: Cloudflare 5xx (520-524) + 408 are retryable on safe methods only.""" + + @respx.mock + def test_retry_includes_cloudflare_5xx_521_on_get( + self, transport: SyncTransport + ) -> None: + route = respx.get("https://test.kalshi.com/trade-api/v2/markets").mock( + side_effect=[ + httpx.Response(521, text="Web Server Is Down"), + 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_521_not_retried(self, transport: SyncTransport) -> None: + route = respx.post( + "https://test.kalshi.com/trade-api/v2/portfolio/orders" + ).mock(return_value=httpx.Response(521, text="down")) + with pytest.raises(KalshiServerError): + transport.request("POST", "/portfolio/orders", json={"ticker": "T"}) + assert route.call_count == 1 + + @respx.mock + def test_408_retried_on_get(self, transport: SyncTransport) -> None: + route = respx.get("https://test.kalshi.com/trade-api/v2/markets").mock( + side_effect=[ + httpx.Response(408, json={"message": "request timeout"}), + httpx.Response(200, json={"markets": []}), + ] + ) + resp = transport.request("GET", "/markets") + assert resp.status_code == 200 + assert route.call_count == 2 + + +class TestStatusToTypedException: + """#201: 422 → KalshiValidationError, 409 → KalshiConflictError, 410 → KalshiError.""" + + def test_422_maps_to_KalshiValidationError(self) -> None: # noqa: N802 + resp = httpx.Response(422, json={"message": "unprocessable"}) + err = _map_error(resp) + assert isinstance(err, KalshiValidationError) + assert err.status_code == 422 + + def test_409_maps_to_KalshiConflictError(self) -> None: # noqa: N802 + resp = httpx.Response(409, json={"message": "duplicate client_order_id"}) + err = _map_error(resp) + assert isinstance(err, KalshiConflictError) + assert err.status_code == 409 + + def test_410_falls_back_to_KalshiError(self) -> None: # noqa: N802 + # 410 unmapped — see follow-up. Pin current behavior so a future + # change is intentional. + resp = httpx.Response(410, json={"message": "gone"}) + err = _map_error(resp) + assert type(err) is KalshiError + assert err.status_code == 410 + + +class TestErrorBodyBuffering: + """#203: bound memory + log-line size for hostile error responses.""" + + def test_oversized_error_body_truncated_by_content_length_header(self) -> None: + # respx builds a Response with explicit content-length header. + resp = httpx.Response( + 500, + headers={"content-length": "50000"}, + text="x", + ) + err = _map_error(resp) + assert "suppressed" in str(err) + assert "50000" in str(err) + + def test_error_message_truncated_to_1024_chars(self) -> None: + long_msg = "A" * 5000 + resp = httpx.Response(400, json={"message": long_msg}) + err = _map_error(resp) + assert len(str(err)) <= 1024 + + def test_malformed_content_length_falls_through(self) -> None: + # Malformed Content-Length should not crash; we parse the body normally. + resp = httpx.Response( + 400, + headers={"content-length": "not-a-number"}, + json={"message": "bad"}, + ) + err = _map_error(resp) + assert isinstance(err, KalshiValidationError) + + +class TestPoolTimeout: + """#204: pool exhaustion is always retryable; surfaces as KalshiPoolExhaustedError.""" + + @respx.mock + def test_pool_timeout_retried_on_post( + self, transport: SyncTransport, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setattr("kalshi._base_client.time.sleep", lambda d: None) + route = respx.post( + "https://test.kalshi.com/trade-api/v2/portfolio/orders" + ).mock( + side_effect=[ + httpx.PoolTimeout("pool full"), + httpx.Response(200, json={"order_id": "abc"}), + ] + ) + resp = transport.request("POST", "/portfolio/orders", json={"ticker": "T"}) + assert resp.status_code == 200 + assert route.call_count == 2 + + @respx.mock + def test_pool_timeout_raises_KalshiPoolExhaustedError( # noqa: N802 + self, transport: SyncTransport, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setattr("kalshi._base_client.time.sleep", lambda d: None) + respx.post( + "https://test.kalshi.com/trade-api/v2/portfolio/orders" + ).mock(side_effect=httpx.PoolTimeout("pool full")) + with pytest.raises(KalshiPoolExhaustedError, match="pool exhausted"): + transport.request("POST", "/portfolio/orders", json={"ticker": "T"}) + + +class TestTypedTimeoutException: + """#204: read/connect timeouts surface as KalshiTimeoutError, not bare KalshiError.""" + + @respx.mock + def test_read_timeout_on_post_raises_KalshiTimeoutError_no_retry( # noqa: N802 + self, transport: SyncTransport + ) -> None: + route = respx.post( + "https://test.kalshi.com/trade-api/v2/portfolio/orders" + ).mock(side_effect=httpx.ReadTimeout("read timed out")) + with pytest.raises(KalshiTimeoutError, match="timed out"): + transport.request("POST", "/portfolio/orders", json={"ticker": "T"}) + assert route.call_count == 1 + + +class TestTotalTimeoutBudget: + """#193: wall-clock budget cuts the retry loop short before sleeping past it.""" + + @respx.mock + def test_total_timeout_short_circuits_retries( + self, + test_auth: KalshiAuth, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + # 2-attempt window: monkeypatch monotonic so after the first sleep + # we appear to have blown the budget — third attempt MUST NOT fire. + cfg = KalshiConfig( + base_url="https://test.kalshi.com/trade-api/v2", + timeout=5.0, + max_retries=5, + retry_base_delay=0.01, + retry_max_delay=0.1, + total_timeout=0.05, + ) + transport = SyncTransport(test_auth, cfg) + sleeps: list[float] = [] + monkeypatch.setattr( + "kalshi._base_client.time.sleep", lambda d: sleeps.append(d) + ) + # Fake monotonic: start at 0, jump past the 0.05s budget on every + # subsequent poll so the very first retry's budget check trips. + clock = {"t": 0.0} + + def fake_monotonic() -> float: + t = clock["t"] + clock["t"] += 1.0 + return t + + monkeypatch.setattr( + "kalshi._base_client.time.monotonic", fake_monotonic + ) + route = respx.get("https://test.kalshi.com/trade-api/v2/markets").mock( + return_value=httpx.Response(502, text="bad gateway") + ) + with pytest.raises(KalshiServerError): + transport.request("GET", "/markets") + # Only the first attempt ran; the budget check refused to sleep into it. + assert route.call_count == 1 + assert sleeps == [] + transport.close() + + @respx.mock + def test_total_timeout_None_preserves_legacy_behavior( # noqa: N802 + self, test_auth: KalshiAuth, monkeypatch: pytest.MonkeyPatch + ) -> None: + cfg = KalshiConfig( + base_url="https://test.kalshi.com/trade-api/v2", + timeout=5.0, + max_retries=2, + retry_base_delay=0.001, + retry_max_delay=0.01, + total_timeout=None, + ) + transport = SyncTransport(test_auth, cfg) + monkeypatch.setattr("kalshi._base_client.time.sleep", lambda d: None) + route = respx.get("https://test.kalshi.com/trade-api/v2/markets").mock( + return_value=httpx.Response(502, text="bad gateway") + ) + with pytest.raises(KalshiServerError): + transport.request("GET", "/markets") + # max_retries=2 → 1 initial + 2 retries = 3 attempts. + assert route.call_count == 3 + transport.close() + + +class TestCloseOwnership: + """#210: close() must not shut down a caller-owned KalshiAuth.""" + + def test_close_does_not_shut_externally_provided_auth( + self, test_auth: KalshiAuth + ) -> None: + # Two clients share the same auth; closing one must not break the other. + client_a = KalshiClient(auth=test_auth) + client_b = KalshiClient(auth=test_auth) + assert client_a._auth_owned is False + assert client_b._auth_owned is False + client_a.close() + # The auth still works — signing a request after close on client_a. + headers = test_auth.sign_request("GET", "/trade-api/v2/markets") + assert "KALSHI-ACCESS-KEY" in headers + client_b.close() + + def test_close_shuts_locally_constructed_auth(self, pem_string: str) -> None: + client = KalshiClient(key_id="test-key", private_key=pem_string) + assert client._auth_owned is True + auth = client._auth + assert auth is not None + client.close() + # Idempotent: close() is safe to call twice without raising. + client.close() + + def test_from_env_owns_auth( + self, monkeypatch: pytest.MonkeyPatch, pem_string: str + ) -> None: + monkeypatch.setenv("KALSHI_KEY_ID", "env-key") + monkeypatch.setenv("KALSHI_PRIVATE_KEY", pem_string) + monkeypatch.delenv("KALSHI_PRIVATE_KEY_PATH", raising=False) + monkeypatch.delenv("KALSHI_DEMO", raising=False) + monkeypatch.delenv("KALSHI_API_BASE_URL", raising=False) + client = KalshiClient.from_env() + assert client._auth_owned is True + client.close() + + def test_from_env_no_credentials_owns_nothing( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.delenv("KALSHI_KEY_ID", raising=False) + monkeypatch.delenv("KALSHI_PRIVATE_KEY", raising=False) + monkeypatch.delenv("KALSHI_PRIVATE_KEY_PATH", raising=False) + monkeypatch.delenv("KALSHI_DEMO", raising=False) + monkeypatch.delenv("KALSHI_API_BASE_URL", raising=False) + client = KalshiClient.from_env() + assert client._auth is None + # No auth → ownership flag is moot but should be False (nothing to own). + assert client._auth_owned is False + client.close() diff --git a/tests/test_config.py b/tests/test_config.py index 484524b9..f271beb4 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -194,3 +194,37 @@ def test_custom_limits_forwarded_to_sync_client( assert transport._config.limits is limits finally: transport.close() + + +class TestBaseUrlPathValidation: + """#202: base_url MUST include the /trade-api/v2 path component.""" + + def test_base_url_without_trade_api_v2_raises(self) -> None: + import pytest + + with pytest.raises(ValueError, match="/trade-api/v2"): + KalshiConfig(base_url="https://demo-api.kalshi.co/") + with pytest.raises(ValueError, match="/trade-api/v2"): + KalshiConfig(base_url="https://demo-api.kalshi.co/v1") + with pytest.raises(ValueError, match="/trade-api/v2"): + KalshiConfig(base_url="https://demo-api.kalshi.co/trade-api") + + def test_base_url_with_trade_api_v2_accepted(self) -> None: + config = KalshiConfig(base_url="https://demo-api.kalshi.co/trade-api/v2") + assert config.base_url == "https://demo-api.kalshi.co/trade-api/v2" + + def test_base_url_with_trailing_slash_accepted(self) -> None: + # Trailing slash is stripped before path validation. + config = KalshiConfig(base_url="https://demo-api.kalshi.co/trade-api/v2/") + assert config.base_url == "https://demo-api.kalshi.co/trade-api/v2" + + +class TestTotalTimeoutField: + """#193: total_timeout field exists, defaults to None (legacy).""" + + def test_total_timeout_field_exists_and_defaults_None(self) -> None: # noqa: N802 + assert KalshiConfig().total_timeout is None + + def test_total_timeout_can_be_set(self) -> None: + config = KalshiConfig(total_timeout=30.0) + assert config.total_timeout == 30.0