fix(chat_completion_helpers): guard against OverflowError when _stale_timeout is inf in wait notice - #64924
Conversation
…_timeout is inf in wait notice
When _compute_non_stream_stale_timeout returns float('inf') (e.g., local
endpoint with implicit default), _emit_wait_notice crashes because
int(float('inf')) raises OverflowError.
Fix: render 'no limit' instead of int(_deadline) when _deadline is inf.
|
Confirmed this exact regression on a real Hermes installation after updating to commit Environment
Observed impactNo explicit float("inf")After approximately 30 seconds without a response, the wait-state notice attempted: int(_deadline)and raised: This was not limited to a display problem. It interrupted the provider polling path and resulted in repeated retries, eventually producing: The streaming wait-notice path already guards against an infinite timeout, while the non-streaming path introduced in #64775 did not. Also, the fail-open behavior inside Local validationI tested a local fix that safely omits the reconnect suffix for non-finite deadlines and added a regression test covering: float("inf")The targeted test suite passed:
The guard in this PR fixes the observed
Since an infinite stale timeout means there is no scheduled automatic stale reconnect, omitting the Thanks for addressing this quickly. |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating a real regression: current main returns float("inf") for implicit local-endpoint stale timeouts (run_agent.py:1318-1322) and formats it with int(_deadline) in the non-stream heartbeat (agent/chat_completion_helpers.py:614-623).
Problems
- The proposed message says
auto-reconnect at no limit, but the reconnect branch only runs when_elapsed > _stale_timeout(agent/chat_completion_helpers.py:729-731), which never occurs for infinity. == float("inf")does not protect other non-finite values. The timeout coercion acceptsnan(hermes_cli/timeouts.py:4-11), andint(float("nan"))also fails.- No regression test accompanies the fix; the existing wait-notice test uses a finite timeout (
tests/run_agent/test_wait_state_visibility.py:78-123).
Suggested changes
- Use
math.isfinite()and omit the recovery suffix when the deadline is non-finite, matching the streaming path (agent/chat_completion_helpers.py:3183-3189). - Add a regression test for the infinite non-stream timeout path.
Automated hermes-sweeper review.
| and getattr(agent, "_codex_stream_last_event_ts", None) is None | ||
| ): | ||
| _deadline = min(_deadline, _ttfb_timeout) | ||
| _deadline_text = "no limit" if _deadline == float("inf") else f"{int(_deadline)}s" |
There was a problem hiding this comment.
_deadline == float("inf") fixes the reported case, but use math.isfinite(_deadline) and omit the auto-reconnect suffix when it is false. nan is accepted by the current timeout coercion and also fails under int(), while an infinite stale timeout never reaches the reconnect branch.
|
Confirmed the exact production signature on #65594 now includes a deterministic regression test that drives the 100-poll heartbeat without sleeping 30s. It fails with the reported |
|
Superseded by #66139, which has now merged the same heartbeat crash fix with broader handling for all non-finite deadlines, accurate finite-watchdog reporting, fail-open notice construction, and full regression coverage. Your PR identified this bug first, and that first-submitter credit is recorded in #66139. Thank you for isolating the regression. |
Summary
Guard against
OverflowErrorin_emit_wait_noticewhen the non-stream stale timeout isfloat('inf').Problem
_compute_non_stream_stale_timeoutreturnsfloat('inf')for local endpoints with implicit default timeouts (line 1313 in run_agent.py). The wait-notice heartbeat ininterruptible_api_calldoesint(_deadline)to format the auto-reconnect ETA, which crashes withOverflowError: cannot convert float infinity to integerwhen the deadline is infinite. This turns a harmless display-layer heartbeat into an uncaught exception that can break the polling loop.Fix
Render
"no limit"instead ofint(_deadline)when_deadline == float("inf"). Minimal 2-line change — only affects the display string.