mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-10 03:28:53 +00:00
[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
This commit is contained in:
parent
825ea65b96
commit
eb4bd26f24
5 changed files with 203 additions and 3 deletions
|
|
@ -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:
|
||||
|
|
|
|||
1
.github/workflows/test-litellm.yml
vendored
1
.github/workflows/test-litellm.yml
vendored
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
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
|
||||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
125
tests/test_litellm/proxy/test_fastapi_offline_routes.py
Normal file
125
tests/test_litellm/proxy/test_fastapi_offline_routes.py
Normal file
|
|
@ -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")
|
||||
Loading…
Add table
Reference in a new issue