From eb4bd26f2490c943b6b291545c4e76e80a69816b Mon Sep 17 00:00:00 2001 From: Ishaan Jaff Date: Sat, 9 Aug 2025 12:52:23 -0700 Subject: [PATCH] [Bug Fix] - Get Routes (#13466) * fixes get_routes_for_mounted_app * fix - use _safe_get_endpoint_name * fix code QA check * test_get_routes_for_mounted_app_with_static_files * test fixes --- .circleci/config.yml | 1 + .github/workflows/test-litellm.yml | 1 + litellm/proxy/common_utils/get_routes.py | 25 +++- .../proxy/common_utils/test_get_routes.py | 54 ++++++++ .../proxy/test_fastapi_offline_routes.py | 125 ++++++++++++++++++ 5 files changed, 203 insertions(+), 3 deletions(-) create mode 100644 tests/test_litellm/proxy/test_fastapi_offline_routes.py diff --git a/.circleci/config.yml b/.circleci/config.yml index bf1d33c618..a89fedc751 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -957,6 +957,7 @@ jobs: pip install "responses==0.25.7" pip install "pytest-xdist==3.6.1" pip install "semantic_router==0.1.10" + pip install "fastapi-offline==1.7.3" - setup_litellm_enterprise_pip # Run pytest and generate JUnit XML report - run: diff --git a/.github/workflows/test-litellm.yml b/.github/workflows/test-litellm.yml index 2f6e81c8ce..4ec3dcbb4c 100644 --- a/.github/workflows/test-litellm.yml +++ b/.github/workflows/test-litellm.yml @@ -31,6 +31,7 @@ jobs: poetry run pip install "pytest-retry==1.6.3" poetry run pip install pytest-xdist poetry run pip install "google-genai==1.22.0" + poetry run pip install "fastapi-offline==1.7.3" - name: Setup litellm-enterprise as local package run: | cd enterprise diff --git a/litellm/proxy/common_utils/get_routes.py b/litellm/proxy/common_utils/get_routes.py index 19465675c1..bf3773037e 100644 --- a/litellm/proxy/common_utils/get_routes.py +++ b/litellm/proxy/common_utils/get_routes.py @@ -2,10 +2,12 @@ Utility class for getting routes from a FastAPI app. """ -from typing import Any, Dict, List +from typing import Any, Dict, List, Optional from starlette.routing import BaseRoute +from litellm._logging import verbose_logger + class GetRoutes: @staticmethod @@ -53,8 +55,25 @@ class GetRoutes: "path": full_path, "methods": getattr(sub_route, "methods", ["GET", "POST"]), "name": getattr(sub_route, "name", None), - "endpoint": endpoint_func.__name__ if callable(endpoint_func) else None, + "endpoint": GetRoutes._safe_get_endpoint_name(endpoint_func), "mounted_app": True, } routes.append(route_info) - return routes \ No newline at end of file + return routes + + + @staticmethod + def _safe_get_endpoint_name(endpoint_function: Any) -> Optional[str]: + """ + Safely get the name of the endpoint function. + """ + try: + if hasattr(endpoint_function, '__name__'): + return getattr(endpoint_function, '__name__') + elif hasattr(endpoint_function, '__class__') and hasattr(endpoint_function.__class__, '__name__'): + return getattr(endpoint_function.__class__, '__name__') + else: + return None + except Exception: + verbose_logger.exception(f"Error getting endpoint name for route: {endpoint_function}") + return None \ No newline at end of file diff --git a/tests/test_litellm/proxy/common_utils/test_get_routes.py b/tests/test_litellm/proxy/common_utils/test_get_routes.py index 48eadffe2e..210e044e75 100644 --- a/tests/test_litellm/proxy/common_utils/test_get_routes.py +++ b/tests/test_litellm/proxy/common_utils/test_get_routes.py @@ -166,3 +166,57 @@ class TestGetRoutes: assert mount_route["endpoint"] == "handle_streamable_http_mcp" assert mount_route["mounted_app"] is True + def test_get_routes_for_mounted_app_with_static_files(self): + """ + Test getting routes for mounted app with StaticFiles object (reproduces AttributeError bug). + + This test reproduces the exact stacktrace scenario: + AttributeError: 'StaticFiles' object has no attribute '__name__'. Did you mean: '__ne__'? + + The original bug occurred when the code tried to access endpoint_func.__name__ + directly on a StaticFiles object. The fix uses _safe_get_endpoint_name() which + gracefully handles objects without __name__ by falling back to class name. + """ + # Mock the main mount route (e.g., /ui) + mock_mount_route = Mock() + mock_mount_route.path = "/ui" + + # Mock sub-app with routes + mock_sub_app = Mock() + mock_sub_app.routes = [] + + # Create a mock StaticFiles route (this is the problematic case) + mock_static_route = Mock(spec=['path', 'name', 'endpoint', 'app']) + mock_static_route.path = "" + mock_static_route.name = "ui" + mock_static_route.endpoint = None + + # Mock StaticFiles object - this is the key part that caused the AttributeError + # Real StaticFiles objects don't have __name__ attribute + # Create a mock that simulates StaticFiles behavior (no __name__ attribute) + class StaticFiles: + """Mock class that simulates real StaticFiles without __name__ attribute""" + pass + + mock_static_files = StaticFiles() + # Verify no __name__ attribute exists on the instance (reproduces bug condition) + assert not hasattr(mock_static_files, '__name__') + + mock_static_route.app = mock_static_files + + mock_sub_app.routes.append(mock_static_route) + mock_mount_route.app = mock_sub_app + + # This should NOT raise AttributeError thanks to _safe_get_endpoint_name + # In the old code, this would fail with: 'StaticFiles' object has no attribute '__name__' + result = GetRoutes.get_routes_for_mounted_app(mock_mount_route) + + # Should handle StaticFiles gracefully without throwing AttributeError + assert len(result) == 1 + assert result[0]["path"] == "/ui" + assert result[0]["methods"] == ["GET", "POST"] # Default methods + assert result[0]["name"] == "ui" + # Should fall back to class name since instance doesn't have __name__ attribute + assert result[0]["endpoint"] == "StaticFiles" # Falls back to class name + assert result[0]["mounted_app"] is True + diff --git a/tests/test_litellm/proxy/test_fastapi_offline_routes.py b/tests/test_litellm/proxy/test_fastapi_offline_routes.py new file mode 100644 index 0000000000..71d26ad3dd --- /dev/null +++ b/tests/test_litellm/proxy/test_fastapi_offline_routes.py @@ -0,0 +1,125 @@ +""" +Unit test for testing /routes endpoint with FastAPIOffline app initialization. + +This test verifies that the /routes endpoint works correctly when the proxy +server is initialized using FastAPIOffline instead of regular FastAPI. +""" + +import os +import sys + +sys.path.insert( + 0, os.path.abspath("../../..") +) # Adds the parent directory to the system path + +import pytest +from fastapi.testclient import TestClient +from fastapi_offline import FastAPIOffline + + +class TestFastAPIOfflineRoutes: + """Test that /routes endpoint works with FastAPIOffline app initialization.""" + + def test_routes_endpoint_with_fastapi_offline(self): + """ + Test that /routes endpoint responds correctly when using FastAPIOffline. + + This test verifies that when the proxy server app is initialized using + FastAPIOffline instead of regular FastAPI, the /routes endpoint still + functions properly without throwing the StaticFiles AttributeError. + """ + from litellm.proxy.proxy_server import router + + # Initialize app using FastAPIOffline instead of regular FastAPI + app = FastAPIOffline() + + # Add a simple root endpoint to verify app is working + @app.get("/") + async def root(): + return {"message": "Hello World"} + + # Include the litellm proxy router which contains the /routes endpoint + app.include_router(router) + + # Create test client + client = TestClient(app) + + # Test the root endpoint first to ensure app is working + response = client.get("/") + assert response.status_code == 200 + assert response.json() == {"message": "Hello World"} + + # Test the /routes endpoint - this should not fail even with FastAPIOffline + # The important part is that it doesn't fail with the StaticFiles AttributeError + response = client.get("/routes") + + # Print response for debugging + print(f"Response status: {response.status_code}") + print(f"Response content: {response.text}") + + # The key test: we should NOT get a 500 (Internal Server Error) + # which would indicate the StaticFiles AttributeError bug + assert response.status_code != 500, f"Got 500 error: {response.text}" + + # We accept either 200 (success) or 401 (auth required) - both are valid + assert response.status_code in [200, 401], f"Unexpected status: {response.status_code}" + + if response.status_code == 200: + # If successful, verify it has the expected structure + response_json = response.json() + assert "routes" in response_json + assert isinstance(response_json["routes"], list) + print("✓ /routes endpoint returns valid routes data with FastAPIOffline") + else: + # If auth fails, ensure it's a proper JSON error response + response_json = response.json() + assert "detail" in response_json + print("✓ /routes endpoint handles auth properly with FastAPIOffline") + + # If we get here without any AttributeError exceptions, the fix is working + print("✓ /routes endpoint handles FastAPIOffline initialization correctly") + + def test_routes_endpoint_with_auth_token_fastapi_offline(self): + """ + Test /routes endpoint with auth token using FastAPIOffline. + + This test provides a mock auth token to actually test the routes response. + """ + from unittest.mock import patch + + from litellm.proxy.proxy_server import router + + # Initialize app using FastAPIOffline + app = FastAPIOffline() + + @app.get("/") + async def root(): + return {"message": "Hello World"} + + app.include_router(router) + client = TestClient(app) + + # Mock the authentication to bypass the auth requirement + with patch('litellm.proxy.auth.user_api_key_auth.user_api_key_auth') as mock_auth: + # Configure mock to return a successful auth response + mock_auth.return_value = {"user_id": "test_user", "api_key": "test_key"} + + # Test with Authorization header + headers = {"Authorization": "Bearer sk-test-token"} + response = client.get("/routes", headers=headers) + + # If authentication is properly mocked, we should get a 200 response + # If not, we might get 401, but we should NOT get 500 (AttributeError) + assert response.status_code in [200, 401], f"Unexpected status code: {response.status_code}" + + if response.status_code == 200: + # If we get a successful response, verify it has the expected structure + response_json = response.json() + assert "routes" in response_json + assert isinstance(response_json["routes"], list) + print("✓ /routes endpoint returns valid response with FastAPIOffline") + else: + # Even if auth fails, ensure it's a proper JSON error response + response_json = response.json() + assert "detail" in response_json + print("✓ /routes endpoint handles auth properly with FastAPIOffline") \ No newline at end of file