Skip to content

fix(error-classifier): classify HTTP 408 as retryable timeout, not a non-retryable 4xx - #753

Open
hashbender wants to merge 1 commit into
mainfrom
mirror/pr-56932
Open

fix(error-classifier): classify HTTP 408 as retryable timeout, not a non-retryable 4xx#753
hashbender wants to merge 1 commit into
mainfrom
mirror/pr-56932

Conversation

@hashbender

Copy link
Copy Markdown
Owner

Problem

On a long session against GitHub Copilot, the chat turn would occasionally end
as a blank/empty assistant message (no content) with a visible
Error: HTTP 408: Timed out reading request body. Try again, or use a smaller request size. The turn aborted instead of recovering.

Root cause

agent/error_classifier.py::_classify_by_status had no branch for HTTP 408.
It fell through to the generic tail:

# Other 4xx — non-retryable
if 400 <= status_code < 500:
    return result_fn(FailoverReason.format_error, retryable=False, should_fallback=True)

But 408 is a timeout, not a permanent client error. GitHub Copilot returns
408 {'code': 'user_request_timeout', 'message': 'Timed out reading request body. ... use a smaller request size.'} when a large prompt is slow to read.
Classified as non-retryable format_error, the loop took the client-error abort
path and persisted an empty assistant turn (the blank bubble).

Fix

Add an explicit 408 branch classifying all 408s as a transient timeout
(retryable=True, not should_compress). The loop's existing transport
retry + eager-fallback-after-2-attempts path then recovers the turn.

Why NOT route it to compression (payload_too_large)

The 408 message literally says "use a smaller request size", so compressing like
a 413 is tempting — but field data argued against it. On a long copilot/opus-4.8
session the 408 was probabilistic jitter well below the hard prompt ceiling:
the same ~785k-token request that 408'd once succeeded on the very next attempt
at ~786k
. Retrying the same body usually works, so auto-compaction on a 408
would silently delete conversation history for a merely transient timeout.
Genuine over-window prompts already surface as 413 / context_overflow and keep
their own compression path; a user can compact a 408-prone long session
deliberately via /compress.

Testing

  • Classification (tests/agent/test_error_classifier.py::Test408RequestTimeout):
    oversized-body 408 and plain 408 both → timeout / retryable / not-compress,
    plus a test_408_never_auto_compresses guard.
  • Loop-level (tests/run_agent/test_408_request_timeout_loop.py): a 408 in
    the real run_conversation loop retries into a real assistant turn (no blank
    turn), never calls _compress_context, and resends the same request body.
  • Mutation-verified: deleting the 408 branch makes both suites fail with
    format_error/non-retryable (reproducing the blank-turn regression),
    confirming the tests guard the symptom rather than being tautological.

Full suite green on latest main: tests/agent/test_error_classifier.py +
tests/run_agent/test_408_request_timeout_loop.py (188 passed), and the sibling
tests/run_agent/test_413_compression.py unaffected.


Mirror-of: NousResearch#56932
NousResearch#56932

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant