fix(providers): apply fallback policy to raised errors

nanobot/providers/fallback_provider.py

@@ -4,11 +4,13 @@
 
 from __future__ import annotations
 
+import asyncio
 import time
 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 (
@@ -59,6 +61,7 @@ _AUTHENTICATION_ERROR_TOKENS = (
     "access_denied",
     "account_deactivated",
     "organization_deactivated",
+    "not logged in",
 )
 _NON_FALLBACK_ERROR_KINDS = frozenset({
     "content_filter",
@@ -428,7 +431,14 @@ class FallbackProvider(LLMProvider):
 
         if self._primary_available():
             primary_was_attempted = True
-            response = await call(self._primary, kwargs)
+            response, primary_exception = await self._call_provider(
+                call, self._primary, kwargs
+            )
+            if primary_exception is not None:
+                logger.warning(
+                    "Primary model '{}' raised {} before responding",
+                    primary_model, type(primary_exception).__name__,
+                )
             if response.finish_reason != "error":
                 self._primary_failures = 0
                 self._primary_tripped_at = None
@@ -457,7 +467,7 @@ class FallbackProvider(LLMProvider):
 
             if not self._should_fallback(response):
                 logger.warning(
-                    "Primary model '{}' returned non-fallbackable error: {}",
+                    "Primary model '{}' failed with non-fallbackable error: {}",
                     primary_model,
                     (response.content or "")[:120],
                 )
@@ -544,7 +554,14 @@ class FallbackProvider(LLMProvider):
                 fallback_kwargs.pop("reasoning_effort", None)
             else:
                 fallback_kwargs["reasoning_effort"] = fallback.reasoning_effort
-            fallback_response = await call(fallback_provider, fallback_kwargs)
+            fallback_response, fallback_exception = await self._call_provider(
+                call, fallback_provider, fallback_kwargs
+            )
+            if fallback_exception is not None:
+                logger.warning(
+                    "Fallback '{}' raised {}",
+                    fallback_model, type(fallback_exception).__name__,
+                )
 
             if fallback_response.finish_reason != "error":
                 # Do not publish a model switch merely because a fallback was
@@ -593,6 +610,57 @@ class FallbackProvider(LLMProvider):
             error_should_retry=True,
         )
 
+    @staticmethod
+    async def _call_provider(
+        call: Callable[[LLMProvider, dict[str, Any]], Awaitable[LLMResponse]],
+        provider: LLMProvider,
+        kwargs: dict[str, Any],
+    ) -> tuple[LLMResponse, Exception | None]:
+        """Turn provider exceptions into error responses without swallowing cancellation."""
+        try:
+            return await call(provider, kwargs), None
+        except asyncio.CancelledError:
+            raise
+        except Exception as exc:
+            error_name = type(exc).__name__.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")
+            ):
+                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
+
+            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_should_retry=error_should_retry,
+            ), exc
+
     async def _notify_fallback_model(self, model: str) -> None:
         if self._fallback_model_observer is None:
             return

tests/agent/test_runner_fallback.py

@@ -2,9 +2,11 @@
 
 from __future__ import annotations
 
+import asyncio
 from typing import Any
 from unittest.mock import AsyncMock, MagicMock, patch
 
+import httpx
 import pytest
 from loguru import logger
 
@@ -347,6 +349,190 @@ class TestNoFallbackWhenPrimarySucceeds:
         factory.assert_not_called()
 
 
+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__()
+        self.name = name
+        self._exc = exc if exc is not None else RuntimeError("GitHub Copilot is not logged in.")
+
+    def get_default_model(self) -> str:
+        return f"{self.name}/model"
+
+    async def chat(self, **kwargs: Any) -> LLMResponse:
+        raise self._exc
+
+    async def chat_stream(self, **kwargs: Any) -> LLMResponse:
+        raise self._exc
+
+
+class _StreamingThenRaisingProvider(_RaisingProvider):
+    async def chat_stream(self, **kwargs: Any) -> LLMResponse:
+        on_content_delta = kwargs.get("on_content_delta")
+        if on_content_delta:
+            await on_content_delta("partial")
+        raise self._exc
+
+
+class TestFallbackWhenPrimaryRaises:
+    @pytest.mark.parametrize(
+        "exc",
+        [TimeoutError(), httpx.ReadTimeout(""), httpx.ConnectError("")],
+        ids=["asyncio-timeout", "httpx-timeout", "connection"],
+    )
+    @pytest.mark.asyncio
+    async def test_transient_exception_triggers_fallback(self, exc: Exception) -> None:
+        """Transient exceptions remain eligible even when their messages are empty."""
+        primary = _RaisingProvider("primary", exc)
+        fallback = _FakeProvider("fallback", _make_response("fallback ok"))
+        factory = MagicMock(return_value=fallback)
+
+        fb = FallbackProvider(
+            primary=primary,
+            fallback_presets=[_fallback("fallback-a")],
+            provider_factory=factory,
+        )
+
+        result = await fb.chat(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_primary_exception_triggers_fallback(self) -> None:
+        """A primary whose chat() raises must not abort failover.
+
+        GitHubCopilotProvider.chat refreshes its token before the request and
+        raises when not logged in; the exception must be treated as a
+        fallbackable primary error, not swallow the whole fallback chain.
+        """
+        primary = _RaisingProvider("primary")
+        fallback = _FakeProvider("fallback", _make_response("fallback ok"))
+        factory = MagicMock(return_value=fallback)
+
+        fb = FallbackProvider(
+            primary=primary,
+            fallback_presets=[_fallback("fallback-a")],
+            provider_factory=factory,
+        )
+
+        result = await fb.chat(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.parametrize(
+        "exc",
+        [
+            ValueError("unexpected response shape"),
+            PermissionError("local provider file access failed"),
+        ],
+        ids=["value-error", "local-permission-error"],
+    )
+    @pytest.mark.asyncio
+    async def test_non_fallbackable_primary_exception_does_not_trigger_fallback(
+        self, exc: Exception,
+    ) -> None:
+        """An unrelated provider bug must not silently switch models."""
+        primary = _RaisingProvider("primary", exc)
+        factory = MagicMock(
+            return_value=_FakeProvider("fallback", _make_response("fallback"))
+        )
+        fb = FallbackProvider(
+            primary=primary,
+            fallback_presets=[_fallback("fallback-a")],
+            provider_factory=factory,
+        )
+
+        result = await fb.chat(
+            messages=[{"role": "user", "content": "hi"}],
+            model="primary-model",
+        )
+
+        assert result.finish_reason == "error"
+        assert result.content is not None
+        assert str(exc) in result.content
+        factory.assert_not_called()
+
+    @pytest.mark.asyncio
+    async def test_fallback_exception_advances_to_next_fallback(self) -> None:
+        """A fallback whose chat() raises must advance to the next fallback."""
+        primary = _FakeProvider("primary", _error_response())
+        raising_fb = _RaisingProvider("fb1")
+        ok_fb = _FakeProvider("fb2", _make_response("second fallback ok"))
+        factory = MagicMock(side_effect=[raising_fb, ok_fb])
+
+        fb = FallbackProvider(
+            primary=primary,
+            fallback_presets=[_fallback("fallback-a"), _fallback("fallback-b")],
+            provider_factory=factory,
+        )
+
+        result = await fb.chat(messages=[{"role": "user", "content": "hi"}], model="primary-model")
+        assert result.content == "second fallback ok"
+        assert result.finish_reason == "stop"
+
+    @pytest.mark.asyncio
+    async def test_primary_exception_stream_triggers_fallback(self) -> None:
+        """Streaming path: a raising primary still fails over when nothing streamed."""
+        primary = _RaisingProvider("primary")
+        fallback = _FakeProvider("fallback", _make_response("fallback ok"))
+        factory = MagicMock(return_value=fallback)
+
+        fb = FallbackProvider(
+            primary=primary,
+            fallback_presets=[_fallback("fallback-a")],
+            provider_factory=factory,
+        )
+
+        result = await fb.chat_stream(messages=[{"role": "user", "content": "hi"}], model="primary-model")
+        assert result.content == "fallback ok"
+        assert result.finish_reason == "stop"
+
+    @pytest.mark.asyncio
+    async def test_primary_exception_after_streaming_does_not_duplicate_output(self) -> None:
+        """Once content was emitted, an exception must not start a replacement stream."""
+        primary = _StreamingThenRaisingProvider("primary")
+        factory = MagicMock(return_value=_FakeProvider("fallback", _make_response("duplicate")))
+        deltas: list[str] = []
+
+        fb = FallbackProvider(
+            primary=primary,
+            fallback_presets=[_fallback("fallback-a")],
+            provider_factory=factory,
+        )
+
+        async def collect_delta(text: str) -> None:
+            deltas.append(text)
+
+        result = await fb.chat_stream(
+            messages=[{"role": "user", "content": "hi"}],
+            model="primary-model",
+            on_content_delta=collect_delta,
+        )
+
+        assert deltas == ["partial"]
+        assert result.finish_reason == "error"
+        factory.assert_not_called()
+
+    @pytest.mark.asyncio
+    async def test_primary_cancellation_is_not_converted_to_failover(self) -> None:
+        """Task cancellation must retain asyncio cancellation semantics."""
+        primary = _RaisingProvider("primary", asyncio.CancelledError())
+        factory = MagicMock(return_value=_FakeProvider("fallback", _make_response("fallback")))
+        fb = FallbackProvider(
+            primary=primary,
+            fallback_presets=[_fallback("fallback-a")],
+            provider_factory=factory,
+        )
+
+        with pytest.raises(asyncio.CancelledError):
+            await fb.chat(messages=[{"role": "user", "content": "hi"}])
+
+        factory.assert_not_called()
+
 class TestFallbackOnPrimaryError:
     @pytest.mark.asyncio
     async def test_first_fallback_succeeds(self) -> None: