From bda2917273d310dbae6359837246081629decc85 Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Sat, 29 Aug 2026 17:13:10 -0700 Subject: [PATCH] test(e2e): retry upstream-saturation failures in the claude CLI driver The driver retries rate-limit-shaped failures twice with a 65s backoff, but RATE_LIMIT_SHAPED_RE only matches 429 / rate limit / too many requests / throttled / CLI timeout. A saturated provider does not always say any of those: litellm-e2e-pr build 182 turned a green cell red on status 503: litellm.ServiceUnavailableError: BedrockException - {"message":"Bedrock is unable to process your request."} which matched nothing and so was never retried. Add TRANSIENT_UPSTREAM_SHAPED_RE for 503, 529, "service unavailable", "overloaded" and Bedrock's "unable to process your request", and have the retry loop ask is_retryable_shaped (either shape) instead of is_rate_limit_shaped. Kept as a second pattern rather than widened into the first on purpose. The conftest feeds RATE_LIMIT_SHAPED_RE into the rate-limit summary, which the binary-search helper reads to decide whether to lower a provider's request rate. A 503 says the provider is out of capacity, not that we are asking too often, so folding it in there would keep ratcheting our rate down against a condition our rate never caused. Retry-loop logging now names which shape fired, so a saturated upstream is distinguishable from a throttled one in the run output. Verified against the literal failure text from build 182: is_retryable_shaped is True while is_rate_limit_shaped stays False. Note this covers CLI-driven rows only. HTTP-probe rows (tool_search, count_tokens) have no retry layer at all, so the specific cell that failed in build 182 is still unprotected -- that is a separate change to http_probe. --- .../_driver_unit_tests/__init__.py | 0 .../test_retry_classification.py | 74 +++++++++++++++++++ tests/e2e/claude_code/cli_driver.py | 38 ++++++++-- 3 files changed, 107 insertions(+), 5 deletions(-) create mode 100644 tests/e2e/claude_code/_driver_unit_tests/__init__.py create mode 100644 tests/e2e/claude_code/_driver_unit_tests/test_retry_classification.py diff --git a/tests/e2e/claude_code/_driver_unit_tests/__init__.py b/tests/e2e/claude_code/_driver_unit_tests/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/e2e/claude_code/_driver_unit_tests/test_retry_classification.py b/tests/e2e/claude_code/_driver_unit_tests/test_retry_classification.py new file mode 100644 index 00000000000..868110addb6 --- /dev/null +++ b/tests/e2e/claude_code/_driver_unit_tests/test_retry_classification.py @@ -0,0 +1,74 @@ +"""Unit tests for the retry-shape classification in `cli_driver`. + +Markerless harness tests: they exercise driver plumbing over hand-built +outcomes, not a product feature, so they run without a proxy and carry no +`e2e` marker. + +The pairing that matters is that a saturated upstream is retryable but is not +rate-limit-shaped. litellm-e2e-pr build 182 failed a green cell on a Bedrock +503 that no pattern matched, while feeding a 503 to the rate-limit summary +would tell the rate-limiter's binary search to lower a request rate that was +never the problem. +""" + +from __future__ import annotations + +import pytest + +from claude_code.cli_driver import ( + ClaudeCLIError, + DriverResult, + is_rate_limit_shaped, + is_retryable_shaped, + is_transient_upstream_shaped, +) + +_BEDROCK_503 = ( + "[claude-opus-4-7-bedrock-converse] tool_search probe failed: status 503: " + '{"error":{"message":"litellm.ServiceUnavailableError: BedrockException - ' + '{\\"message\\":\\"Bedrock is unable to process your request.\\"}"}}' +) +_ANTHROPIC_529 = "status 529: {\"type\":\"overloaded_error\"}" +_OPENAI_429 = 'status 429: {"error":{"message":"Rate limit reached"}}' + + +def _failed(text: str) -> DriverResult: + return DriverResult(text=text, exit_code=1) + + +@pytest.mark.parametrize( + "text, rate_limit, transient", + [ + (_BEDROCK_503, False, True), + (_ANTHROPIC_529, False, True), + ("status 503 service unavailable", False, True), + ("upstream overloaded, try again later", False, True), + (_OPENAI_429, True, False), + ("throttling exception from provider", True, False), + ("claude CLI timed out after 120s", True, False), + ('status 400: {"error":"bad request"}', False, False), + ], +) +def test_shapes_are_classified_independently(text: str, rate_limit: bool, transient: bool) -> None: + outcome = _failed(text) + assert is_rate_limit_shaped(outcome) is rate_limit + assert is_transient_upstream_shaped(outcome) is transient + assert is_retryable_shaped(outcome) is (rate_limit or transient) + + +def test_bedrock_503_is_retryable_but_not_rate_limit_shaped() -> None: + outcome = _failed(_BEDROCK_503) + assert is_retryable_shaped(outcome) + assert not is_rate_limit_shaped(outcome) + + +def test_passing_outcome_is_never_retryable() -> None: + passed = DriverResult(text=_BEDROCK_503, exit_code=0) + assert not is_retryable_shaped(passed) + assert not is_transient_upstream_shaped(passed) + + +def test_driver_error_message_is_classified() -> None: + assert is_transient_upstream_shaped(ClaudeCLIError("upstream returned 503")) + assert is_rate_limit_shaped(ClaudeCLIError("claude CLI timed out")) + assert not is_retryable_shaped(ClaudeCLIError("binary not found")) diff --git a/tests/e2e/claude_code/cli_driver.py b/tests/e2e/claude_code/cli_driver.py index 5b18c1c291a..447e8cc0bbb 100644 --- a/tests/e2e/claude_code/cli_driver.py +++ b/tests/e2e/claude_code/cli_driver.py @@ -52,6 +52,17 @@ the CLI retries 429s internally until the harness timeout kills it, so a saturated upstream usually surfaces as a timeout rather than a clean 429.""" +TRANSIENT_UPSTREAM_SHAPED_RE = re.compile( + r"(?:\b503\b|\b529\b|service[\s_-]?unavailable|overloaded|" + r"unable\s+to\s+process\s+your\s+request)", + re.IGNORECASE, +) +"""Upstream saturation, retried on the same terms as a 429 but deliberately a +separate pattern: it must not reach the rate-limit summary, whose only remedy is +lowering our own request rate, which does nothing for a provider that is simply +out of capacity.""" + + DEFAULT_RATE_LIMIT_RETRIES = int( os.environ.get("LITELLM_COMPAT_RATE_LIMIT_RETRIES") or 2 ) @@ -298,11 +309,27 @@ def is_rate_limit_shaped(outcome: ModelResult) -> bool: CLI's stdout text or `api_error_status` are both caught. Passing results are never rate-limit-shaped. """ + return _matches_failure_shape(outcome, RATE_LIMIT_SHAPED_RE) + + +def is_transient_upstream_shaped(outcome: ModelResult) -> bool: + """Classify an outcome as a retryable upstream-saturation failure: a 503 or + 529, an "overloaded" marker, or Bedrock's "unable to process your request".""" + return _matches_failure_shape(outcome, TRANSIENT_UPSTREAM_SHAPED_RE) + + +def is_retryable_shaped(outcome: ModelResult) -> bool: + """Either retryable shape. This, not `is_rate_limit_shaped`, is what the + retry loop asks: both shapes clear on their own given time.""" + return is_rate_limit_shaped(outcome) or is_transient_upstream_shaped(outcome) + + +def _matches_failure_shape(outcome: ModelResult, pattern: "re.Pattern[str]") -> bool: if isinstance(outcome, ClaudeCLIError): - return bool(RATE_LIMIT_SHAPED_RE.search(str(outcome))) + return bool(pattern.search(str(outcome))) if outcome.exit_code == 0: return False - return bool(RATE_LIMIT_SHAPED_RE.search(failure_diagnostic(outcome))) + return bool(pattern.search(failure_diagnostic(outcome))) def run_claude_models_parallel( @@ -333,7 +360,7 @@ def run_claude_models_parallel( keep the synchronous CLI driver unchanged so unit tests can keep injecting a fake `runner`. - Rate-limit-shaped failures (see `is_rate_limit_shaped`) are retried + Retryable failures (see `is_retryable_shaped`) are retried per model up to `rate_limit_retries` times, sleeping `rate_limit_backoff_seconds` before each retry so per-minute quota windows can reset; both default to the `LITELLM_COMPAT_RATE_LIMIT_*` @@ -401,10 +428,11 @@ def run_claude_models_parallel( started = time.monotonic() outcome = _run_once(model) for attempt in range(retries): - if not is_rate_limit_shaped(outcome): + if not is_retryable_shaped(outcome): break + shape = "rate-limit" if is_rate_limit_shaped(outcome) else "transient-upstream" print( - f"[retry] {model}: rate-limit-shaped failure; sleeping " + f"[retry] {model}: {shape}-shaped failure; sleeping " f"{backoff:.0f}s before attempt {attempt + 2}/{retries + 1}", file=sys.stderr, flush=True,