From bbe0698cc3e6d21c379f73199a17fc4f7bd85f7c Mon Sep 17 00:00:00 2001 From: qbit-mirror-bot Date: Thu, 2 Jul 2026 11:28:02 +0000 Subject: [PATCH] fix(error-classifier): classify HTTP 408 as retryable timeout, not a non-retryable 4xx --- agent/error_classifier.py | 58 +++++ tests/agent/test_error_classifier.py | 131 ++++++++++ .../test_408_request_timeout_loop.py | 235 ++++++++++++++++++ 3 files changed, 424 insertions(+) create mode 100644 tests/run_agent/test_408_request_timeout_loop.py diff --git a/agent/error_classifier.py b/agent/error_classifier.py index 8111880a7ec6..c7529858f07d 100644 --- a/agent/error_classifier.py +++ b/agent/error_classifier.py @@ -110,6 +110,7 @@ def is_auth(self) -> bool: "exceeded your current quota", "account is deactivated", "plan does not include", + "out of extra usage", # Anthropic OAuth Pro/Max overage bucket depleted (HTTP 400) "out of funds", "run out of funds", "balance_depleted", @@ -898,6 +899,41 @@ def _classify_by_status( should_compress=True, ) + if status_code == 408: + # HTTP 408 is a timeout, NOT a permanent client error. Without this + # branch it falls through to the generic "other 4xx -> non-retryable + # format_error" catch-all at the bottom of this function, which aborts + # the turn and persists an empty assistant bubble (the "disappeared + # conversation" / blank-turn symptom). + # + # We classify ALL 408s as a transient ``timeout`` (retryable, NO + # compression). This deliberately covers the GitHub Copilot + # ``user_request_timeout`` / "Timed out reading request body ... use a + # smaller request size" case too, even though that one is nominally + # about request SIZE. Field evidence (long copilot/opus-4.8 session, + # 2026-07-02): the 408 is PROBABILISTIC/jitter in a wide band well + # BELOW the hard prompt ceiling — the same ~785k-token request that + # 408'd once succeeded on the very next attempt at ~786k. The edge + # occasionally reads the large body too slowly and times out; it is + # not a hard "body exceeds the limit" rejection until the prompt + # actually approaches the ceiling (~936k for opus-4.8). So the correct, + # least-destructive recovery is a plain retry (the SAME body usually + # succeeds on the next attempt), NOT auto-compression. + # + # We intentionally do NOT set should_compress here: auto-compaction on + # a 408 would silently delete conversation history the moment a request + # merely jitters, which is a heavy, surprising, user-visible side + # effect for a transient timeout. Genuine "prompt too large for the + # window" is a SEPARATE signal (413 / context_overflow) and stays on + # its own compression path. When retries here are exhausted the loop + # falls back to another provider (transport-failure eager-fallback + # after 2 attempts); the user can always compact deliberately with + # ``/compress`` if a long session keeps timing out. + return result_fn( + FailoverReason.timeout, + retryable=True, + ) + if status_code == 429: # Already checked long_context_tier above. Some providers (notably # Z.AI / Zhipu) reuse HTTP 429 for server-wide overload — same status @@ -963,9 +999,31 @@ def _classify_by_status( retryable=False, should_fallback=True, ) + # Some local inference servers (notably llama.cpp / llama-server) + # report context overflow with an HTTP 500 instead of the standard + # 400/413. The request-validation guard above already ran, so any + # remaining explicit context-overflow signal routes into the + # compression-and-retry path (mirroring _classify_400) instead of + # blind server_error retries that exhaust and drop the turn. + if any(p in error_msg for p in _CONTEXT_OVERFLOW_PATTERNS): + return result_fn( + FailoverReason.context_overflow, + retryable=True, + should_compress=True, + ) return result_fn(FailoverReason.server_error, retryable=True) if status_code in {503, 529}: + # Same overflow-as-5xx variant (server busy / model-load OOM, or a + # Cloudflare/Tailscale hop relabeling the status). Route explicit + # overflow bodies into compression; otherwise treat as transient + # overload and retry. + if any(p in error_msg for p in _CONTEXT_OVERFLOW_PATTERNS): + return result_fn( + FailoverReason.context_overflow, + retryable=True, + should_compress=True, + ) return result_fn(FailoverReason.overloaded, retryable=True) # Other 4xx — non-retryable diff --git a/tests/agent/test_error_classifier.py b/tests/agent/test_error_classifier.py index 16b881861e65..5e2c303f7787 100644 --- a/tests/agent/test_error_classifier.py +++ b/tests/agent/test_error_classifier.py @@ -469,6 +469,44 @@ def test_502_plain_bad_gateway_still_retryable(self): assert result.reason == FailoverReason.server_error assert result.retryable is True + # ── 5xx that are actually context overflow ── + # Some local inference servers (llama.cpp / llama-server, and vLLM/Ollama + # behind a Cloudflare/Tailscale hop) report context overflow with a 5xx + # status instead of the standard 400/413. These must route into the + # compression-and-retry path, not the blind server_error/overloaded retry + # that exhausts and drops the turn. + + @pytest.mark.parametrize("status_code", [500, 502, 503, 529]) + def test_5xx_context_overflow_routes_to_compression(self, status_code): + """Explicit context-overflow wording on any of the codes the fix covers + (500/502/503/529) must route to context_overflow + compression, not a + blind server_error/overloaded retry. Covers all four branches the code + touches (the original PR only asserted 500 and 503).""" + e = MockAPIError( + "Context size has been exceeded.", + status_code=status_code, + body={"error": {"code": status_code, "message": "Context size has been exceeded.", "type": "server_error"}}, + ) + result = classify_api_error(e) + assert result.reason == FailoverReason.context_overflow + assert result.should_compress is True + assert result.retryable is True + + def test_500_plain_server_error_not_compressed(self): + """A genuine 500 crash without overflow wording must NOT be swallowed + into compression — it stays a retryable server_error.""" + e = MockAPIError("Internal Server Error", status_code=500) + result = classify_api_error(e) + assert result.reason == FailoverReason.server_error + assert result.should_compress is False + + def test_503_plain_overloaded_not_compressed(self): + """A genuine 503 overload without overflow wording stays overloaded.""" + e = MockAPIError("Service Unavailable", status_code=503) + result = classify_api_error(e) + assert result.reason == FailoverReason.overloaded + assert result.should_compress is False + # ── Model not found ── def test_404_model_not_found(self): @@ -1295,6 +1333,25 @@ def test_400_with_billing_text(self): result = classify_api_error(e) assert result.reason == FailoverReason.billing + def test_400_anthropic_extra_usage_exhausted(self): + """Anthropic returns 400 with 'out of extra usage' when the user's + extra-usage allowance is depleted. Must classify as billing so the + fallback chain engages (with credential rotation) instead of the + generic format_error path, which never rotates. (#11736, #13170)""" + e = MockAPIError( + "You're out of extra usage. Add more at claude.ai/settings/usage and keep going.", + status_code=400, + body={"error": { + "type": "invalid_request_error", + "message": "You're out of extra usage. Add more at claude.ai/settings/usage and keep going.", + }}, + ) + result = classify_api_error(e, provider="anthropic") + assert result.reason == FailoverReason.billing + assert result.should_fallback is True + assert result.retryable is False + assert result.should_rotate_credential is True + def test_200_with_error_body(self): """200 status with error in body — should be unknown, not crash.""" class WeirdSuccess(Exception): @@ -1824,3 +1881,77 @@ def test_overload_429_takes_precedence_over_upstream(self): # Overload disambiguation runs first; the outer message is the overload # phrase, so this is an overload, not an upstream rate-limit. assert result.reason == FailoverReason.overloaded + + +# ── HTTP 408 request timeout ──────────────────────────────────────────── + +class Test408RequestTimeout: + """HTTP 408 must never fall through to the non-retryable 'other 4xx' + bucket (that abort persists an empty assistant turn — the "disappeared + conversation" / blank-bubble symptom). ALL 408s are classified as a transient + ``timeout``: retryable, and explicitly NOT should_compress. + + Design decision (field 2026-07-02): even the GitHub Copilot + ``user_request_timeout`` / "Timed out reading request body ... use a + smaller request size" case is a plain retry, NOT auto-compression. Real + data showed the 408 is probabilistic jitter well below the hard prompt + ceiling — the same ~785k-token request that 408'd once succeeded on the + next attempt at ~786k — so retrying the same body usually works, and + auto-compaction would silently delete conversation history for a merely + transient timeout. Genuine over-window prompts surface as 413 / + context_overflow (their own compression path); users compact 408-prone + long sessions deliberately via ``/compress``. + """ + + def test_copilot_oversized_body_408_retries_as_timeout_not_compress(self): + # The exact shape GitHub Copilot returns on a long session. It must + # retry (timeout), and must NOT auto-compress. + e = MockAPIError( + "Error code: 408 - {'error': {'message': 'Timed out reading " + "request body. Try again, or use a smaller request size.', " + "'code': 'user_request_timeout'}}", + status_code=408, + body={"error": {"message": "Timed out reading request body. " + "Try again, or use a smaller request size.", + "code": "user_request_timeout"}}, + ) + result = classify_api_error(e, provider="copilot", model="claude-opus-4.8") + assert result.reason == FailoverReason.timeout + assert result.retryable is True + assert result.should_compress is False + + def test_408_never_auto_compresses(self): + # Hard guard on the user's explicit preference: a 408 must NEVER + # trigger auto-compaction (which would delete history unprompted). + # This must FAIL if anyone re-routes 408 to payload_too_large. + for msg, body in [ + ("Timed out reading request body. Use a smaller request size.", {}), + ("Request timed out.", {"error": {"code": "user_request_timeout"}}), + ("Request Timeout", {}), + ]: + e = MockAPIError(msg, status_code=408, body=body) + result = classify_api_error(e, provider="copilot", model="claude-opus-4.8") + assert result.should_compress is False, msg + assert result.reason != FailoverReason.payload_too_large, msg + + def test_oversized_body_408_is_not_non_retryable_format_error(self): + # Falsification guard: if the 408 branch is removed, this 408 would + # be classified as a non-retryable format_error and the turn would + # abort into a blank bubble. This assertion must FAIL on buggy code. + e = MockAPIError( + "Timed out reading request body. Try again, or use a smaller " + "request size.", + status_code=408, + ) + result = classify_api_error(e, provider="copilot", model="claude-opus-4.8") + assert result.retryable is True + assert result.reason != FailoverReason.format_error + + def test_plain_408_is_transient_timeout(self): + # A generic gateway/request timeout must retry as a transport timeout. + e = MockAPIError("Request Timeout", status_code=408) + result = classify_api_error(e, provider="openai", model="gpt-5.5") + assert result.reason == FailoverReason.timeout + assert result.retryable is True + assert result.should_compress is False + diff --git a/tests/run_agent/test_408_request_timeout_loop.py b/tests/run_agent/test_408_request_timeout_loop.py new file mode 100644 index 000000000000..25e161ac41b3 --- /dev/null +++ b/tests/run_agent/test_408_request_timeout_loop.py @@ -0,0 +1,235 @@ +"""Loop-level tests for HTTP 408 request-timeout recovery in AIAgent. + +Symptom-level companion to tests/agent/test_error_classifier.py::Test408RequestTimeout +(which asserts the *classification*). These drive the REAL run_conversation loop +and assert the *turn outcome*: a 408 must be retried as a transient timeout and +produce a real assistant turn — NOT abort into an empty assistant bubble (the +"disappeared conversation" / blank-turn symptom), and NOT silently compress +away conversation history. + +Regression guarded: before agent/error_classifier.py grew a `408` branch, a 408 +fell through to the generic "other 4xx -> non-retryable format_error" abort, +which persisted an empty assistant turn (blank bubble). The fix classifies 408 +as `timeout` (retryable, NOT should_compress) so the loop retries the SAME +request — jitter-type 408s from GitHub Copilot near the prompt-size band clear +on the next attempt without destroying history. +""" + +import pytest + +from types import SimpleNamespace +from unittest.mock import patch, MagicMock + +from run_agent import AIAgent +import run_agent + + +@pytest.fixture(autouse=True) +def _no_retry_sleep(monkeypatch): + """Short-circuit retry backoff so the loop tests run fast.""" + import time as _time + monkeypatch.setattr(_time, "sleep", lambda *_a, **_k: None) + monkeypatch.setattr(run_agent, "jittered_backoff", lambda *a, **k: 0.0) + + +def _make_tool_defs(*names: str) -> list: + return [ + { + "type": "function", + "function": { + "name": n, + "description": f"{n} tool", + "parameters": {"type": "object", "properties": {}}, + }, + } + for n in names + ] + + +def _mock_response(content="Hello", finish_reason="stop", tool_calls=None): + msg = SimpleNamespace( + content=content, + tool_calls=tool_calls, + reasoning_content=None, + reasoning=None, + ) + choice = SimpleNamespace(message=msg, finish_reason=finish_reason) + resp = SimpleNamespace(choices=[choice], model="test/model") + resp.usage = None + return resp + + +def _make_408_error(*, message="Request Timeout", use_status_code=True, oversized=False): + """Create an exception mimicking an HTTP 408. + + oversized=True reproduces GitHub Copilot's user_request_timeout body + ("Timed out reading request body ... use a smaller request size"). + """ + if oversized: + message = ("Error code: 408 - {'error': {'message': 'Timed out reading " + "request body. Try again, or use a smaller request size.', " + "'code': 'user_request_timeout'}}") + err = Exception(message) + if use_status_code: + err.status_code = 408 + return err + + +@pytest.fixture() +def agent(): + with ( + patch("run_agent.get_tool_definitions", return_value=_make_tool_defs("web_search")), + patch("run_agent.check_toolset_requirements", return_value={}), + patch("run_agent.OpenAI"), + ): + a = AIAgent( + api_key="test-key-1234567890", + base_url="https://api.githubcopilot.com", + quiet_mode=True, + skip_context_files=True, + skip_memory=True, + ) + a.client = MagicMock() + a._cached_system_prompt = "You are helpful." + a._use_prompt_caching = False + a.tool_delay = 0 + a.compression_enabled = True # prove we DON'T compress even when enabled + a.save_trajectories = False + return a + + +class TestHTTP408Loop: + """A 408 must retry-and-recover into a real turn, never abort blank, + never auto-compress.""" + + def test_408_recovers_into_real_turn_not_blank_bubble(self, agent): + """First call 408s, second succeeds → a real assistant answer, no abort.""" + err_408 = _make_408_error() + ok_resp = _mock_response(content="Recovered after 408 retry", finish_reason="stop") + agent.client.chat.completions.create.side_effect = [err_408, ok_resp] + + prefill = [ + {"role": "user", "content": "previous question"}, + {"role": "assistant", "content": "previous answer"}, + ] + + with ( + patch.object(agent, "_persist_session"), + patch.object(agent, "_save_trajectory"), + patch.object(agent, "_cleanup_task_resources"), + ): + result = agent.run_conversation("hello", conversation_history=prefill) + + # Symptom assertion: 408 did NOT abort into a failed/blank turn. + assert result.get("failed") is not True + assert result["completed"] is True + assert result["final_response"] == "Recovered after 408 retry" + + def test_408_does_not_compress_history(self, agent): + """A 408 must NOT call _compress_context — history stays intact.""" + err_408 = _make_408_error() + ok_resp = _mock_response(content="OK", finish_reason="stop") + agent.client.chat.completions.create.side_effect = [err_408, ok_resp] + + prefill = [ + {"role": "user", "content": "previous question"}, + {"role": "assistant", "content": "previous answer"}, + ] + + with ( + patch.object(agent, "_compress_context") as mock_compress, + patch.object(agent, "_persist_session"), + patch.object(agent, "_save_trajectory"), + patch.object(agent, "_cleanup_task_resources"), + ): + result = agent.run_conversation("hello", conversation_history=prefill) + + # The core user-requested guarantee: 408 never silently compacts. + mock_compress.assert_not_called() + assert result["completed"] is True + + def test_408_retries_same_request_not_shrunk(self, agent): + """Retry after 408 must resend the SAME request (plain retry), not a + compressed/shrunk one — proving it took the timeout path, not the + payload_too_large/compression path.""" + err_408 = _make_408_error() + ok_resp = _mock_response(content="OK", finish_reason="stop") + + request_payloads = [] + + def _side_effect(**kwargs): + request_payloads.append(kwargs) + if len(request_payloads) == 1: + raise err_408 + return ok_resp + + agent.client.chat.completions.create.side_effect = _side_effect + + prefill = [ + {"role": "user", "content": "previous question"}, + {"role": "assistant", "content": "previous answer"}, + ] + + with ( + patch.object(agent, "_compress_context") as mock_compress, + patch.object(agent, "_persist_session"), + patch.object(agent, "_save_trajectory"), + patch.object(agent, "_cleanup_task_resources"), + ): + result = agent.run_conversation("hello", conversation_history=prefill) + + assert result["completed"] is True + assert len(request_payloads) == 2 + mock_compress.assert_not_called() + # Same request body length on retry (no compression shrank it). + assert len(request_payloads[1]["messages"]) == len(request_payloads[0]["messages"]) + + def test_oversized_body_408_also_retries_without_compression(self, agent): + """The Copilot 'reading request body / smaller request size' 408 must + ALSO retry-not-compress — it is jitter near the size band, not a hard + overflow. (This is the exact case the user challenged: 'why must 408 + compress?' — answer: it must not.)""" + err_408 = _make_408_error(oversized=True) + ok_resp = _mock_response(content="Recovered oversized 408", finish_reason="stop") + agent.client.chat.completions.create.side_effect = [err_408, ok_resp] + + prefill = [ + {"role": "user", "content": "previous question"}, + {"role": "assistant", "content": "previous answer"}, + ] + + with ( + patch.object(agent, "_compress_context") as mock_compress, + patch.object(agent, "_persist_session"), + patch.object(agent, "_save_trajectory"), + patch.object(agent, "_cleanup_task_resources"), + ): + result = agent.run_conversation("hello", conversation_history=prefill) + + mock_compress.assert_not_called() + assert result.get("failed") is not True + assert result["completed"] is True + assert result["final_response"] == "Recovered oversized 408" + + def test_408_via_message_string_without_status_code(self, agent): + """A 408 surfaced only via message text (no status_code attr) must + still recover, not abort.""" + err = _make_408_error(use_status_code=False, message="error code: 408 Request Timeout") + ok_resp = _mock_response(content="OK", finish_reason="stop") + agent.client.chat.completions.create.side_effect = [err, ok_resp] + + prefill = [ + {"role": "user", "content": "previous question"}, + {"role": "assistant", "content": "previous answer"}, + ] + + with ( + patch.object(agent, "_compress_context") as mock_compress, + patch.object(agent, "_persist_session"), + patch.object(agent, "_save_trajectory"), + patch.object(agent, "_cleanup_task_resources"), + ): + result = agent.run_conversation("hello", conversation_history=prefill) + + mock_compress.assert_not_called() + assert result["completed"] is True