diff --git a/nanobot/providers/openai_codex_provider.py b/nanobot/providers/openai_codex_provider.py index 523b2a72a..dee66c073 100644 --- a/nanobot/providers/openai_codex_provider.py +++ b/nanobot/providers/openai_codex_provider.py @@ -84,9 +84,21 @@ class OpenAICodexProvider(LLMProvider): ) return LLMResponse(content=content, tool_calls=tool_calls, finish_reason=finish_reason) except Exception as e: - msg = f"Error calling Codex: {e}" - retry_after = getattr(e, "retry_after", None) or self._extract_retry_after(msg) - return LLMResponse(content=msg, finish_reason="error", retry_after=retry_after) + response = _codex_error_response(e) + exc_type = "CodexHTTPError" if isinstance(e, _CodexHTTPError) else type(e).__name__ + logger.warning( + "Codex API request failed: type={} kind={} retryable={} status={} " + "error_type={} error_code={} retry_after={} summary={}", + exc_type, + response.error_kind, + response.error_should_retry, + response.error_status_code, + response.error_type, + response.error_code, + response.retry_after, + _codex_log_summary(exc_type, response), + ) + return response async def chat( self, messages: list[dict[str, Any]], tools: list[dict[str, Any]] | None = None, @@ -139,9 +151,22 @@ def _build_headers(account_id: str, token: str) -> dict[str, str]: class _CodexHTTPError(RuntimeError): - def __init__(self, message: str, retry_after: float | None = None): + def __init__( + self, + message: str, + *, + status_code: int | None = None, + retry_after: float | None = None, + error_type: str | None = None, + error_code: str | None = None, + should_retry: bool | None = None, + ): super().__init__(message) + self.status_code = status_code self.retry_after = retry_after + self.error_type = error_type + self.error_code = error_code + self.should_retry = should_retry async def _request_codex( @@ -156,10 +181,16 @@ async def _request_codex( async with client.stream("POST", url, headers=headers, json=body) as response: if response.status_code != 200: text = await response.aread() + raw = text.decode("utf-8", "ignore") retry_after = LLMProvider._extract_retry_after_from_headers(response.headers) + error_type, error_code = LLMProvider._extract_error_type_code(raw) raise _CodexHTTPError( - _friendly_error(response.status_code, text.decode("utf-8", "ignore")), + _friendly_error(response.status_code, raw), + status_code=response.status_code, retry_after=retry_after, + error_type=error_type, + error_code=error_code, + should_retry=_should_retry_status(response.status_code, error_type, error_code, raw), ) return await consume_sse(response, on_content_delta, on_tool_call_delta) @@ -170,6 +201,94 @@ def _prompt_cache_key(messages: list[dict[str, Any]]) -> str: def _friendly_error(status_code: int, raw: str) -> str: + _ = raw if status_code == 429: return "ChatGPT usage quota exceeded or rate limit triggered. Please try again later." - return f"HTTP {status_code}: {raw}" + return f"HTTP {status_code}: Codex API request failed" + + +def _codex_error_response(exc: Exception) -> LLMResponse: + """Convert Codex transport/API failures into actionable, retryable metadata.""" + exc_type = "CodexHTTPError" if isinstance(exc, _CodexHTTPError) else type(exc).__name__ + detail = str(exc).strip() + + status_code = getattr(exc, "status_code", None) + error_kind: str | None = None + default_detail: str | None = None + should_retry: bool | None = getattr(exc, "should_retry", None) + + if isinstance(exc, (httpx.TimeoutException, asyncio.TimeoutError)): + error_kind = "timeout" + default_detail = "timed out waiting for response" + should_retry = True if should_retry is None else should_retry + elif isinstance(exc, httpx.RemoteProtocolError): + error_kind = "connection" + default_detail = "network protocol error while reading response" + should_retry = True if should_retry is None else should_retry + elif isinstance(exc, (httpx.NetworkError, httpx.TransportError)): + error_kind = "connection" + default_detail = "network connection failed" + should_retry = True if should_retry is None else should_retry + elif isinstance(exc, _CodexHTTPError): + error_kind = "http" + default_detail = "HTTP request failed" + + if status_code is not None and should_retry is None: + retry_content = None if int(status_code) == 429 and isinstance(exc, _CodexHTTPError) else detail + should_retry = _should_retry_status( + int(status_code), + getattr(exc, "error_type", None), + getattr(exc, "error_code", None), + retry_content, + ) + + detail = detail or default_detail or "unexpected error" + message = f"Error calling Codex ({exc_type}): {detail}" + retry_after = getattr(exc, "retry_after", None) or LLMProvider._extract_retry_after(message) + return LLMResponse( + content=message, + finish_reason="error", + retry_after=retry_after, + error_status_code=int(status_code) if status_code is not None else None, + error_kind=error_kind, + error_type=getattr(exc, "error_type", None), + error_code=getattr(exc, "error_code", None), + error_retry_after_s=retry_after, + error_should_retry=should_retry, + ) + + +def _codex_log_summary(exc_type: str, response: LLMResponse) -> str: + """Return a bounded diagnostic summary without request body or raw upstream payload.""" + if response.error_status_code is not None: + parts = [f"HTTP {response.error_status_code}"] + if response.error_type: + parts.append(f"type={response.error_type}") + if response.error_code: + parts.append(f"code={response.error_code}") + return " ".join(parts) + + kind = (response.error_kind or "").strip() + if kind: + return f"{exc_type} {kind}" + + return exc_type + + +def _should_retry_status( + status_code: int, + error_type: str | None, + error_code: str | None, + content: str | None, +) -> bool: + if status_code == 429: + return LLMProvider._is_retryable_429_response( + LLMResponse( + content=content or "", + finish_reason="error", + error_status_code=status_code, + error_type=error_type, + error_code=error_code, + ) + ) + return status_code in LLMProvider._RETRYABLE_STATUS_CODES or status_code >= 500 diff --git a/tests/providers/test_openai_codex_provider.py b/tests/providers/test_openai_codex_provider.py index e31b8547f..02e43638a 100644 --- a/tests/providers/test_openai_codex_provider.py +++ b/tests/providers/test_openai_codex_provider.py @@ -1,20 +1,105 @@ from __future__ import annotations +import io from types import SimpleNamespace +from typing import Any +import httpx import pytest +from loguru import logger -from nanobot.providers.openai_codex_provider import OpenAICodexProvider +import nanobot.providers.base as provider_base +from nanobot.providers.openai_codex_provider import ( + OpenAICodexProvider, + _codex_error_response, + _CodexHTTPError, + _friendly_error, + _request_codex, + _should_retry_status, +) + + +def _mock_codex_token(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr( + "nanobot.providers.openai_codex_provider.get_codex_token", + lambda: SimpleNamespace(account_id="acct", access="token"), + ) + + +class _WarningCaptureLogger: + def __init__(self) -> None: + self.calls: list[tuple[str, tuple[Any, ...]]] = [] + + def warning(self, *args: Any, **kwargs: Any) -> None: + self.calls.append((args[0], args[1:])) + + def exception(self, message: str, *args: Any, **kwargs: Any) -> None: + raise AssertionError("Codex diagnostics must not log exception tracebacks") + + +def _capture_codex_warnings(monkeypatch: pytest.MonkeyPatch) -> _WarningCaptureLogger: + capture = _WarningCaptureLogger() + monkeypatch.setattr("nanobot.providers.openai_codex_provider.logger", capture) + return capture + + +def test_codex_blank_timeout_root_cause_reproduction() -> None: + """Document why upstream produced a bare ``Error calling Codex:`` message.""" + exc = httpx.ReadTimeout("") + legacy_content = f"Error calling Codex: {exc}" + + assert str(exc) == "" + assert legacy_content == "Error calling Codex: " + legacy_response = provider_base.LLMResponse(content=legacy_content, finish_reason="error") + assert legacy_response.error_kind is None + assert legacy_response.error_should_retry is None + + +def test_codex_http_friendly_error_omits_raw_body() -> None: + raw = "raw upstream body with PRIVATE PROMPT MUST NOT APPEAR" + + message = _friendly_error(500, raw) + + assert message == "HTTP 500: Codex API request failed" + assert "PRIVATE PROMPT MUST NOT APPEAR" not in message + + +@pytest.mark.asyncio +async def test_codex_request_non_200_populates_http_metadata(monkeypatch) -> None: + original_client = httpx.AsyncClient + + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response( + 429, + headers={"retry-after": "2"}, + json={"error": {"type": "rate_limit_exceeded", "code": "rate_limit_exceeded"}}, + request=request, + ) + + def fake_client(*, timeout: float, verify: bool) -> httpx.AsyncClient: + assert timeout == 60.0 + assert verify is True + return original_client(transport=httpx.MockTransport(handler), timeout=timeout) + + monkeypatch.setattr("nanobot.providers.openai_codex_provider.httpx.AsyncClient", fake_client) + + with pytest.raises(_CodexHTTPError) as caught: + await _request_codex("https://codex.example/responses", {}, {"input": []}, verify=True) + + error = caught.value + assert str(error) == "ChatGPT usage quota exceeded or rate limit triggered. Please try again later." + assert error.status_code == 429 + assert error.retry_after == 2.0 + assert error.error_type == "rate_limit_exceeded" + assert error.error_code == "rate_limit_exceeded" + assert error.should_retry is True @pytest.mark.asyncio async def test_codex_prompt_cache_key_uses_stable_conversation_prefix(monkeypatch) -> None: bodies: list[dict] = [] - monkeypatch.setattr( - "nanobot.providers.openai_codex_provider.get_codex_token", - lambda: SimpleNamespace(account_id="acct", access="token"), - ) + _mock_codex_token(monkeypatch) async def fake_request( url, @@ -56,3 +141,238 @@ async def test_codex_prompt_cache_key_uses_stable_conversation_prefix(monkeypatc assert bodies[0]["prompt_cache_key"] == bodies[1]["prompt_cache_key"] assert bodies[0]["prompt_cache_key"] != bodies[2]["prompt_cache_key"] + + +@pytest.mark.asyncio +async def test_codex_timeout_error_is_typed_and_retryable(monkeypatch) -> None: + _mock_codex_token(monkeypatch) + + async def fake_request(*args, **kwargs): + raise httpx.ReadTimeout("") + + monkeypatch.setattr("nanobot.providers.openai_codex_provider._request_codex", fake_request) + + provider = OpenAICodexProvider() + response = await provider.chat([{"role": "user", "content": "hello"}]) + + assert response.finish_reason == "error" + assert response.content == ( + "Error calling Codex (ReadTimeout): timed out waiting for response" + ) + assert response.error_kind == "timeout" + assert response.error_should_retry is True + + +@pytest.mark.asyncio +async def test_codex_timeout_error_writes_diagnostic_log(monkeypatch) -> None: + log_capture = _capture_codex_warnings(monkeypatch) + _mock_codex_token(monkeypatch) + + async def fake_request(*args: Any, **kwargs: Any): + raise httpx.ReadTimeout("") + + monkeypatch.setattr("nanobot.providers.openai_codex_provider._request_codex", fake_request) + + provider = OpenAICodexProvider() + response = await provider.chat([{"role": "user", "content": "hello"}]) + + assert response.content == ( + "Error calling Codex (ReadTimeout): timed out waiting for response" + ) + assert log_capture.calls == [ + ( + "Codex API request failed: type={} kind={} retryable={} status={} " + "error_type={} error_code={} retry_after={} summary={}", + ( + "ReadTimeout", + "timeout", + True, + None, + None, + None, + None, + "ReadTimeout timeout", + ), + ) + ] + + +@pytest.mark.asyncio +async def test_codex_diagnostic_log_omits_prompt_content(monkeypatch) -> None: + sink = io.StringIO() + logger.enable("nanobot") + handler_id = logger.add(sink, format="{message}", backtrace=True, diagnose=True) + try: + _mock_codex_token(monkeypatch) + + async def fake_request(*args: Any, **kwargs: Any): + raise httpx.ReadTimeout("") + + monkeypatch.setattr("nanobot.providers.openai_codex_provider._request_codex", fake_request) + + provider = OpenAICodexProvider() + response = await provider.chat( + [{"role": "user", "content": "PRIVATE PROMPT MUST NOT APPEAR"}] + ) + finally: + logger.remove(handler_id) + + log_text = sink.getvalue() + assert response.error_kind == "timeout" + assert "Codex API request failed" in log_text + assert "ReadTimeout" in log_text + assert "PRIVATE PROMPT MUST NOT APPEAR" not in log_text + + +@pytest.mark.asyncio +async def test_codex_retry_uses_structured_timeout_metadata(monkeypatch) -> None: + calls = 0 + delays: list[float] = [] + + _mock_codex_token(monkeypatch) + + async def fake_request(*args, **kwargs): + nonlocal calls + calls += 1 + if calls == 1: + raise httpx.ReadTimeout("") + return "ok", [], "stop" + + async def fake_sleep(delay: float) -> None: + delays.append(delay) + + monkeypatch.setattr("nanobot.providers.openai_codex_provider._request_codex", fake_request) + monkeypatch.setattr(provider_base.asyncio, "sleep", fake_sleep) + + provider = OpenAICodexProvider() + response = await provider.chat_with_retry(messages=[{"role": "user", "content": "hello"}]) + + assert response.content == "ok" + assert calls == 2 + assert delays == [1] + + +@pytest.mark.asyncio +async def test_codex_http_error_preserves_status_and_retry_after(monkeypatch) -> None: + _mock_codex_token(monkeypatch) + + async def fake_request(*args, **kwargs): + raise _CodexHTTPError( + "HTTP 503: backend unavailable", + status_code=503, + retry_after=2.5, + error_type="server_error", + error_code="overloaded", + ) + + monkeypatch.setattr("nanobot.providers.openai_codex_provider._request_codex", fake_request) + + provider = OpenAICodexProvider() + response = await provider.chat([{"role": "user", "content": "hello"}]) + + assert response.finish_reason == "error" + assert response.content == "Error calling Codex (CodexHTTPError): HTTP 503: backend unavailable" + assert response.error_status_code == 503 + assert response.error_kind == "http" + assert response.error_type == "server_error" + assert response.error_code == "overloaded" + assert response.retry_after == 2.5 + assert response.error_should_retry is True + + +@pytest.mark.asyncio +async def test_codex_http_diagnostic_log_omits_raw_body(monkeypatch) -> None: + log_capture = _capture_codex_warnings(monkeypatch) + _mock_codex_token(monkeypatch) + + async def fake_request(*args: Any, **kwargs: Any): + raise _CodexHTTPError( + _friendly_error(500, "raw upstream body with PRIVATE PROMPT MUST NOT APPEAR"), + status_code=500, + error_type="server_error", + error_code="overloaded", + ) + + monkeypatch.setattr("nanobot.providers.openai_codex_provider._request_codex", fake_request) + + provider = OpenAICodexProvider() + response = await provider.chat([{"role": "user", "content": "hello"}]) + + assert response.content == "Error calling Codex (CodexHTTPError): HTTP 500: Codex API request failed" + assert log_capture.calls == [ + ( + "Codex API request failed: type={} kind={} retryable={} status={} " + "error_type={} error_code={} retry_after={} summary={}", + ( + "CodexHTTPError", + "http", + True, + 500, + "server_error", + "overloaded", + None, + "HTTP 500 type=server_error code=overloaded", + ), + ) + ] + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + ("error_type", "error_code", "expected_retry"), + [ + ("rate_limit_exceeded", "rate_limit_exceeded", True), + ("insufficient_quota", "insufficient_quota", False), + ], +) +async def test_codex_429_preserves_retry_semantics( + monkeypatch, + error_type: str, + error_code: str, + expected_retry: bool, +) -> None: + _mock_codex_token(monkeypatch) + + async def fake_request(*args: Any, **kwargs: Any): + raise _CodexHTTPError( + "ChatGPT usage quota exceeded or rate limit triggered. Please try again later.", + status_code=429, + error_type=error_type, + error_code=error_code, + should_retry=expected_retry, + ) + + monkeypatch.setattr("nanobot.providers.openai_codex_provider._request_codex", fake_request) + + provider = OpenAICodexProvider() + response = await provider.chat([{"role": "user", "content": "hello"}]) + + assert response.error_status_code == 429 + assert response.error_type == error_type + assert response.error_code == error_code + assert response.error_should_retry is expected_retry + + +def test_codex_429_friendly_message_fallback_does_not_override_unknown_retry() -> None: + response = _codex_error_response( + _CodexHTTPError(_friendly_error(429, ""), status_code=429) + ) + + assert response.error_status_code == 429 + assert response.error_should_retry is True + + +@pytest.mark.parametrize( + ("raw", "expected_retry"), + [ + ('{"error":{"type":"rate_limit_exceeded","code":"rate_limit_exceeded"}}', True), + ('{"error":{"type":"insufficient_quota","code":"insufficient_quota"}}', False), + ], +) +def test_codex_429_classification_uses_raw_error_semantics( + raw: str, + expected_retry: bool, +) -> None: + error_type, error_code = provider_base.LLMProvider._extract_error_type_code(raw) + + assert _should_retry_status(429, error_type, error_code, raw) is expected_retry