mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
fix: guard api_kwargs in except handler to prevent UnboundLocalError (#7376)
When _build_api_kwargs() throws an exception, the except handler in the retry loop referenced api_kwargs before it was assigned. This caused an UnboundLocalError that masked the real error, making debugging impossible for the user. Two _dump_api_request_debug() calls in the except block (non-retryable client error path and max-retries-exhausted path) both accessed api_kwargs without checking if it was assigned. Fix: initialize api_kwargs = None before the retry loop and guard both dump calls. Now the real error surfaces instead of the masking UnboundLocalError. Reported by Discord user gruman0.
This commit is contained in:
parent
496e378b10
commit
ea81aa2eec
2 changed files with 31 additions and 6 deletions
15
run_agent.py
15
run_agent.py
|
|
@ -7708,6 +7708,7 @@ class AIAgent:
|
|||
|
||||
finish_reason = "stop"
|
||||
response = None # Guard against UnboundLocalError if all retries fail
|
||||
api_kwargs = None # Guard against UnboundLocalError in except handler
|
||||
|
||||
while retry_count < max_retries:
|
||||
try:
|
||||
|
|
@ -8742,9 +8743,10 @@ class AIAgent:
|
|||
if self._try_activate_fallback():
|
||||
retry_count = 0
|
||||
continue
|
||||
self._dump_api_request_debug(
|
||||
api_kwargs, reason="non_retryable_client_error", error=api_error,
|
||||
)
|
||||
if api_kwargs is not None:
|
||||
self._dump_api_request_debug(
|
||||
api_kwargs, reason="non_retryable_client_error", error=api_error,
|
||||
)
|
||||
self._emit_status(
|
||||
f"❌ Non-retryable error (HTTP {status_code}): "
|
||||
f"{self._summarize_api_error(api_error)}"
|
||||
|
|
@ -8847,9 +8849,10 @@ class AIAgent:
|
|||
self.log_prefix, max_retries, _final_summary,
|
||||
_provider, _model, len(api_messages), f"{approx_tokens:,}",
|
||||
)
|
||||
self._dump_api_request_debug(
|
||||
api_kwargs, reason="max_retries_exhausted", error=api_error,
|
||||
)
|
||||
if api_kwargs is not None:
|
||||
self._dump_api_request_debug(
|
||||
api_kwargs, reason="max_retries_exhausted", error=api_error,
|
||||
)
|
||||
self._persist_session(messages, conversation_history)
|
||||
_final_response = f"API call failed after {max_retries} retries: {_final_summary}"
|
||||
if _is_stream_drop:
|
||||
|
|
|
|||
|
|
@ -2125,6 +2125,28 @@ class TestRetryExhaustion:
|
|||
assert "error" in result
|
||||
assert "rate limited" in result["error"]
|
||||
|
||||
def test_build_api_kwargs_error_no_unbound_local(self, agent):
|
||||
"""When _build_api_kwargs raises, except handler must not crash with UnboundLocalError.
|
||||
|
||||
Regression: _dump_api_request_debug(api_kwargs, ...) in the except block
|
||||
referenced api_kwargs before it was assigned when _build_api_kwargs threw.
|
||||
"""
|
||||
self._setup_agent(agent)
|
||||
with (
|
||||
patch.object(agent, "_build_api_kwargs", side_effect=ValueError("bad messages")),
|
||||
patch.object(agent, "_persist_session"),
|
||||
patch.object(agent, "_save_trajectory"),
|
||||
patch.object(agent, "_cleanup_task_resources"),
|
||||
patch("run_agent.time", self._make_fast_time_mock()),
|
||||
):
|
||||
result = agent.run_conversation("hello")
|
||||
# Must surface the real error, not UnboundLocalError
|
||||
assert result.get("completed") is False
|
||||
assert result.get("failed") is True
|
||||
assert "error" in result
|
||||
assert "UnboundLocalError" not in result.get("error", "")
|
||||
assert "bad messages" in result["error"]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Flush sentinel leak
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue