Merge PR #223: fix: correct off-by-one in retry exhaustion checks
Authored by Farukest. Fixes #222.
This commit is contained in:
commit
e27e3a4f8a
2 changed files with 48 additions and 2 deletions
|
|
@ -3067,7 +3067,7 @@ class AIAgent:
|
||||||
print(f"{self.log_prefix} 📝 Provider message: {error_msg[:200]}")
|
print(f"{self.log_prefix} 📝 Provider message: {error_msg[:200]}")
|
||||||
print(f"{self.log_prefix} ⏱️ Response time: {api_duration:.2f}s (fast response often indicates rate limiting)")
|
print(f"{self.log_prefix} ⏱️ Response time: {api_duration:.2f}s (fast response often indicates rate limiting)")
|
||||||
|
|
||||||
if retry_count > max_retries:
|
if retry_count >= max_retries:
|
||||||
print(f"{self.log_prefix}❌ Max retries ({max_retries}) exceeded for invalid responses. Giving up.")
|
print(f"{self.log_prefix}❌ Max retries ({max_retries}) exceeded for invalid responses. Giving up.")
|
||||||
logging.error(f"{self.log_prefix}Invalid API response after {max_retries} retries.")
|
logging.error(f"{self.log_prefix}Invalid API response after {max_retries} retries.")
|
||||||
self._persist_session(messages, conversation_history)
|
self._persist_session(messages, conversation_history)
|
||||||
|
|
@ -3339,7 +3339,7 @@ class AIAgent:
|
||||||
"partial": True
|
"partial": True
|
||||||
}
|
}
|
||||||
|
|
||||||
if retry_count > max_retries:
|
if retry_count >= max_retries:
|
||||||
print(f"{self.log_prefix}❌ Max retries ({max_retries}) exceeded. Giving up.")
|
print(f"{self.log_prefix}❌ Max retries ({max_retries}) exceeded. Giving up.")
|
||||||
logging.error(f"{self.log_prefix}API call failed after {max_retries} retries. Last error: {api_error}")
|
logging.error(f"{self.log_prefix}API call failed after {max_retries} retries. Last error: {api_error}")
|
||||||
logging.error(f"{self.log_prefix}Request details - Messages: {len(api_messages)}, Approx tokens: {approx_tokens:,}")
|
logging.error(f"{self.log_prefix}Request details - Messages: {len(api_messages)}, Approx tokens: {approx_tokens:,}")
|
||||||
|
|
|
||||||
|
|
@ -758,3 +758,49 @@ class TestRunConversation:
|
||||||
)
|
)
|
||||||
result = agent.run_conversation("search something")
|
result = agent.run_conversation("search something")
|
||||||
mock_compress.assert_called_once()
|
mock_compress.assert_called_once()
|
||||||
|
|
||||||
|
|
||||||
|
class TestRetryExhaustion:
|
||||||
|
"""Regression: retry_count > max_retries was dead code (off-by-one).
|
||||||
|
|
||||||
|
When retries were exhausted the condition never triggered, causing
|
||||||
|
the loop to exit and fall through to response.choices[0] on an
|
||||||
|
invalid response, raising IndexError.
|
||||||
|
"""
|
||||||
|
|
||||||
|
def _setup_agent(self, agent):
|
||||||
|
agent._cached_system_prompt = "You are helpful."
|
||||||
|
agent._use_prompt_caching = False
|
||||||
|
agent.tool_delay = 0
|
||||||
|
agent.compression_enabled = False
|
||||||
|
agent.save_trajectories = False
|
||||||
|
|
||||||
|
def test_invalid_response_returns_error_not_crash(self, agent):
|
||||||
|
"""Exhausted retries on invalid (empty choices) response must not IndexError."""
|
||||||
|
self._setup_agent(agent)
|
||||||
|
# Return response with empty choices every time
|
||||||
|
bad_resp = SimpleNamespace(
|
||||||
|
choices=[],
|
||||||
|
model="test/model",
|
||||||
|
usage=None,
|
||||||
|
)
|
||||||
|
agent.client.chat.completions.create.return_value = bad_resp
|
||||||
|
with (
|
||||||
|
patch.object(agent, "_persist_session"),
|
||||||
|
patch.object(agent, "_save_trajectory"),
|
||||||
|
patch.object(agent, "_cleanup_task_resources"),
|
||||||
|
):
|
||||||
|
result = agent.run_conversation("hello")
|
||||||
|
assert result.get("failed") is True or result.get("completed") is False
|
||||||
|
|
||||||
|
def test_api_error_raises_after_retries(self, agent):
|
||||||
|
"""Exhausted retries on API errors must raise, not fall through."""
|
||||||
|
self._setup_agent(agent)
|
||||||
|
agent.client.chat.completions.create.side_effect = RuntimeError("rate limited")
|
||||||
|
with (
|
||||||
|
patch.object(agent, "_persist_session"),
|
||||||
|
patch.object(agent, "_save_trajectory"),
|
||||||
|
patch.object(agent, "_cleanup_task_resources"),
|
||||||
|
):
|
||||||
|
with pytest.raises(RuntimeError, match="rate limited"):
|
||||||
|
agent.run_conversation("hello")
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue