From 480c8dd744b5265bd03dcbb870d1f43b4a55b7c5 Mon Sep 17 00:00:00 2001 From: KDB <937925477@qq.com> Date: Sun, 30 Aug 2026 03:38:11 +0800 Subject: [PATCH] fix(providers): preserve retry metadata for raised errors --- nanobot/providers/base.py | 59 +++++++++++++++++++++++++- nanobot/providers/fallback_provider.py | 47 +++----------------- tests/agent/test_runner_fallback.py | 30 +++++++++++-- 3 files changed, 91 insertions(+), 45 deletions(-) diff --git a/nanobot/providers/base.py b/nanobot/providers/base.py index f4a89ffe4..d95180c42 100644 --- a/nanobot/providers/base.py +++ b/nanobot/providers/base.py @@ -943,6 +943,61 @@ class LLMProvider(ABC): """ pass + @staticmethod + def _error_response_from_exception(exc: Exception) -> LLMResponse: + """Convert an unexpected exception while retaining retry metadata.""" + error_names = tuple(cls.__name__.lower() for cls in type(exc).__mro__) + error_kind: str | None = None + error_should_retry: bool | None = None + if any("timeout" in name for name in error_names): + error_kind = "timeout" + error_should_retry = True + elif any( + token in name + for name in error_names + for token in ("connect", "connection", "network", "protocol", "transport") + ): + error_kind = "connection" + error_should_retry = True + elif any( + "ratelimit" in name or "throttl" in name + for name in error_names + ): + error_kind = "rate_limit" + error_should_retry = True + elif any( + "server" in name or "internal" in name + for name in error_names + ): + error_kind = "server_error" + error_should_retry = True + elif any( + token in name + for name in error_names + for token in ("auth", "credential", "permissiondenied", "unauthor") + ): + error_kind = "authentication" + + response = getattr(exc, "response", None) + raw_status = getattr(exc, "status_code", None) + if raw_status is None and response is not None: + raw_status = getattr(response, "status_code", None) + try: + error_status_code = int(raw_status) if raw_status is not None else None + except (TypeError, ValueError): + error_status_code = None + + detail = str(exc).strip() or type(exc).__name__ + return LLMResponse( + content=f"Error calling LLM: {detail}", + finish_reason="error", + error_status_code=error_status_code, + error_kind=error_kind, + error_type=getattr(exc, "error_type", None), + error_code=getattr(exc, "error_code", None), + error_should_retry=error_should_retry, + ) + @classmethod def _is_transient_error(cls, content: str | None) -> bool: err = (content or "").lower() @@ -1226,7 +1281,7 @@ class LLMProvider(ABC): ) raise except Exception as exc: - response = LLMResponse(content=f"Error calling LLM: {exc}", finish_reason="error") + response = self._error_response_from_exception(exc) return self._observe_llm_call( response, kwargs, @@ -1368,7 +1423,7 @@ class LLMProvider(ABC): ) raise except Exception as exc: - response = LLMResponse(content=f"Error calling LLM: {exc}", finish_reason="error") + response = self._error_response_from_exception(exc) return self._observe_llm_call( _attach_stream_timing(response), kwargs, diff --git a/nanobot/providers/fallback_provider.py b/nanobot/providers/fallback_provider.py index d8a6173d7..427bf44f4 100644 --- a/nanobot/providers/fallback_provider.py +++ b/nanobot/providers/fallback_provider.py @@ -10,7 +10,6 @@ from collections.abc import Awaitable, Callable from dataclasses import replace from typing import Any -import httpx from loguru import logger from nanobot.providers.base import ( @@ -622,45 +621,13 @@ class FallbackProvider(LLMProvider): except asyncio.CancelledError: raise except Exception as exc: - error_name = type(exc).__name__.lower() - detail = str(exc).strip() or type(exc).__name__ - detail_lower = detail.lower() - error_kind: str | None = None - error_should_retry: bool | None = None - if isinstance(exc, (httpx.TimeoutException, asyncio.TimeoutError)): - error_kind = "timeout" - error_should_retry = True - elif isinstance(exc, (httpx.NetworkError, httpx.TransportError, ConnectionError)): - error_kind = "connection" - error_should_retry = True - elif any( - token in error_name - for token in ("auth", "credential", "permissiondenied", "unauthor") - ) or any(token in detail_lower for token in _AUTHENTICATION_ERROR_TOKENS): - error_kind = "authentication" - elif "ratelimit" in error_name or "throttl" in error_name: - error_kind = "rate_limit" - error_should_retry = True - elif "server" in error_name or "internal" in error_name: - error_kind = "server_error" - error_should_retry = True - - response = getattr(exc, "response", None) - raw_status = getattr(exc, "status_code", None) - if raw_status is None and response is not None: - raw_status = getattr(response, "status_code", None) - try: - error_status_code = int(raw_status) if raw_status is not None else None - except (TypeError, ValueError): - error_status_code = None - - return LLMResponse( - content=f"Error calling LLM: {detail}", - finish_reason="error", - error_status_code=error_status_code, - error_kind=error_kind, - error_should_retry=error_should_retry, - ), exc + response = LLMProvider._error_response_from_exception(exc) + if response.error_kind is None and any( + token in (str(exc).strip() or type(exc).__name__).lower() + for token in _AUTHENTICATION_ERROR_TOKENS + ): + response.error_kind = "authentication" + return response, exc async def _notify_fallback_model(self, model: str) -> None: if self._fallback_model_observer is None: diff --git a/tests/agent/test_runner_fallback.py b/tests/agent/test_runner_fallback.py index c8297bee3..15231a244 100644 --- a/tests/agent/test_runner_fallback.py +++ b/tests/agent/test_runner_fallback.py @@ -353,7 +353,7 @@ class _RaisingProvider(LLMProvider): """Provider whose chat/chat_stream raise, like an auth/setup failure.""" def __init__(self, name: str = "raiser", exc: BaseException | None = None): - super().__init__() + super().__init__(provider_name=name) self.name = name self._exc = exc if exc is not None else RuntimeError("GitHub Copilot is not logged in.") @@ -378,8 +378,8 @@ class _StreamingThenRaisingProvider(_RaisingProvider): class TestFallbackWhenPrimaryRaises: @pytest.mark.parametrize( "exc", - [TimeoutError(), httpx.ReadTimeout(""), httpx.ConnectError("")], - ids=["asyncio-timeout", "httpx-timeout", "connection"], + [TimeoutError(), httpx.ReadTimeout(""), httpx.ConnectError(""), httpx.ReadError("")], + ids=["asyncio-timeout", "httpx-timeout", "connection", "httpx-network-error"], ) @pytest.mark.asyncio async def test_transient_exception_triggers_fallback(self, exc: Exception) -> None: @@ -423,6 +423,30 @@ class TestFallbackWhenPrimaryRaises: assert result.finish_reason == "stop" factory.assert_called_once_with(_fallback("fallback-a")) + @pytest.mark.asyncio + @pytest.mark.parametrize("stream", [False, True], ids=["chat", "stream"]) + async def test_retry_entry_point_preserves_transient_exception_metadata( + self, + stream: bool, + ) -> None: + """A retry wrapper must not hide an empty transient exception from failover.""" + primary = _RaisingProvider("primary", TimeoutError()) + fallback = _FakeProvider("fallback", _make_response("fallback ok")) + factory = MagicMock(return_value=fallback) + + fb = FallbackProvider( + primary=primary, + fallback_presets=[_fallback("fallback-a")], + provider_factory=factory, + ) + + retry = fb.chat_stream_with_retry if stream else fb.chat_with_retry + result = await retry(messages=[{"role": "user", "content": "hi"}], model="primary-model") + + assert result.content == "fallback ok" + assert result.finish_reason == "stop" + factory.assert_called_once_with(_fallback("fallback-a")) + @pytest.mark.asyncio async def test_authentication_exception_message_is_classified(self) -> None: primary = _RaisingProvider("primary")