Skip to content

Commit 1cf718c

Browse files
committed
fix(reliability,observability,cli): retry adapter-wrapped transient errors, bound real timeouts, expose reliability state
1 parent 8515187 commit 1cf718c

14 files changed

Lines changed: 767 additions & 46 deletions

File tree

docs/observability/reliability.md

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,15 @@ async def call_api_async() -> str:
108108
`is_transient_error` returns `True` for:
109109
- `ConnectionError`, `ConnectionResetError`, `OSError`
110110
- `TimeoutError`, `asyncio.TimeoutError`, `EffGenTimeoutError`
111-
- HTTP 429 (rate-limited) via `.status_code` or `.response.status_code`
112-
- HTTP 5xx (server errors)
111+
- Any error carrying a structured `.error_context` (every provider adapter
112+
wraps a real SDK failure this way, so a live 429/5xx from any provider
113+
classifies correctly even after wrapping — see `effgen.models.errors`)
114+
- A raw, never-wrapped SDK exception with HTTP 429 or 5xx via `.status_code`
115+
or `.response.status_code`
116+
- `httpx` network/timeout errors
117+
118+
A plain application exception with none of the above signals is *not*
119+
retried — retrying a bug in your own code will not fix it.
113120

114121
### Delay computation
115122

effgen/cli/_main.py

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3526,12 +3526,22 @@ def _handle_doctor_command(args) -> int:
35263526
# System / CUDA / vLLM / pip-check report.
35273527
system_report = _doctor_system_report(include_pip_check=live)
35283528

3529+
# Circuit-breaker/bulkhead state for any provider that has been routed
3530+
# through effgen.reliability middleware this process — surfaces an open
3531+
# circuit or a saturated bulkhead without the caller instrumenting their
3532+
# own code.
3533+
reliability_report = _doctor_reliability_report()
3534+
35293535
# Exit nonzero if a live probe was requested and a keyed provider failed.
35303536
# Computed once so every output format (JSON and human) agrees.
35313537
exit_code = _doctor_exit_code(results, live)
35323538

35333539
if getattr(args, 'output_json', False):
3534-
print(_json.dumps({"providers": results, "system": system_report}, indent=2))
3540+
print(_json.dumps({
3541+
"providers": results,
3542+
"system": system_report,
3543+
"reliability": reliability_report,
3544+
}, indent=2))
35353545
return exit_code
35363546

35373547
# Pretty-print
@@ -3585,6 +3595,32 @@ def _handle_doctor_command(args) -> int:
35853595
sys_table.add_row(k, str(v))
35863596
console.print(sys_table)
35873597

3598+
# Reliability section — only shown once a provider has actually been
3599+
# routed through circuit-breaker/bulkhead middleware this process.
3600+
if reliability_report:
3601+
console.print("\n[bold cyan]Reliability[/bold cyan]")
3602+
rel_table = Table(show_header=True)
3603+
rel_table.add_column("Provider", style="cyan", no_wrap=True)
3604+
rel_table.add_column("Circuit", style="white")
3605+
rel_table.add_column("Bulkhead", style="white")
3606+
for prov, rec in sorted(reliability_report.items()):
3607+
cb = rec.get("circuit_breaker")
3608+
bh = rec.get("bulkhead")
3609+
if cb is None:
3610+
circuit_cell = "[dim]—[/dim]"
3611+
elif cb["state"] == "closed":
3612+
circuit_cell = "[green]closed[/green]"
3613+
elif cb["state"] == "half_open":
3614+
circuit_cell = "[yellow]half_open[/yellow]"
3615+
else:
3616+
circuit_cell = "[red]open[/red]"
3617+
if bh is None:
3618+
bulkhead_cell = "[dim]—[/dim]"
3619+
else:
3620+
bulkhead_cell = f"active={bh['active']}/{bh['max_concurrency']}, queued={bh['queued']}/{bh['queue_size']}"
3621+
rel_table.add_row(prov, circuit_cell, bulkhead_cell)
3622+
console.print(rel_table)
3623+
35883624
# Print hints for missing keys
35893625
missing = [p for p, i in results.items() if not i.get("available")]
35903626
if missing:
@@ -3609,6 +3645,17 @@ def _handle_doctor_command(args) -> int:
36093645
print("\nSystem:")
36103646
for k, v in system_report.items():
36113647
print(f" {k}: {v}")
3648+
if reliability_report:
3649+
print("\nReliability:")
3650+
for prov, rec in sorted(reliability_report.items()):
3651+
cb = rec.get("circuit_breaker")
3652+
bh = rec.get("bulkhead")
3653+
circuit_str = cb["state"] if cb else "—"
3654+
bulkhead_str = (
3655+
f"active={bh['active']}/{bh['max_concurrency']}, queued={bh['queued']}/{bh['queue_size']}"
3656+
if bh else "—"
3657+
)
3658+
print(f" {prov:12s} circuit={circuit_str:10s} bulkhead={bulkhead_str}")
36123659
missing = [p for p, i in results.items() if not i.get("available")]
36133660
if missing:
36143661
print("\nMissing keys — set in ~/.effgen/.env or export:")
@@ -3635,6 +3682,27 @@ def _doctor_exit_code(results: dict[str, dict], live: bool) -> int:
36353682
return 0
36363683

36373684

3685+
def _doctor_reliability_report() -> dict[str, dict]:
3686+
"""Circuit-breaker/bulkhead state for providers routed through reliability
3687+
middleware this process, keyed by provider name (empty if none have).
3688+
3689+
A provider only appears once ``ProviderRegistry.get_circuit_breaker``/
3690+
``get_bulkhead`` has been used for it — no calls yet made means no state
3691+
to report, which is the common case for a fresh CLI invocation.
3692+
"""
3693+
try:
3694+
from effgen.models.registry import ProviderRegistry
3695+
3696+
stats = ProviderRegistry.reliability_stats()
3697+
except Exception:
3698+
return {}
3699+
return {
3700+
prov: rec
3701+
for prov, rec in stats.items()
3702+
if rec.get("circuit_breaker") is not None or rec.get("bulkhead") is not None
3703+
}
3704+
3705+
36383706
def _doctor_system_report(*, include_pip_check: bool = False) -> dict[str, Any]:
36393707
"""Collect a CUDA / torch / vLLM / pip-check diagnostic snapshot."""
36403708
report: dict[str, Any] = {}

effgen/models/_rate_limit.py

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,19 @@ async def call():
5050

5151

5252
class RateLimitExceeded(Exception):
53-
"""Raised when the daily budget for a model is exhausted."""
53+
"""Raised when the daily budget for a model is exhausted.
54+
55+
Carries the same structured ``.error_context`` shape as
56+
:mod:`effgen.models.errors`'s typed provider errors (category
57+
``"rate_limited"``) so ``is_transient_error()``/``classify_provider_error()``
58+
classify it consistently with every other rate-limit signal.
59+
"""
60+
61+
def __init__(self, message: str) -> None:
62+
super().__init__(message)
63+
from effgen.models.errors import error_context_dict
64+
65+
self.error_context = error_context_dict("", "", "request", "rate_limited")
5466

5567

5668
@dataclass

effgen/models/routing/retry.py

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,29 @@
66
- ``RateLimitExceeded`` — provider rate-limited; back off and try again.
77
- ``ProviderTransientError`` — 5xx from provider; may resolve on retry.
88
- ``ModelTimeoutError`` — prediction timed out; worth retrying.
9+
- ``BudgetExceededError`` — not retried against the *same* provider, but
10+
treated as retriable here because the router's meaning of "retriable" is
11+
"worth trying the next candidate" (e.g. a free-tier fallback), not "the
12+
same call might succeed if repeated."
913
1014
Do NOT retry on (permanent, operator-action required):
1115
- ``ModelAuthError`` — bad/missing API key.
1216
- ``ModelRefusalError`` — model refused; retrying will produce the same result.
1317
- ``InvalidRequestError`` — malformed request; retrying will always fail.
1418
19+
Relationship to ``effgen.models.errors.classify_provider_error`` /
20+
``effgen.reliability.retry.is_transient_error``: those answer "is retrying
21+
*this exact call* worth it", so an unrecognized exception defaults to
22+
retryable (a genuine transient blip should not become a hard failure) and
23+
``BudgetExceededError`` defaults to *not* retryable (repeating the same call
24+
against the same provider/budget will not succeed). ``RetryPolicy`` answers a
25+
related but distinct question — "should the router try the next candidate" —
26+
so it deliberately keeps its own explicit allow/deny lists rather than
27+
delegating: an unrecognized exception here defaults to *not* retriable
28+
(fail closed on an unknown failure rather than failing over blindly), and
29+
``BudgetExceededError`` is retriable here specifically because failing over
30+
to a different, cheaper/free-tier candidate is exactly the useful response.
31+
1532
Usage::
1633
1734
from effgen.models.routing.retry import RetryPolicy
@@ -104,7 +121,13 @@ def __init__(
104121
# ------------------------------------------------------------------
105122

106123
def is_retriable(self, exc: Exception) -> bool:
107-
"""Return True if *exc* is safe to retry."""
124+
"""Return True if *exc* is worth trying the next candidate for.
125+
126+
Uses this module's own allow/deny lists rather than
127+
``classify_provider_error`` — see the module docstring for why the
128+
two intentionally disagree on ``BudgetExceededError`` and on
129+
unrecognized exceptions.
130+
"""
108131
if isinstance(exc, _NON_RETRIABLE):
109132
return False
110133
return isinstance(exc, _RETRIABLE)

effgen/models/transformers_engine.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,21 @@
4848
logger = logging.getLogger(__name__)
4949

5050

51+
def _reraise_if_classified(exc: Exception) -> None:
52+
"""Re-raise *exc* unwrapped when it already carries retry classification.
53+
54+
A timeout raised by ``effgen.reliability.timeouts.with_timeout()`` around
55+
a local generate call must propagate as-is instead of being flattened
56+
into a generic ``RuntimeError`` by the callers' blanket exception
57+
handlers below — flattening discards the type information
58+
``is_transient_error()`` relies on to retry it correctly.
59+
"""
60+
from effgen.reliability.timeouts import TimeoutError as EffGenTimeoutError
61+
62+
if isinstance(exc, EffGenTimeoutError):
63+
raise exc
64+
65+
5166
# Native tool-call delimiters that the downstream parser (core.tool_calling)
5267
# needs to see; these must survive the special-token strip on the tool path.
5368
_TOOL_CALL_DELIMITERS = frozenset({
@@ -716,6 +731,7 @@ def generate(
716731

717732
except Exception as e:
718733
logger.error(f"Generation failed: {e}")
734+
_reraise_if_classified(e)
719735
raise RuntimeError(f"Generation failed: {e}") from e
720736
finally:
721737
self._tokenizer_lock.release()
@@ -849,6 +865,7 @@ def _generate_with_error_capture() -> None:
849865

850866
except Exception as e:
851867
logger.error(f"Streaming generation failed: {e}")
868+
_reraise_if_classified(e)
852869
raise RuntimeError(f"Streaming generation failed: {e}") from e
853870

854871
def generate_batch(
@@ -941,6 +958,7 @@ def generate_batch(
941958

942959
except Exception as e:
943960
logger.error(f"Batch generation failed: {e}")
961+
_reraise_if_classified(e)
944962
raise RuntimeError(f"Batch generation failed: {e}") from e
945963
finally:
946964
self._tokenizer_lock.release()

0 commit comments

Comments
 (0)