Skip to content

Retry transient transport errors in native LLM client - #128

Merged
whatever merged 1 commit into
mainfrom
feat/llm-transport-retries
Aug 5, 2026
Merged

Retry transient transport errors in native LLM client#128
whatever merged 1 commit into
mainfrom
feat/llm-transport-retries

Conversation

@jorge-garcia-le

@jorge-garcia-le jorge-garcia-le commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Broadens the native LLM client's retry handling to cover transient transport failures, not just rate limits.

  • Renames _with_rate_limit_retries_with_retries and retries transient transport errors (reqwest connection/send failures, stalled/aborted streams) alongside rate limits. These never reached a response, so retrying can't double-bill; they share the existing backoff schedule and rate_limit_max_retries cap.
  • Adds transport-classification helpers:
    • _is_transient_transport_error / _TRANSPORT_ERROR_MARKERS — retryable transport failures.
    • _is_definitely_unbilled_transport_error / _PRE_RESPONSE_TRANSPORT_MARKERS — the subset that provably precede generation (connection refused, DNS, TLS, handshake, "error sending request").
  • Allows the aiohttp OpenAI chat/completions fallback to run even while the spend ledger is enforcing when the error is definitely-unbilled (applied consistently in both _achat_with_provider_policy and the streaming path).
  • Treats definitely-unbilled transport errors as unbilled in the spend ledger (_is_definitely_unbilled_error).
  • Logs call/stream failures at WARNING via _format_exc_chain, surfacing the nested reqwest cause instead of the terse top-level message.
  • Drops "glm" from _REASONING_EFFORT_UNSUPPORTED_PATTERNS.

Main's existing reasoning_effort and max_output_tokens rejection-retry branches are preserved and now route through the broadened _with_retries.

Context

These changes originated from local edits made against an older master; this PR reconciles them with main's newer max_output_tokens retry work (the two overlapped). Pricing-table (telemetry.py) changes from the same local batch are intentionally not included here.

Test plan

  • python -m py_compile clearwing/llm/native.py
  • ruff check clearwing/llm/native.py
  • python -m pytest — LLM/native/provider suite: 112 passed (test_llm_openai_fallback, test_llm_spend_budget, test_native_reasoning_effort, test_providers, test_providers_env)

Rename `_with_rate_limit_retries` -> `_with_retries` and broaden it to
retry transient transport failures (reqwest connection/send errors,
stalled/aborted streams) in addition to rate limits. These failures
never reached a response, so retrying them can't double-bill; they share
the rate-limit backoff schedule and retry cap.

Also:
- Add `_is_transient_transport_error` / `_TRANSPORT_ERROR_MARKERS` and
  `_is_definitely_unbilled_transport_error` / `_PRE_RESPONSE_TRANSPORT_MARKERS`.
- Allow the aiohttp OpenAI fallback to run even under budget enforcement
  when the transport error provably preceded generation (connection
  refused, DNS, TLS, "error sending request").
- Treat definitely-unbilled transport errors as unbilled in the spend
  ledger.
- Log call/stream failures at WARNING with `_format_exc_chain` so the
  nested reqwest cause is visible, not just the terse top-level message.
- Drop "glm" from the reasoning_effort-unsupported patterns.

Preserves main's existing reasoning_effort and max_output_tokens retry
branches, now routed through the broadened `_with_retries`.
@whatever
whatever marked this pull request as ready for review August 5, 2026 19:20
@whatever
whatever requested a review from ehartford as a code owner August 5, 2026 19:20
@whatever
whatever merged commit da00357 into main Aug 5, 2026
1 check passed
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.

3 participants