Skip to content

Commit 2ad59da

Browse files
author
Mateusz
committed
fix: address code review findings from HTTP 429/rate-limit and resilience work
- Anthropic connector: remove broad except Exception in Retry-After parsing and simplify extraction logic to use single headers.get() path - Composite failure recovery: clarify recycle policy (fresh round keeps only current failed selector excluded) and remove unnecessary isinstance check - Integration tests: add explicit assertions for recycle exclusion-state policy - Unit tests: add direct coverage for Retry-After metadata extraction (numeric and non-numeric header values)
1 parent ba07bf8 commit 2ad59da

4 files changed

Lines changed: 51 additions & 26 deletions

File tree

src/connectors/anthropic.py

Lines changed: 19 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -74,35 +74,31 @@
7474
_LLM_PROXY_CLIENT_HOST_KEY = "_llm_proxy_client_host"
7575

7676

77-
def _retry_after_metadata_from_httpx_headers(
78-
headers: Any,
79-
) -> tuple[dict[str, Any], int | None]:
77+
def _retry_after_metadata_from_httpx_headers(
78+
headers: Any,
79+
) -> tuple[dict[str, Any], int | None]:
8080
"""Extract Retry-After for resilience (same ``details['headers']`` shape as OpenAI).
8181
8282
``RateLimitErrorHandler`` reads ``details['headers']['retry-after']`` when
8383
``reset_at`` is not a usable wall-clock hint, so we populate that structure here.
8484
"""
8585

86-
details: dict[str, Any] = {}
87-
reset_hint: int | None = None
88-
retry_after: str | None = None
89-
try:
90-
if hasattr(headers, "get"):
91-
got = headers.get("retry-after")
92-
if got is not None:
93-
retry_after = str(got).strip()
94-
if not retry_after:
95-
for key, value in headers.items():
96-
if str(key).lower() == "retry-after":
97-
retry_after = str(value).strip()
98-
break
99-
if retry_after:
100-
details["headers"] = {"retry-after": retry_after}
101-
with contextlib.suppress(ValueError, TypeError):
102-
reset_hint = int(retry_after.split(",")[0].strip())
103-
except Exception:
104-
return {}, None
105-
return details, reset_hint
86+
if not hasattr(headers, "get"):
87+
return {}, None
88+
89+
retry_after_raw = headers.get("retry-after")
90+
if retry_after_raw is None:
91+
return {}, None
92+
93+
retry_after = str(retry_after_raw).strip()
94+
if not retry_after:
95+
return {}, None
96+
97+
details: dict[str, Any] = {"headers": {"retry-after": retry_after}}
98+
reset_hint: int | None = None
99+
with contextlib.suppress(ValueError, TypeError):
100+
reset_hint = int(retry_after.split(",", 1)[0].strip())
101+
return details, reset_hint
106102

107103

108104
def _message_tool_calls(msg: Any) -> list[Any] | None:

src/core/services/composite_failure_recovery_bridge.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -175,8 +175,9 @@ def _build_weighted_retry_request(
175175
remaining.append(item)
176176

177177
if not remaining:
178-
# All branches were exhausted once; recycle candidates by keeping only
179-
# the current failed selector excluded and retrying the rest.
178+
# All branches were exhausted once; start a fresh recycle round.
179+
# Keep only the currently failed selector excluded so all others
180+
# become eligible again within the same hop budget window.
180181
excluded = [selected]
181182
excluded_set = {selected}
182183
remaining = [
@@ -256,7 +257,7 @@ def _is_weighted_reroll_eligible(exc: Exception) -> bool:
256257
status_code = getattr(exc, "status_code", None)
257258
if status_code in {401, 403, 499}:
258259
return False
259-
details = exc.details if isinstance(exc.details, dict) else {}
260+
details = exc.details
260261
error_code = getattr(exc, "code", None)
261262
if not isinstance(error_code, str) or not error_code:
262263
candidate = details.get("code")

tests/integration/core/services/test_weighted_routing_recycles_on_transient_400.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -451,6 +451,9 @@ def _rng() -> float:
451451
state = context.extensions["composite_routing_state"]
452452
assert isinstance(state, dict)
453453
assert state["selected_selector"] == "zai-coding-plan:glm-5.1"
454+
# Recycle policy: after full exhaustion, only the immediately failed selector
455+
# remains excluded so prior candidates become eligible again.
456+
assert state["excluded_selectors"] == ["qwen-oauth:coder-model"]
454457
assert state["hop_count"] == 2
455458

456459

@@ -522,4 +525,5 @@ def _rng() -> float:
522525
state = context.extensions["composite_routing_state"]
523526
assert isinstance(state, dict)
524527
assert state["selected_selector"] == "zai-coding-plan:glm-5.1"
528+
assert state["excluded_selectors"] == ["qwen-oauth:coder-model"]
525529
assert state["hop_count"] == 2

tests/unit/connectors/test_anthropic_error_handling.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,30 @@
77
from src.core.domain.chat import CanonicalChatRequest, ChatMessage
88

99

10+
def test_retry_after_metadata_from_headers() -> None:
11+
"""Retry-After extraction preserves header and parses numeric seconds."""
12+
from src.connectors.anthropic import _retry_after_metadata_from_httpx_headers
13+
14+
details, reset_hint = _retry_after_metadata_from_httpx_headers(
15+
httpx.Headers({"Retry-After": "42"})
16+
)
17+
18+
assert details == {"headers": {"retry-after": "42"}}
19+
assert reset_hint == 42
20+
21+
22+
def test_retry_after_metadata_handles_non_numeric_header() -> None:
23+
"""Non-numeric Retry-After is preserved while reset hint remains unset."""
24+
from src.connectors.anthropic import _retry_after_metadata_from_httpx_headers
25+
26+
details, reset_hint = _retry_after_metadata_from_httpx_headers(
27+
httpx.Headers({"Retry-After": "Wed, 21 Oct 2015 07:28:00 GMT"})
28+
)
29+
30+
assert details == {"headers": {"retry-after": "Wed, 21 Oct 2015 07:28:00 GMT"}}
31+
assert reset_hint is None
32+
33+
1034
@pytest.mark.asyncio
1135
async def test_anthropic_streaming_handles_error_events():
1236
"""Test that Anthropic connector properly handles error events in streaming."""

0 commit comments

Comments
 (0)