mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-23 16:36:23 +00:00
fix(codex): recover from SDK none-output iterator crash
This commit is contained in:
parent
aae5638fd3
commit
f9665d0475
6 changed files with 137 additions and 6 deletions
|
|
@ -281,7 +281,35 @@ def _consume_codex_event_stream(
|
|||
terminal_error: Any = None
|
||||
saw_terminal = False
|
||||
|
||||
for event in event_iter:
|
||||
event_iter = iter(event_iter)
|
||||
while True:
|
||||
try:
|
||||
event = next(event_iter)
|
||||
except StopIteration:
|
||||
break
|
||||
except TypeError as exc:
|
||||
# The OpenAI SDK's streamed Responses iterator still runs its own
|
||||
# accumulator while yielding SSE events. When chatgpt.com emits a
|
||||
# terminal snapshot whose ``response.output`` is ``null``, the SDK
|
||||
# trips over ``for output in response.output`` and raises
|
||||
# ``TypeError: 'NoneType' object is not iterable`` *after* earlier
|
||||
# ``response.output_item.done`` / ``response.output_text.delta``
|
||||
# events have already arrived. If we have recoverable streamed
|
||||
# state, synthesize the final response from that state instead of
|
||||
# letting a local parse bug poison credential-pool health.
|
||||
if "not iterable" not in str(exc):
|
||||
raise
|
||||
if not collected_output_items and not collected_text_deltas:
|
||||
raise
|
||||
logger.debug(
|
||||
"Codex Responses iterator hit SDK None-output TypeError; "
|
||||
"recovering from streamed state (items=%d, chars=%d)",
|
||||
len(collected_output_items),
|
||||
sum(len(p) for p in collected_text_deltas),
|
||||
)
|
||||
saw_terminal = True
|
||||
terminal_status = terminal_status or "completed"
|
||||
break
|
||||
if on_event is not None:
|
||||
try:
|
||||
on_event(event)
|
||||
|
|
|
|||
|
|
@ -2914,7 +2914,11 @@ def run_conversation(
|
|||
if is_client_error:
|
||||
# Try fallback before aborting — a different provider
|
||||
# may not have the same issue (rate limit, auth, etc.)
|
||||
agent._emit_status(f"⚠️ Non-retryable error (HTTP {status_code}) — trying fallback...")
|
||||
_err_kind = agent._nonretryable_error_kind(status_code, api_error)
|
||||
agent._emit_status(
|
||||
f"⚠️ Non-retryable error ({_err_kind}): "
|
||||
f"{agent._summarize_api_error(api_error)} — trying fallback..."
|
||||
)
|
||||
if agent._try_activate_fallback():
|
||||
retry_count = 0
|
||||
compression_attempts = 0
|
||||
|
|
@ -2925,10 +2929,10 @@ def run_conversation(
|
|||
api_kwargs, reason="non_retryable_client_error", error=api_error,
|
||||
)
|
||||
agent._emit_status(
|
||||
f"❌ Non-retryable error (HTTP {status_code}): "
|
||||
f"❌ Non-retryable error ({_err_kind}): "
|
||||
f"{agent._summarize_api_error(api_error)}"
|
||||
)
|
||||
agent._vprint(f"{agent.log_prefix}❌ Non-retryable client error (HTTP {status_code}). Aborting.", force=True)
|
||||
agent._vprint(f"{agent.log_prefix}❌ Non-retryable client error ({_err_kind}). Aborting.", force=True)
|
||||
agent._vprint(f"{agent.log_prefix} 🔌 Provider: {_provider} Model: {_model}", force=True)
|
||||
agent._vprint(f"{agent.log_prefix} 🌐 Endpoint: {_base}", force=True)
|
||||
# Actionable guidance for common auth errors
|
||||
|
|
|
|||
12
run_agent.py
12
run_agent.py
|
|
@ -1563,6 +1563,18 @@ class AIAgent:
|
|||
prefix = f"HTTP {status_code}: " if status_code else ""
|
||||
return f"{prefix}{raw[:500]}"
|
||||
|
||||
@staticmethod
|
||||
def _nonretryable_error_kind(status_code, api_error) -> str:
|
||||
"""Short label for a non-retryable failure.
|
||||
|
||||
Returns ``HTTP <code>`` when an HTTP status is present, otherwise the
|
||||
exception class name. This avoids misleading labels like ``HTTP None``
|
||||
for local parse or transport failures.
|
||||
"""
|
||||
if status_code:
|
||||
return f"HTTP {status_code}"
|
||||
return type(api_error).__name__
|
||||
|
||||
def _mask_api_key_for_logs(self, key: Any) -> Optional[str]:
|
||||
# Azure Foundry Entra ID bearer providers are callables — never
|
||||
# invoke them in log paths; identify the auth surface instead.
|
||||
|
|
|
|||
|
|
@ -2591,6 +2591,35 @@ class TestCodexAuxiliaryAdapterNullOutputRecovery:
|
|||
|
||||
assert response.choices[0].message.content == "aux survived"
|
||||
|
||||
def test_recovers_when_sdk_iterator_raises_none_output_typeerror(self):
|
||||
"""Recover from streamed items when the SDK iterator crashes post-stream."""
|
||||
output_item = SimpleNamespace(
|
||||
type="message",
|
||||
content=[SimpleNamespace(type="output_text", text="aux recovered")],
|
||||
)
|
||||
events = [
|
||||
SimpleNamespace(type="response.created"),
|
||||
SimpleNamespace(type="response.output_item.done", item=output_item),
|
||||
]
|
||||
|
||||
class _BrokenCreateStream:
|
||||
def __iter__(self):
|
||||
for event in events:
|
||||
yield event
|
||||
raise TypeError("'NoneType' object is not iterable")
|
||||
def close(self): pass
|
||||
|
||||
class FakeResponses:
|
||||
def create(self, **kwargs):
|
||||
return _BrokenCreateStream()
|
||||
|
||||
fake_client = SimpleNamespace(responses=FakeResponses())
|
||||
adapter = _CodexCompletionsAdapter(fake_client, "gpt-5.5")
|
||||
|
||||
response = adapter.create(messages=[{"role": "user", "content": "summarize"}])
|
||||
|
||||
assert response.choices[0].message.content == "aux recovered"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Issue #23432 — auxiliary timeout poisons cached client; later aux calls fail
|
||||
|
|
|
|||
|
|
@ -5632,3 +5632,15 @@ class TestMemoryProviderTurnStart:
|
|||
# The extracted body uses ``agent.X`` rather than ``self.X``;
|
||||
# assert the extracted-form spelling directly.
|
||||
assert "on_turn_start(agent._user_turn_count" in src
|
||||
|
||||
|
||||
def test_nonretryable_error_kind_surfaces_exception_type_without_status():
|
||||
"""Transport/parse failures with no status must not render as HTTP None."""
|
||||
assert (
|
||||
AIAgent._nonretryable_error_kind(
|
||||
None, TypeError("'NoneType' object is not iterable")
|
||||
)
|
||||
== "TypeError"
|
||||
)
|
||||
assert AIAgent._nonretryable_error_kind(0, ValueError("bad")) == "ValueError"
|
||||
assert AIAgent._nonretryable_error_kind(404, RuntimeError("nope")) == "HTTP 404"
|
||||
|
|
|
|||
|
|
@ -161,12 +161,16 @@ class _FakeCreateStream:
|
|||
tests use this to drive it through the same code paths the wire does.
|
||||
"""
|
||||
|
||||
def __init__(self, events):
|
||||
def __init__(self, events, *, iter_error=None):
|
||||
self._events = list(events)
|
||||
self._iter_error = iter_error
|
||||
self.closed = False
|
||||
|
||||
def __iter__(self):
|
||||
return iter(self._events)
|
||||
for event in self._events:
|
||||
yield event
|
||||
if self._iter_error is not None:
|
||||
raise self._iter_error
|
||||
|
||||
def close(self):
|
||||
self.closed = True
|
||||
|
|
@ -531,6 +535,48 @@ def test_run_codex_stream_ignores_completed_response_with_null_output(monkeypatc
|
|||
assert response.usage.total_tokens == 11
|
||||
|
||||
|
||||
def test_run_codex_stream_recovers_after_sdk_none_output_typeerror(monkeypatch):
|
||||
"""If the SDK iterator crashes after streamed items arrived, recover from them.
|
||||
|
||||
``responses.create(stream=True)`` still runs the OpenAI SDK's internal
|
||||
accumulator while yielding SSE events. When chatgpt.com sends a terminal
|
||||
snapshot with ``response.output = null``, the iterator can raise
|
||||
``TypeError: 'NoneType' object is not iterable`` after earlier
|
||||
``response.output_item.done`` events were already delivered. The runtime
|
||||
must synthesize the final response from those streamed items instead of
|
||||
surfacing a local parse failure.
|
||||
"""
|
||||
agent = _build_agent(monkeypatch)
|
||||
tool_call_item = SimpleNamespace(
|
||||
type="function_call",
|
||||
call_id="call_123",
|
||||
name="lookup_weather",
|
||||
arguments='{"city":"Lahore"}',
|
||||
)
|
||||
create_stream = _FakeCreateStream(
|
||||
[
|
||||
SimpleNamespace(type="response.created"),
|
||||
SimpleNamespace(type="response.output_item.done", item=tool_call_item),
|
||||
],
|
||||
iter_error=TypeError("'NoneType' object is not iterable"),
|
||||
)
|
||||
|
||||
def _fake_create(**kwargs):
|
||||
assert kwargs.get("stream") is True
|
||||
return create_stream
|
||||
|
||||
agent.client = SimpleNamespace(
|
||||
responses=SimpleNamespace(create=_fake_create),
|
||||
)
|
||||
|
||||
response = agent._run_codex_stream(_codex_request_kwargs())
|
||||
assert response is not None
|
||||
assert create_stream.closed is True
|
||||
assert response.status == "completed"
|
||||
assert response.output == [tool_call_item]
|
||||
assert response.output_text == ""
|
||||
|
||||
|
||||
def test_run_conversation_codex_plain_text(monkeypatch):
|
||||
agent = _build_agent(monkeypatch)
|
||||
monkeypatch.setattr(agent, "_interruptible_api_call", lambda api_kwargs: _codex_message_response("OK"))
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue