diff --git a/app/api/openai.py b/app/api/openai.py index 025cc9b4..6101ae07 100644 --- a/app/api/openai.py +++ b/app/api/openai.py @@ -15,6 +15,28 @@ logger = logging.getLogger(__name__) router = APIRouter() +def _get_exception_chain_detail(exc: Exception) -> str: + """ + Extract a verbose diagnostic message by walking the full exception chain. + + Surfaces DNS resolution failures, TCP connection refused errors, SSL issues, + and other low-level network problems that are normally hidden behind a generic + 'Connection error.' message. + """ + parts: list[str] = [str(exc)] + + cause = getattr(exc, "__cause__", None) or getattr(exc, "__context__", None) + seen: set[int] = {id(exc)} + while cause is not None and id(cause) not in seen: + seen.add(id(cause)) + cause_str = str(cause) + if cause_str and cause_str not in parts: + parts.append(f"caused by: {type(cause).__name__}: {cause_str}") + cause = getattr(cause, "__cause__", None) or getattr(cause, "__context__", None) + + return " | ".join(parts) + + @router.get("/openai/test") @require_login async def test_openai_connection(request: Request): @@ -46,9 +68,66 @@ async def test_openai_connection(request: Request): "message": "OpenAI API key is valid", "models_available": len(models.data) if hasattr(models, "data") else "Unknown", } + + except openai.APITimeoutError as e: + detail = _get_exception_chain_detail(e) + logger.error(f"OpenAI API request timed out: {detail}", exc_info=True) + return { + "status": "error", + "message": f"Request timed out: {detail}", + "is_auth_error": False, + "error_type": "timeout", + } + + except openai.APIConnectionError as e: + detail = _get_exception_chain_detail(e) + base_url = getattr(getattr(client, "_client", None), "base_url", None) + base_url_info = f" (base_url: {base_url})" if base_url else "" + logger.error( + f"OpenAI API connection error{base_url_info}: {detail}", + exc_info=True, + ) + return { + "status": "error", + "message": f"Connection error{base_url_info}: {detail}", + "is_auth_error": False, + "error_type": "connection_error", + } + + except openai.AuthenticationError as e: + logger.error(f"OpenAI authentication error (status {e.status_code}): {e.message}", exc_info=True) + return { + "status": "error", + "message": f"Authentication failed: {e.message}", + "is_auth_error": True, + "error_type": "authentication_error", + } + + except openai.RateLimitError as e: + logger.warning(f"OpenAI rate limit exceeded (status {e.status_code}): {e.message}") + return { + "status": "error", + "message": f"Rate limit exceeded: {e.message}", + "is_auth_error": False, + "error_type": "rate_limit", + } + + except openai.APIStatusError as e: + logger.error( + f"OpenAI API returned HTTP {e.status_code}: {e.message} | " + f"request_id={e.response.headers.get('x-request-id', 'n/a')}" + ) + return { + "status": "error", + "message": f"API error (HTTP {e.status_code}): {e.message}", + "is_auth_error": e.status_code == 401, + "error_type": "api_status_error", + "http_status": e.status_code, + } + except Exception as e: error_msg = str(e) - logger.error(f"OpenAI API key test failed: {error_msg}") + logger.error(f"OpenAI API key test failed: {error_msg}", exc_info=True) # Determine if this is an authentication error is_auth_error = "auth" in error_msg.lower() or "api key" in error_msg.lower() diff --git a/tests/test_api_openai.py b/tests/test_api_openai.py index 6bcbe94d..b730e130 100644 --- a/tests/test_api_openai.py +++ b/tests/test_api_openai.py @@ -62,34 +62,143 @@ class TestOpenAIConnectionErrors: @patch("app.api.openai.settings") def test_openai_api_key_validation_auth_error(self, mock_settings, mock_openai_class, client): """Test OpenAI API key validation with auth error.""" + import openai as openai_module + mock_settings.openai_api_key = "sk-invalid-key" mock_client = MagicMock() - mock_client.models.list.side_effect = Exception("Incorrect API key") + auth_exc = openai_module.AuthenticationError( + message="Incorrect API key provided", + response=MagicMock(status_code=401, headers={}), + body={"error": {"message": "Incorrect API key provided"}}, + ) + mock_client.models.list.side_effect = auth_exc mock_openai_class.return_value = mock_client response = client.get("/api/openai/test") data = response.json() assert data["status"] == "error" - assert "api key" in data["message"].lower() or "validation failed" in data["message"].lower() assert data.get("is_auth_error") is True + assert data.get("error_type") == "authentication_error" @patch("openai.OpenAI") @patch("app.api.openai.settings") def test_openai_api_key_validation_network_error(self, mock_settings, mock_openai_class, client): - """Test OpenAI API key validation with network error.""" + """Test OpenAI API key validation with network/connection error.""" + import openai as openai_module + mock_settings.openai_api_key = "sk-test-key" mock_client = MagicMock() - mock_client.models.list.side_effect = Exception("Network timeout") + conn_exc = openai_module.APIConnectionError(request=MagicMock()) + mock_client.models.list.side_effect = conn_exc mock_openai_class.return_value = mock_client response = client.get("/api/openai/test") data = response.json() assert data["status"] == "error" - # Not an auth error, so is_auth_error should be False + assert data.get("is_auth_error") is False + assert data.get("error_type") == "connection_error" + assert "connection error" in data["message"].lower() + + @patch("openai.OpenAI") + @patch("app.api.openai.settings") + def test_openai_api_connection_error_with_dns_cause(self, mock_settings, mock_openai_class, client): + """Test that DNS resolution failures are surfaced in the connection error message.""" + import openai as openai_module + + mock_settings.openai_api_key = "sk-test-key" + + dns_err = OSError("[Errno -2] Name or service not known") + conn_exc = openai_module.APIConnectionError(request=MagicMock()) + conn_exc.__cause__ = dns_err + mock_client = MagicMock() + mock_client.models.list.side_effect = conn_exc + mock_openai_class.return_value = mock_client + + response = client.get("/api/openai/test") + data = response.json() + + assert data["status"] == "error" + assert data.get("error_type") == "connection_error" + # The DNS detail should be propagated to the message + assert "name or service not known" in data["message"].lower() + + @patch("openai.OpenAI") + @patch("app.api.openai.settings") + def test_openai_api_timeout_error(self, mock_settings, mock_openai_class, client): + """Test OpenAI API key validation with timeout error.""" + import openai as openai_module + + mock_settings.openai_api_key = "sk-test-key" + + mock_client = MagicMock() + timeout_exc = openai_module.APITimeoutError(request=MagicMock()) + mock_client.models.list.side_effect = timeout_exc + mock_openai_class.return_value = mock_client + + response = client.get("/api/openai/test") + data = response.json() + + assert data["status"] == "error" + assert data.get("is_auth_error") is False + assert data.get("error_type") == "timeout" + assert "timed out" in data["message"].lower() + + @patch("openai.OpenAI") + @patch("app.api.openai.settings") + def test_openai_api_status_error(self, mock_settings, mock_openai_class, client): + """Test OpenAI API returning an unexpected HTTP status code.""" + import openai as openai_module + + mock_settings.openai_api_key = "sk-test-key" + + mock_response = MagicMock() + mock_response.status_code = 500 + mock_response.headers = {"x-request-id": "test-req-id"} + status_exc = openai_module.InternalServerError( + message="Internal Server Error", + response=mock_response, + body={"error": {"message": "Internal Server Error"}}, + ) + mock_client = MagicMock() + mock_client.models.list.side_effect = status_exc + mock_openai_class.return_value = mock_client + + response = client.get("/api/openai/test") + data = response.json() + + assert data["status"] == "error" + assert data.get("error_type") == "api_status_error" + assert data.get("http_status") == 500 + + @patch("openai.OpenAI") + @patch("app.api.openai.settings") + def test_openai_api_rate_limit_error(self, mock_settings, mock_openai_class, client): + """Test OpenAI API returning a rate-limit error.""" + import openai as openai_module + + mock_settings.openai_api_key = "sk-test-key" + + mock_response = MagicMock() + mock_response.status_code = 429 + mock_response.headers = {} + rate_exc = openai_module.RateLimitError( + message="Rate limit exceeded", + response=mock_response, + body={"error": {"message": "Rate limit exceeded"}}, + ) + mock_client = MagicMock() + mock_client.models.list.side_effect = rate_exc + mock_openai_class.return_value = mock_client + + response = client.get("/api/openai/test") + data = response.json() + + assert data["status"] == "error" + assert data.get("error_type") == "rate_limit" assert data.get("is_auth_error") is False @patch("app.api.openai.settings") @@ -138,3 +247,35 @@ class TestOpenAIConnectionErrors: assert data["status"] == "error" assert "unexpected error" in data["message"].lower() + + @patch("openai.OpenAI") + @patch("app.api.openai.settings") + def test_openai_generic_error_still_detects_auth(self, mock_settings, mock_openai_class, client): + """Test that generic exception fallback still detects auth-related errors.""" + mock_settings.openai_api_key = "sk-test-key" + + mock_client = MagicMock() + mock_client.models.list.side_effect = Exception("Incorrect API key provided") + mock_openai_class.return_value = mock_client + + response = client.get("/api/openai/test") + data = response.json() + + assert data["status"] == "error" + assert data.get("is_auth_error") is True + + @patch("openai.OpenAI") + @patch("app.api.openai.settings") + def test_openai_generic_network_error_not_auth(self, mock_settings, mock_openai_class, client): + """Test that generic network errors are not flagged as auth errors.""" + mock_settings.openai_api_key = "sk-test-key" + + mock_client = MagicMock() + mock_client.models.list.side_effect = Exception("Network timeout") + mock_openai_class.return_value = mock_client + + response = client.get("/api/openai/test") + data = response.json() + + assert data["status"] == "error" + assert data.get("is_auth_error") is False