refactor(V2): IBKR client — log tickle, type _http, retry once on 401
Code review fixes (commit 0c74691):
- _maybe_tickle logs failures via logger.debug instead of silent pass
- _http typed as httpx.AsyncClient | None
- 30s timeout commented (matches Alpaca, IBKR gateway latency)
- _request retries once on 401 with forced LST refresh (spec §4 IBKR_AUTH_FAILED)
- New tests: test_request_retries_once_on_401, test_request_raises_on_persistent_401
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,11 +1,14 @@
|
|||||||
"""IBKR Client Portal Web API client (REST httpx + OAuth1a)."""
|
"""IBKR Client Portal Web API client (REST httpx + OAuth1a)."""
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import logging
|
||||||
import time
|
import time
|
||||||
from collections import OrderedDict
|
from collections import OrderedDict
|
||||||
from dataclasses import dataclass, field
|
from dataclasses import dataclass, field
|
||||||
from typing import Any
|
from typing import Any
|
||||||
|
|
||||||
|
import httpx
|
||||||
|
|
||||||
from cerbero_mcp.common.http import async_client
|
from cerbero_mcp.common.http import async_client
|
||||||
from cerbero_mcp.exchanges.ibkr.oauth import (
|
from cerbero_mcp.exchanges.ibkr.oauth import (
|
||||||
IBKRAuthError,
|
IBKRAuthError,
|
||||||
@@ -13,6 +16,8 @@ from cerbero_mcp.exchanges.ibkr.oauth import (
|
|||||||
_percent_encode,
|
_percent_encode,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
|
||||||
class IBKRError(Exception):
|
class IBKRError(Exception):
|
||||||
"""Generic IBKR API error (non-auth)."""
|
"""Generic IBKR API error (non-auth)."""
|
||||||
@@ -32,11 +37,13 @@ class IBKRClient:
|
|||||||
default_factory=OrderedDict, init=False, repr=False
|
default_factory=OrderedDict, init=False, repr=False
|
||||||
)
|
)
|
||||||
_last_request_at: float = field(default=0.0, init=False, repr=False)
|
_last_request_at: float = field(default=0.0, init=False, repr=False)
|
||||||
_http: Any = field(default=None, init=False, repr=False)
|
_http: httpx.AsyncClient | None = field(default=None, init=False, repr=False)
|
||||||
|
|
||||||
_CONID_CACHE_MAX = 1024
|
_CONID_CACHE_MAX = 1024
|
||||||
|
|
||||||
def __post_init__(self) -> None:
|
def __post_init__(self) -> None:
|
||||||
|
# IBKR Client Portal gateway latency is higher than crypto exchanges
|
||||||
|
# (lookup roundtrip + session validation); 30s matches Alpaca's choice.
|
||||||
self._http = async_client(timeout=30.0)
|
self._http = async_client(timeout=30.0)
|
||||||
|
|
||||||
async def aclose(self) -> None:
|
async def aclose(self) -> None:
|
||||||
@@ -62,13 +69,21 @@ class IBKRClient:
|
|||||||
async def _maybe_tickle(self) -> None:
|
async def _maybe_tickle(self) -> None:
|
||||||
if time.monotonic() - self._last_request_at < _TICKLE_INTERVAL_S:
|
if time.monotonic() - self._last_request_at < _TICKLE_INTERVAL_S:
|
||||||
return
|
return
|
||||||
|
if self._http is None: # pragma: no cover
|
||||||
|
return
|
||||||
try:
|
try:
|
||||||
url = f"{self.base_url}/tickle"
|
url = f"{self.base_url}/tickle"
|
||||||
auth = await self._build_auth_header("POST", url)
|
auth = await self._build_auth_header("POST", url)
|
||||||
await self._http.post(url, headers={"Authorization": auth})
|
await self._http.post(url, headers={"Authorization": auth})
|
||||||
except Exception:
|
except Exception as exc:
|
||||||
# Tickle is best-effort; failure shouldn't block real request
|
# Best-effort: failure shouldn't block the real request, but log
|
||||||
pass
|
# so misconfigured signer / dead session aren't invisible.
|
||||||
|
logger.debug("ibkr tickle best-effort failed: %s", exc)
|
||||||
|
|
||||||
|
async def _force_lst_refresh(self) -> None:
|
||||||
|
"""Invalidate cached LST and remint on next call."""
|
||||||
|
self.signer._live_session_token = None
|
||||||
|
self.signer._lst_expires_at = 0.0
|
||||||
|
|
||||||
async def _request(
|
async def _request(
|
||||||
self,
|
self,
|
||||||
@@ -78,7 +93,10 @@ class IBKRClient:
|
|||||||
params: dict[str, Any] | None = None,
|
params: dict[str, Any] | None = None,
|
||||||
json_body: dict[str, Any] | None = None,
|
json_body: dict[str, Any] | None = None,
|
||||||
skip_tickle: bool = False,
|
skip_tickle: bool = False,
|
||||||
|
_retried_auth: bool = False,
|
||||||
) -> Any:
|
) -> Any:
|
||||||
|
if self._http is None: # pragma: no cover — set in __post_init__
|
||||||
|
raise IBKRError("http client not initialized")
|
||||||
if not skip_tickle:
|
if not skip_tickle:
|
||||||
await self._maybe_tickle()
|
await self._maybe_tickle()
|
||||||
url = f"{self.base_url}{path}"
|
url = f"{self.base_url}{path}"
|
||||||
@@ -94,16 +112,22 @@ class IBKRClient:
|
|||||||
headers={"Authorization": auth, "User-Agent": "cerbero-mcp/2.0"},
|
headers={"Authorization": auth, "User-Agent": "cerbero-mcp/2.0"},
|
||||||
)
|
)
|
||||||
self._last_request_at = time.monotonic()
|
self._last_request_at = time.monotonic()
|
||||||
|
if resp.status_code == 401 and not _retried_auth:
|
||||||
|
# Retry once with fresh LST (per spec: IBKR_AUTH_FAILED retryable)
|
||||||
|
logger.info("ibkr 401 on %s %s — refreshing LST and retrying once", method, path)
|
||||||
|
await self._force_lst_refresh()
|
||||||
|
return await self._request(
|
||||||
|
method, path, params=params, json_body=json_body,
|
||||||
|
skip_tickle=skip_tickle, _retried_auth=True,
|
||||||
|
)
|
||||||
if resp.status_code == 401:
|
if resp.status_code == 401:
|
||||||
raise IBKRAuthError(f"401 on {method} {path}: {resp.text[:200]}")
|
raise IBKRAuthError(f"401 on {method} {path} (after retry): {resp.text[:200]}")
|
||||||
if resp.status_code == 429:
|
if resp.status_code == 429:
|
||||||
raise IBKRError(f"IBKR_RATE_LIMITED: {resp.text[:200]}")
|
raise IBKRError(f"IBKR_RATE_LIMITED: {resp.text[:200]}")
|
||||||
if resp.status_code >= 500:
|
if resp.status_code >= 500:
|
||||||
raise IBKRError(f"IBKR_SERVER_ERROR status={resp.status_code}")
|
raise IBKRError(f"IBKR_SERVER_ERROR status={resp.status_code}")
|
||||||
if resp.status_code >= 400:
|
if resp.status_code >= 400:
|
||||||
raise IBKRError(
|
raise IBKRError(f"IBKR_HTTP_{resp.status_code}: {resp.text[:300]}")
|
||||||
f"IBKR_HTTP_{resp.status_code}: {resp.text[:300]}"
|
|
||||||
)
|
|
||||||
if not resp.content:
|
if not resp.content:
|
||||||
return {}
|
return {}
|
||||||
return resp.json()
|
return resp.json()
|
||||||
|
|||||||
@@ -52,3 +52,37 @@ async def test_get_account_summary(httpx_mock: HTTPXMock, client):
|
|||||||
)
|
)
|
||||||
data = await client.get_account()
|
data = await client.get_account()
|
||||||
assert "netliquidation" in data
|
assert "netliquidation" in data
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_request_retries_once_on_401(httpx_mock: HTTPXMock, client):
|
||||||
|
httpx_mock.add_response(url=re.compile(r".*/tickle"), json={})
|
||||||
|
# First call returns 401
|
||||||
|
httpx_mock.add_response(
|
||||||
|
url=re.compile(r".*/portfolio/DU1234/summary"),
|
||||||
|
status_code=401, text="session expired",
|
||||||
|
)
|
||||||
|
# Second call (after LST refresh) succeeds
|
||||||
|
httpx_mock.add_response(
|
||||||
|
url=re.compile(r".*/portfolio/DU1234/summary"),
|
||||||
|
json={"netliquidation": {"amount": 5000}},
|
||||||
|
)
|
||||||
|
data = await client.get_account()
|
||||||
|
assert data["netliquidation"]["amount"] == 5000
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_request_raises_on_persistent_401(httpx_mock: HTTPXMock, client):
|
||||||
|
from cerbero_mcp.exchanges.ibkr.oauth import IBKRAuthError
|
||||||
|
httpx_mock.add_response(url=re.compile(r".*/tickle"), json={})
|
||||||
|
# Both attempts return 401
|
||||||
|
httpx_mock.add_response(
|
||||||
|
url=re.compile(r".*/portfolio/DU1234/summary"),
|
||||||
|
status_code=401, text="bad creds",
|
||||||
|
)
|
||||||
|
httpx_mock.add_response(
|
||||||
|
url=re.compile(r".*/portfolio/DU1234/summary"),
|
||||||
|
status_code=401, text="bad creds",
|
||||||
|
)
|
||||||
|
with pytest.raises(IBKRAuthError, match="after retry"):
|
||||||
|
await client.get_account()
|
||||||
|
|||||||
Reference in New Issue
Block a user