Handle blank Codex transport errors

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}"
+            response = _codex_error_response(e)
-            retry_after = getattr(e, "retry_after", None) or self._extract_retry_after(msg)
+            exc_type = "CodexHTTPError" if isinstance(e, _CodexHTTPError) else type(e).__name__
-            return LLMResponse(content=msg, finish_reason="error", retry_after=retry_after)
+            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

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(
+    _mock_codex_token(monkeypatch)
-        "nanobot.providers.openai_codex_provider.get_codex_token",
-        lambda: SimpleNamespace(account_id="acct", access="token"),
-    )
 
     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