fix: retry transient Anthropic server errors (HTTP 200 with error body) - #43
Open
Sam Schillace (ramparte) wants to merge 6 commits into
Open
fix: retry transient Anthropic server errors (HTTP 200 with error body)#43Sam Schillace (ramparte) wants to merge 6 commits into
Sam Schillace (ramparte) wants to merge 6 commits into
Conversation
added 6 commits
March 3, 2026 12:11
## Problem
Cloudflare interposes HTML challenge pages (HTTP 403) in front of
api.anthropic.com when bot-detection triggers (common during concurrent
delegation requests). The provider-anthropic module was treating ALL 403s
as permanent API failures (KernelAccessDeniedError, non-retryable), causing
sessions to die immediately instead of retrying.
## Root Cause
No distinction between two types of 403 responses:
- **Cloudflare 403**: HTML body with challenge, transient bot challenge
- **API 403**: JSON body from Anthropic, permanent access denial
Both hit the same error path and immediately fail.
## Solution
Added _is_cloudflare_challenge() static method that detects Cloudflare
responses by checking:
- response.body is None (HTML parse failure) or 'content-type: text/html'
- Presence of Cloudflare markers ('Just a moment', 'Ray ID', etc.)
Modified _do_complete() 403 handling:
- Cloudflare challenges now raise KernelProviderUnavailableError (retryable)
- API 403s remain KernelAccessDeniedError (non-retryable)
- retry_with_backoff() wrapper automatically handles retryable errors
## Testing
- 10 new tests in test_cloudflare_retry.py covering detection logic and retry integration
- All 257 tests pass (10 new + 247 existing)
- Verified against both HTML and JSON 403 responses
## Impact
Sessions using delegation no longer hard-fail on transient Cloudflare
challenges. Retryable backoff allows time for Cloudflare bot-challenge
window to close.
Add two features to address Cloudflare bot-detection triggered by concurrent
API calls from multiple delegated sessions:
Feature 1 — Process-wide concurrency semaphore
- New module-level _process_semaphore (asyncio.Semaphore) shared across ALL
AnthropicProvider instances in the same process (parent + child sessions).
- New config key max_concurrent_requests (default: 5). Set to 0 to disable.
- _do_complete_guarded() wraps each _do_complete() attempt with semaphore
acquire/release so that at most N API calls are in-flight simultaneously.
Semaphore is released between retry attempts (during backoff sleep) so
other requests can proceed while a CF-blocked call waits to retry.
- Semaphore is keyed by running event loop so asyncio.run()-based tests each
get a fresh semaphore without 'Future attached to different loop' errors.
Feature 2 — Concurrency diagnostic logging
- provider:concurrency event emitted before each API call attempt with:
active_requests, waiting_requests, max_concurrent, process_id, model
Allows post-mortem correlation in events.jsonl: was request volume the cause?
- provider:cloudflare_challenge event emitted on every Cloudflare 403 with
the same concurrency fields plus a timestamp. One event per attempt,
so retry storms are visible in the event log.
Tests (tests/test_concurrency.py — 24 new tests, 281 total):
- Semaphore config: default=5, override, zero-disables, idempotent, loop-refresh
- Concurrency enforcement: peak in-flight <= limit, limit=1 serializes,
disabled allows full concurrency, all requests complete
- Event emission: fields present, max_concurrent matches config, process_id
correct, emitted with semaphore disabled, active_requests >= 1, no crash
without coordinator
- Cloudflare challenge event: emitted per attempt (incl. retries), all fields
present, process_id correct, NOT emitted for real JSON-body 403s
After each successful API response, write rate-limit header data to a shared JSON file at ~/.amplifier/rate-limit-state.json using an atomic tmp→rename pattern. Before each pre-emptive throttle check, read that file and merge in the lower remaining values so that sibling processes (parallel sessions, Docker containers sharing a home dir) can see how much capacity other processes have consumed. Changes: - _write_shared_rate_limit_state(): atomic JSON write with debounce (skip if data unchanged) and full try/except safety - _read_shared_rate_limit_state(): read+merge with 1s cache, 120s staleness window, and conservative min() merge for remaining fields - __init__: three new instance attrs (shared_state_path, last_read ts, last_written dict); configurable via rate_limit_state_path config key - complete(): read hook before throttle check; write hook after update_from_headers on every successful response - tests/test_shared_state.py: 17 new tests covering all 7 spec cases - tests/test_throttle.py: disable shared state in provider factory so unit tests that seed local _RateLimitState directly are not affected by a real ~/.amplifier/rate-limit-state.json on the test machine All 298 tests pass.
The `enable_1m_context` config value arrives as a YAML string (e.g.,
"false") when set in settings.yaml. Python's truthiness treats the
string "false" as truthy, causing get_info() and list_models() to
report a 1M context window even when the user intended to disable it.
Meanwhile, the __init__ constructor already handled this correctly
with explicit string normalization. The mismatch meant:
- API calls used 200K context (no beta header sent — correct)
- context-simple believed 1M was available (incorrect)
- Compaction threshold set at ~745K, never reachable
- Sessions hit the 200K hard limit without any compaction
This fix:
- Extracts _is_1m_enabled() helper with proper string normalization
- Replaces raw truthiness checks in get_info(), list_models(), and __init__
- Fixes llm:response event key names ("input"/"output" → "input_tokens"/"output_tokens")
to match the Usage model field names expected by consumers
Adds two defensive layers for transient Anthropic API failures that
currently kill sessions instead of being retried:
1. Response validation after API call: checks response.type — if the
SDK returns a response that isn't type "message" (e.g., HTTP 200
with an error body like {"type": "error", "error": {"type":
"api_error", "message": "Internal server error"}}), raises
KernelProviderUnavailableError(retryable=True) so the retry loop
handles it.
2. Base AnthropicAPIError handler: catches the SDK's base APIError
class (not just APIStatusError subclasses). This covers
APIConnectionError, APITimeoutError, and any future SDK error
types that don't inherit from APIStatusError. All treated as
transient/retryable.
Root cause: Session cab8071e died when Anthropic returned HTTP 200
with body {"type": "error", "error": {"type": "api_error", "message":
"Internal server error", "details": null}}. The SDK didn't raise an
exception (HTTP status was 200), the response wasn't validated, and
the session halted with retryable=false. The request that failed was
small (~4.5k tokens) and had no tool calls — three earlier, larger
requests with tool calls all succeeded. Classic transient server-side
failure that should have been retried.
Both new code paths raise KernelProviderUnavailableError(retryable=True),
feeding into the existing retry_with_backoff loop (up to 5 retries
with exponential backoff).
All 298 existing tests pass.
The previous fix (04a73cb) added two guards that turned out to be unreachable for the actual error path: - Fix 1 (response type check) guards get_final_message() return, but the SDK raises during streaming iteration before returning. - Fix 2 (base APIError catch) is pre-empted by the more specific AnthropicAPIStatusError handler above it. The real path: SDK raises APIStatusError(status_code=200) when the streaming response sends an SSE "event: error". This is caught by the AnthropicAPIStatusError handler where status=200 falls through to the retryable=False catch-all (200 < 500). Add `if status < 400` guard before the fallthrough to treat 2xx/3xx APIStatusErrors as retryable transient failures. Add 18 regression tests covering HTTP 200 errors, other 2xx/3xx status codes, and verification that 4xx behavior is unchanged. Fixes: session 897eb0d4 (HTTP 200 error killed session)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Transient Anthropic API failures kill sessions instead of being retried.
Anthropic returned HTTP 200 with an error body:
```json
{
"type": "error",
"error": {
"type": "api_error",
"message": "Internal server error",
"details": null
}
}
```
The SDK didn't raise an exception (HTTP status was 200), the response wasn't validated, and the session halted immediately with `retryable=false`. The failing request was small (~4.5k tokens) with no tool calls — three earlier, larger requests with tool calls all succeeded fine. Textbook transient server-side failure.
Root Cause
Two gaps in the error handling:
No response validation — after the API call returns, the response is passed directly to `_convert_to_chat_response()` without checking if it's actually a valid message. HTTP 200 with an error body slips through.
Base `APIError` not caught — the exception chain catches specific `APIStatusError` subclasses (`RateLimitError`, `BadRequestError`, etc.) and `APIStatusError` itself, but not the base `APIError`. The SDK can raise `APIConnectionError`, `APITimeoutError`, or other non-status errors that fall through to the generic `Exception` handler.
Fix
Two defensive layers, both feeding into the existing `retry_with_backoff` loop:
1. Response validation (belt-and-suspenders)
After the API call, before returning the response, validate `response.type == "message"`. If not, extract error details and raise `KernelProviderUnavailableError(retryable=True)`.
2. Base `AnthropicAPIError` catch
New exception handler between `AnthropicAPIStatusError` and `asyncio.TimeoutError` that catches the SDK's base `APIError` class. Treats all as transient/retryable.
Testing