mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-07 08:26:10 +00:00
fix(tags): preserve HTTPException status codes in tag CRUD handlers
new_tag, update_tag, info_tag, and delete_tag each raise an intentional HTTPException (400 for a duplicate tag, 404 for a missing one) inside a try block, then catch it again in a bare except Exception with no preceding except HTTPException: raise, so every one of those intentional client errors gets rewrapped as a 500 Add except HTTPException: raise before the generic except Exception in all four handlers so the original status code and detail survive Issue #11884 reported this defect in new_tag via an unresolvable model_id and was closed by widening the input instead of fixing the exception handling; this change fixes the underlying pattern there and in update_tag, info_tag, and delete_tag, which #11884 never touched Add regression tests for all four handlers that mock a duplicate or missing tag record and assert the raised HTTPException keeps its 400/404 status code instead of becoming a 500
This commit is contained in:
parent
bf02a4a47f
commit
acf8932fb8
2 changed files with 141 additions and 1 deletions
|
|
@ -241,6 +241,8 @@ async def new_tag(
|
|||
"message": f"Tag {tag.name} created successfully",
|
||||
"tag": tag_config,
|
||||
}
|
||||
except HTTPException:
|
||||
raise
|
||||
except Exception as e:
|
||||
verbose_proxy_logger.exception(f"Error creating tag: {str(e)}")
|
||||
raise HTTPException(status_code=500, detail=str(e))
|
||||
|
|
@ -371,6 +373,8 @@ async def update_tag(
|
|||
"message": f"Tag {tag.name} updated successfully",
|
||||
"tag": tag_config,
|
||||
}
|
||||
except HTTPException:
|
||||
raise
|
||||
except Exception as e:
|
||||
verbose_proxy_logger.exception(f"Error updating tag: {str(e)}")
|
||||
raise HTTPException(status_code=500, detail=str(e))
|
||||
|
|
@ -437,6 +441,8 @@ async def info_tag(
|
|||
requested_tags[tag_record.tag_name] = tag_dict
|
||||
|
||||
return requested_tags
|
||||
except HTTPException:
|
||||
raise
|
||||
except Exception as e:
|
||||
raise HTTPException(status_code=500, detail=str(e))
|
||||
|
||||
|
|
@ -606,6 +612,8 @@ async def delete_tag(
|
|||
await TagRepository(prisma_client).table.delete(where={"tag_name": data.name})
|
||||
|
||||
return {"message": f"Tag {data.name} deleted successfully"}
|
||||
except HTTPException:
|
||||
raise
|
||||
except Exception as e:
|
||||
raise HTTPException(status_code=500, detail=str(e))
|
||||
|
||||
|
|
|
|||
|
|
@ -16,7 +16,12 @@ from unittest.mock import patch
|
|||
import litellm
|
||||
from litellm.proxy._types import LitellmUserRoles, UserAPIKeyAuth
|
||||
from litellm.proxy.proxy_server import app
|
||||
from litellm.types.tag_management import TagDeleteRequest, TagInfoRequest, TagNewRequest
|
||||
from litellm.types.tag_management import (
|
||||
TagDeleteRequest,
|
||||
TagInfoRequest,
|
||||
TagNewRequest,
|
||||
TagUpdateRequest,
|
||||
)
|
||||
|
||||
client = TestClient(app)
|
||||
|
||||
|
|
@ -250,6 +255,133 @@ async def test_delete_tag():
|
|||
app.dependency_overrides.clear()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_new_tag_duplicate_returns_400_not_500():
|
||||
"""
|
||||
Regression test: new_tag() must preserve a 400 for a duplicate tag name
|
||||
instead of letting a bare `except Exception` downgrade it to 500.
|
||||
"""
|
||||
from unittest.mock import AsyncMock, Mock
|
||||
|
||||
from litellm.proxy.management_endpoints.tag_management_endpoints import new_tag
|
||||
|
||||
mock_user_auth = UserAPIKeyAuth(
|
||||
user_id="test-user-123",
|
||||
user_role=LitellmUserRoles.PROXY_ADMIN,
|
||||
)
|
||||
|
||||
with (
|
||||
patch("litellm.proxy.proxy_server.prisma_client") as mock_prisma,
|
||||
patch("litellm.proxy.proxy_server.llm_router", Mock()),
|
||||
):
|
||||
mock_db = Mock()
|
||||
mock_prisma.db = mock_db
|
||||
mock_db.litellm_tagtable.find_unique = AsyncMock(return_value=Mock())
|
||||
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
await new_tag(
|
||||
tag=TagNewRequest(name="existing-tag"),
|
||||
user_api_key_dict=mock_user_auth,
|
||||
)
|
||||
|
||||
assert exc_info.value.status_code == 400
|
||||
assert "already exists" in exc_info.value.detail
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_tag_missing_returns_404_not_500():
|
||||
"""
|
||||
Regression test: update_tag() must preserve a 404 for a missing tag
|
||||
instead of letting a bare `except Exception` downgrade it to 500.
|
||||
"""
|
||||
from unittest.mock import AsyncMock, Mock
|
||||
|
||||
from litellm.proxy.management_endpoints.tag_management_endpoints import (
|
||||
update_tag,
|
||||
)
|
||||
|
||||
mock_user_auth = UserAPIKeyAuth(
|
||||
user_id="test-user-123",
|
||||
user_role=LitellmUserRoles.PROXY_ADMIN,
|
||||
)
|
||||
|
||||
with patch("litellm.proxy.proxy_server.prisma_client") as mock_prisma:
|
||||
mock_db = Mock()
|
||||
mock_prisma.db = mock_db
|
||||
mock_db.litellm_tagtable.find_unique = AsyncMock(return_value=None)
|
||||
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
await update_tag(
|
||||
tag=TagUpdateRequest(name="missing-tag"),
|
||||
user_api_key_dict=mock_user_auth,
|
||||
)
|
||||
|
||||
assert exc_info.value.status_code == 404
|
||||
assert "not found" in exc_info.value.detail
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_info_tag_missing_returns_404_not_500():
|
||||
"""
|
||||
Regression test: info_tag() must preserve a 404 for missing tags
|
||||
instead of letting a bare `except Exception` downgrade it to 500.
|
||||
"""
|
||||
from unittest.mock import AsyncMock, Mock
|
||||
|
||||
from litellm.proxy.management_endpoints.tag_management_endpoints import info_tag
|
||||
|
||||
mock_user_auth = UserAPIKeyAuth(
|
||||
user_id="test-user-123",
|
||||
user_role=LitellmUserRoles.PROXY_ADMIN,
|
||||
)
|
||||
|
||||
with patch("litellm.proxy.proxy_server.prisma_client") as mock_prisma:
|
||||
mock_db = Mock()
|
||||
mock_prisma.db = mock_db
|
||||
mock_db.litellm_tagtable.find_many = AsyncMock(return_value=[])
|
||||
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
await info_tag(
|
||||
data=TagInfoRequest(names=["missing-tag"]),
|
||||
user_api_key_dict=mock_user_auth,
|
||||
)
|
||||
|
||||
assert exc_info.value.status_code == 404
|
||||
assert "not found" in exc_info.value.detail
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_delete_tag_missing_returns_404_not_500():
|
||||
"""
|
||||
Regression test: delete_tag() must preserve a 404 for a missing tag
|
||||
instead of letting a bare `except Exception` downgrade it to 500.
|
||||
"""
|
||||
from unittest.mock import AsyncMock, Mock
|
||||
|
||||
from litellm.proxy.management_endpoints.tag_management_endpoints import (
|
||||
delete_tag,
|
||||
)
|
||||
|
||||
mock_user_auth = UserAPIKeyAuth(
|
||||
user_id="test-user-123",
|
||||
user_role=LitellmUserRoles.PROXY_ADMIN,
|
||||
)
|
||||
|
||||
with patch("litellm.proxy.proxy_server.prisma_client") as mock_prisma:
|
||||
mock_db = Mock()
|
||||
mock_prisma.db = mock_db
|
||||
mock_db.litellm_tagtable.find_unique = AsyncMock(return_value=None)
|
||||
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
await delete_tag(
|
||||
data=TagDeleteRequest(name="missing-tag"),
|
||||
user_api_key_dict=mock_user_auth,
|
||||
)
|
||||
|
||||
assert exc_info.value.status_code == 404
|
||||
assert "not found" in exc_info.value.detail
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_list_tags_with_dynamic_tags():
|
||||
"""
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue