-
Notifications
You must be signed in to change notification settings - Fork 0
feat(agent): make overload (503/529) backoff configurable #129
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -2939,7 +2939,10 @@ def _perform_api_call(next_api_kwargs): | |||||
| } | ||||||
| _should_fallback = ( | ||||||
| is_rate_limited | ||||||
| or (_is_transport_failure and retry_count >= 2) | ||||||
| or (classified.reason == FailoverReason.overloaded | ||||||
| and retry_count >= agent._overload_max_retries) | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 overload_max_retries values 0 and 1 produce identical behavior due to off-by-one in fallback threshold (bug) The overload fallback check uses 💡 Suggestion: Change
Suggested change
📋 Prompt for AI AgentsIn agent/conversation_loop.py line 2943, change |
||||||
| or (classified.reason == FailoverReason.timeout | ||||||
| and retry_count >= 2) | ||||||
| ) | ||||||
| if _should_fallback and agent._fallback_index < len(agent._fallback_chain): | ||||||
| # Don't eagerly fallback if credential pool rotation may | ||||||
|
|
@@ -3822,7 +3825,16 @@ def _perform_api_call(next_api_kwargs): | |||||
| _retry_after = min(float(_ra_raw), 600) | ||||||
| except (TypeError, ValueError): | ||||||
| pass | ||||||
| wait_time = _retry_after if _retry_after else jittered_backoff(retry_count, base_delay=2.0, max_delay=60.0) | ||||||
| if _retry_after: | ||||||
| wait_time = _retry_after | ||||||
| elif classified.reason == FailoverReason.overloaded: | ||||||
| wait_time = jittered_backoff( | ||||||
| retry_count, | ||||||
| base_delay=agent._overload_base_delay, | ||||||
| max_delay=agent._overload_max_delay, | ||||||
| ) | ||||||
| else: | ||||||
| wait_time = jittered_backoff(retry_count, base_delay=2.0, max_delay=60.0) | ||||||
| _backoff_policy = None | ||||||
| if is_rate_limited and not _retry_after: | ||||||
| wait_time, _backoff_policy = adaptive_rate_limit_backoff( | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,159 @@ | ||
| """Tests for configurable overload (503/529) backoff — #55540.""" | ||
|
|
||
| import pytest | ||
| from unittest.mock import MagicMock, patch | ||
|
|
||
|
|
||
| # ── Test: config parsing in agent_init ──────────────────────────────── | ||
|
|
||
|
|
||
| def _make_agent_section(**overrides): | ||
| """Build a minimal _agent_section dict for the overload config block.""" | ||
| return overrides | ||
|
|
||
|
|
||
| def _apply_overload_config(agent, agent_section): | ||
| """Replicate the config-reading logic from agent_init.py.""" | ||
| try: | ||
| _overload_retries = int(agent_section.get("overload_max_retries", 2)) | ||
| _overload_retries = max(_overload_retries, 0) | ||
| except (TypeError, ValueError): | ||
| _overload_retries = 2 | ||
| agent._overload_max_retries = _overload_retries | ||
|
|
||
| try: | ||
| _overload_base = float(agent_section.get("overload_base_delay", 2.0)) | ||
| _overload_base = max(_overload_base, 0.1) | ||
| except (TypeError, ValueError): | ||
| _overload_base = 2.0 | ||
| agent._overload_base_delay = _overload_base | ||
|
|
||
| try: | ||
| _overload_max = float(agent_section.get("overload_max_delay", 60.0)) | ||
| _overload_max = max(_overload_max, 1.0) | ||
| except (TypeError, ValueError): | ||
| _overload_max = 60.0 | ||
| agent._overload_max_delay = _overload_max | ||
|
|
||
|
|
||
| class TestOverloadConfigDefaults: | ||
| """When no overload keys are present, defaults match prior hardcoded values.""" | ||
|
|
||
| def test_defaults_when_absent(self): | ||
| agent = MagicMock() | ||
| _apply_overload_config(agent, _make_agent_section()) | ||
| assert agent._overload_max_retries == 2 | ||
| assert agent._overload_base_delay == 2.0 | ||
| assert agent._overload_max_delay == 60.0 | ||
|
|
||
|
|
||
| class TestOverloadConfigCustom: | ||
| """User-supplied values are respected.""" | ||
|
|
||
| def test_custom_values(self): | ||
| agent = MagicMock() | ||
| _apply_overload_config(agent, _make_agent_section( | ||
| overload_max_retries=5, | ||
| overload_base_delay=10.0, | ||
| overload_max_delay=120.0, | ||
| )) | ||
| assert agent._overload_max_retries == 5 | ||
| assert agent._overload_base_delay == 10.0 | ||
| assert agent._overload_max_delay == 120.0 | ||
|
|
||
| def test_zero_retries_means_immediate_fallback(self): | ||
| agent = MagicMock() | ||
| _apply_overload_config(agent, _make_agent_section(overload_max_retries=0)) | ||
| assert agent._overload_max_retries == 0 | ||
|
|
||
| def test_string_values_are_cast(self): | ||
| agent = MagicMock() | ||
| _apply_overload_config(agent, _make_agent_section( | ||
| overload_max_retries="4", | ||
| overload_base_delay="5.5", | ||
| overload_max_delay="90", | ||
| )) | ||
| assert agent._overload_max_retries == 4 | ||
| assert agent._overload_base_delay == 5.5 | ||
| assert agent._overload_max_delay == 90.0 | ||
|
|
||
|
|
||
| class TestOverloadConfigClamping: | ||
| """Floor clamping prevents nonsensical values.""" | ||
|
|
||
| def test_negative_retries_clamped_to_zero(self): | ||
| agent = MagicMock() | ||
| _apply_overload_config(agent, _make_agent_section(overload_max_retries=-1)) | ||
| assert agent._overload_max_retries == 0 | ||
|
|
||
| def test_base_delay_floor(self): | ||
| agent = MagicMock() | ||
| _apply_overload_config(agent, _make_agent_section(overload_base_delay=0.01)) | ||
| assert agent._overload_base_delay == 0.1 | ||
|
|
||
| def test_max_delay_floor(self): | ||
| agent = MagicMock() | ||
| _apply_overload_config(agent, _make_agent_section(overload_max_delay=0.5)) | ||
| assert agent._overload_max_delay == 1.0 | ||
|
|
||
|
|
||
| class TestOverloadConfigInvalid: | ||
| """Garbage input falls back to defaults.""" | ||
|
|
||
| def test_invalid_retries(self): | ||
| agent = MagicMock() | ||
| _apply_overload_config(agent, _make_agent_section(overload_max_retries="abc")) | ||
| assert agent._overload_max_retries == 2 | ||
|
|
||
| def test_invalid_base_delay(self): | ||
| agent = MagicMock() | ||
| _apply_overload_config(agent, _make_agent_section(overload_base_delay="not_a_number")) | ||
| assert agent._overload_base_delay == 2.0 | ||
|
|
||
| def test_invalid_max_delay(self): | ||
| agent = MagicMock() | ||
| _apply_overload_config(agent, _make_agent_section(overload_max_delay=None)) | ||
| assert agent._overload_max_delay == 60.0 | ||
|
|
||
|
|
||
| # ── Test: fallback threshold logic ──────────────────────────────────── | ||
|
|
||
|
|
||
| class TestOverloadFallbackThreshold: | ||
| """The overload retry threshold uses the configured value.""" | ||
|
|
||
| def _should_fallback_overloaded(self, retry_count, overload_max_retries): | ||
| """Replicate the _should_fallback condition for overloaded.""" | ||
| return retry_count >= overload_max_retries | ||
|
|
||
| def test_default_threshold_triggers_at_2(self): | ||
| assert not self._should_fallback_overloaded(0, 2) | ||
| assert not self._should_fallback_overloaded(1, 2) | ||
| assert self._should_fallback_overloaded(2, 2) | ||
| assert self._should_fallback_overloaded(3, 2) | ||
|
|
||
| def test_zero_threshold_triggers_immediately(self): | ||
| assert self._should_fallback_overloaded(0, 0) | ||
|
|
||
| def test_high_threshold_delays_fallback(self): | ||
| assert not self._should_fallback_overloaded(4, 5) | ||
| assert self._should_fallback_overloaded(5, 5) | ||
|
|
||
|
|
||
| # ── Test: backoff delay uses configured values ──────────────────────── | ||
|
|
||
|
|
||
| class TestOverloadBackoffDelay: | ||
| """jittered_backoff is called with configured base/max for overloaded errors.""" | ||
|
|
||
| def test_custom_delays_passed_to_jittered_backoff(self): | ||
| from agent.retry_utils import jittered_backoff | ||
|
|
||
| delay = jittered_backoff(1, base_delay=10.0, max_delay=30.0) | ||
| assert 10.0 <= delay <= 10.0 * 1.5 # base + up to jitter_ratio * base | ||
|
|
||
| def test_max_delay_caps_high_attempts(self): | ||
| from agent.retry_utils import jittered_backoff | ||
|
|
||
| delay = jittered_backoff(10, base_delay=2.0, max_delay=30.0) | ||
| assert delay <= 30.0 * 1.5 # max + jitter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 overload_max_retries not clamped to api_max_retries, causing silent fallback skip when exceeding the outer loop ceiling (bug)
The config key overload_max_retries (default 2) controls when an overload (503/529) triggers provider fallback. The documentation correctly warns 'Must be <= api_max_retries to take effect (the outer loop is the hard ceiling).' However, the code in agent/agent_init.py:1362-1367 only floors the value to 0 (no upper bound relative to api_max_retries). If a user sets overload_max_retries: 10 while api_max_retries is 3 (or the default), the overload fallback path at conversation_loop.py:2942-2943 never triggers — the outer loop exits at retry_count=3 before retry_count can reach 10. The overload then falls through to the generic 'max_retries exhausted' fallback instead of the targeted overload fallback, losing differentiated behavior. Default values (2 vs 3) happen to work, but any user customization risks this silent mismatch.
💡 Suggestion: Clamp _overload_max_retries to at most _api_max_retries (or _api_max_retries - 1 if the intent is to fire before the loop exhausts). Add
_overload_retries = min(_overload_retries, _api_retries)after line 1364 to enforce the documented constraint.📋 Prompt for AI Agents
In agent/agent_init.py, after line 1364 (
_overload_retries = max(_overload_retries, 0) # 0 = immediate fallback), add a line:_overload_retries = min(_overload_retries, _api_retries) # per doc: must be <= api_max_retries. This ensures the overload-specific eager fallback threshold never exceeds the outer retry loop ceiling. Also add a test to tests/agent/test_overload_backoff_config.py verifying that overload_max_retries=10 with api_max_retries=3 gets clamped to 3.