fix: correct off-by-one in retry exhaustion checks #223
Merged
Conversation
The retry exhaustion checks used > instead of >= to compare retry_count against max_retries. Since the while loop condition is retry_count < max_retries, the check retry_count > max_retries can never be true inside the loop. When retries are exhausted, the loop exits and falls through to response.choices[0] on an invalid response, crashing with IndexError instead of returning a proper error.
teknium1
added a commit
that referenced
this pull request
Mar 2, 2026
The TestRetryExhaustion tests from PR #223 didn't mock time.sleep/time.time, causing the retry backoff loops (275s+ total) to run in real time. Tests would time out instead of running quickly. Added _make_fast_time_mock() helper that creates a mock time module where time.time() advances 500s per call (so sleep_end is always in the past) and time.sleep() is a no-op. Both tests now complete in <1s.
Contributor
|
Merged in e27e3a4 — thanks for the fix and the clear write-up! One follow-up in 234b67f: added |
Contributor
Author
|
Thanks! Good catch on the time mocking for the retry tests |
angelburgosrosado
pushed a commit
to angelburgosrosado/hermes-agent
that referenced
this pull request
Apr 27, 2026
…n checks Authored by Farukest. Fixes NousResearch#222.
angelburgosrosado
pushed a commit
to angelburgosrosado/hermes-agent
that referenced
this pull request
Apr 27, 2026
The TestRetryExhaustion tests from PR NousResearch#223 didn't mock time.sleep/time.time, causing the retry backoff loops (275s+ total) to run in real time. Tests would time out instead of running quickly. Added _make_fast_time_mock() helper that creates a mock time module where time.time() advances 500s per call (so sleep_end is always in the past) and time.sleep() is a no-op. Both tests now complete in <1s.
1 task
olympus-terminal
pushed a commit
to olympus-terminal/hermes-agent
that referenced
this pull request
May 16, 2026
…n checks Authored by Farukest. Fixes NousResearch#222.
olympus-terminal
pushed a commit
to olympus-terminal/hermes-agent
that referenced
this pull request
May 16, 2026
The TestRetryExhaustion tests from PR NousResearch#223 didn't mock time.sleep/time.time, causing the retry backoff loops (275s+ total) to run in real time. Tests would time out instead of running quickly. Added _make_fast_time_mock() helper that creates a mock time module where time.time() advances 500s per call (so sleep_end is always in the past) and time.sleep() is a no-op. Both tests now complete in <1s.
Egavasyug
pushed a commit
to Egavasyug/hermes-agent
that referenced
this pull request
Jun 10, 2026
…n checks Authored by Farukest. Fixes NousResearch#222.
Egavasyug
pushed a commit
to Egavasyug/hermes-agent
that referenced
this pull request
Jun 10, 2026
The TestRetryExhaustion tests from PR NousResearch#223 didn't mock time.sleep/time.time, causing the retry backoff loops (275s+ total) to run in real time. Tests would time out instead of running quickly. Added _make_fast_time_mock() helper that creates a mock time module where time.time() advances 500s per call (so sleep_end is always in the past) and time.sleep() is a no-op. Both tests now complete in <1s.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When API retries are exhausted due to invalid responses (empty/null
choices) or repeated errors, the agent crashes withIndexErrorinstead of returning a proper error.Changes
Changed
>to>=in two retry exhaustion checks inrun_agent.py:The while loop condition is
retry_count < max_retries, soretry_countmaxes out atmax_retriesafter increment. The old checkretry_count > max_retriescould never be true inside the loop, making it dead code. The loop would exit and fall through toresponse.choices[0]on an invalid response, crashing withIndexError.Tests
Added
TestRetryExhaustiontotests/test_run_agent.pywith 2 regression tests:All 74 tests pass.
Closes #222