From 19a7c6be15792866ce4e6b12bd83b944e928ba52 Mon Sep 17 00:00:00 2001 From: ly-wang19 Date: Mon, 29 Jun 2026 19:28:58 +0800 Subject: [PATCH] fix(skills): call raise_for_status before transforming skill responses The 8 skill HTTP handlers (create/list/get/delete x sync/async) passed the raw upstream response straight into transform_*_skill_response without calling raise_for_status(). When Anthropic returned an error response (e.g. 401 on an invalid key, 400 on a malformed request) the body shape {"type": "error", "error": ...} was force-parsed as a success payload and surfaced as a Pydantic ValidationError with HTTP 500, instead of as the proper upstream error. Reproduction (#31587 scenario 2): start the proxy with an invalid ANTHROPIC_API_KEY, GET /v1/skills returns 500 with "ValidationError: 1 validation error for ListSkillsResponse data Field required". Fix: add response.raise_for_status() between the HTTP call and the transform call in all 8 handlers. The raised httpx.HTTPStatusError flows through the existing _handle_error / litellm.exception_type path and is mapped to the appropriate APIError with the upstream status code and message preserved. Tests: tests/test_litellm/llms/custom_httpx/test_llm_http_handler_skills_raise_for_status.py covers all 8 handlers (sync + async) with an Anthropic-shaped error response, asserting that the transform never runs when raise_for_status fires. A success-path test confirms the transform still runs on 2xx. Refs #31587 --- litellm/llms/custom_httpx/llm_http_handler.py | 24 ++ ...lm_http_handler_skills_raise_for_status.py | 264 ++++++++++++++++++ 2 files changed, 288 insertions(+) create mode 100644 tests/test_litellm/llms/custom_httpx/test_llm_http_handler_skills_raise_for_status.py diff --git a/litellm/llms/custom_httpx/llm_http_handler.py b/litellm/llms/custom_httpx/llm_http_handler.py index d8f4856b4ad..a2a6b86dbc8 100644 --- a/litellm/llms/custom_httpx/llm_http_handler.py +++ b/litellm/llms/custom_httpx/llm_http_handler.py @@ -11479,6 +11479,9 @@ class BaseLLMHTTPHandler: provider_config=skills_api_provider_config, ) + # Raise on upstream HTTP errors (auth, rate-limit, etc.) before transform + # parses the error body as a success payload and raises ValidationError. (#31587) + response.raise_for_status() return skills_api_provider_config.transform_create_skill_response( raw_response=response, logging_obj=logging_obj, @@ -11535,6 +11538,9 @@ class BaseLLMHTTPHandler: provider_config=skills_api_provider_config, ) + # Raise on upstream HTTP errors (auth, rate-limit, etc.) before transform + # parses the error body as a success payload and raises ValidationError. (#31587) + response.raise_for_status() return skills_api_provider_config.transform_create_skill_response( raw_response=response, logging_obj=logging_obj, @@ -11594,6 +11600,9 @@ class BaseLLMHTTPHandler: provider_config=skills_api_provider_config, ) + # Raise on upstream HTTP errors (auth, rate-limit, etc.) before transform + # parses the error body as a success payload and raises ValidationError. (#31587) + response.raise_for_status() return skills_api_provider_config.transform_list_skills_response( raw_response=response, logging_obj=logging_obj, @@ -11641,6 +11650,9 @@ class BaseLLMHTTPHandler: provider_config=skills_api_provider_config, ) + # Raise on upstream HTTP errors (auth, rate-limit, etc.) before transform + # parses the error body as a success payload and raises ValidationError. (#31587) + response.raise_for_status() return skills_api_provider_config.transform_list_skills_response( raw_response=response, logging_obj=logging_obj, @@ -11697,6 +11709,9 @@ class BaseLLMHTTPHandler: provider_config=skills_api_provider_config, ) + # Raise on upstream HTTP errors (auth, rate-limit, etc.) before transform + # parses the error body as a success payload and raises ValidationError. (#31587) + response.raise_for_status() return skills_api_provider_config.transform_get_skill_response( raw_response=response, logging_obj=logging_obj, @@ -11742,6 +11757,9 @@ class BaseLLMHTTPHandler: provider_config=skills_api_provider_config, ) + # Raise on upstream HTTP errors (auth, rate-limit, etc.) before transform + # parses the error body as a success payload and raises ValidationError. (#31587) + response.raise_for_status() return skills_api_provider_config.transform_get_skill_response( raw_response=response, logging_obj=logging_obj, @@ -11798,6 +11816,9 @@ class BaseLLMHTTPHandler: provider_config=skills_api_provider_config, ) + # Raise on upstream HTTP errors (auth, rate-limit, etc.) before transform + # parses the error body as a success payload and raises ValidationError. (#31587) + response.raise_for_status() return skills_api_provider_config.transform_delete_skill_response( raw_response=response, logging_obj=logging_obj, @@ -11843,6 +11864,9 @@ class BaseLLMHTTPHandler: provider_config=skills_api_provider_config, ) + # Raise on upstream HTTP errors (auth, rate-limit, etc.) before transform + # parses the error body as a success payload and raises ValidationError. (#31587) + response.raise_for_status() return skills_api_provider_config.transform_delete_skill_response( raw_response=response, logging_obj=logging_obj, diff --git a/tests/test_litellm/llms/custom_httpx/test_llm_http_handler_skills_raise_for_status.py b/tests/test_litellm/llms/custom_httpx/test_llm_http_handler_skills_raise_for_status.py new file mode 100644 index 00000000000..ed05494aa15 --- /dev/null +++ b/tests/test_litellm/llms/custom_httpx/test_llm_http_handler_skills_raise_for_status.py @@ -0,0 +1,264 @@ +"""Regression tests for skills HTTP error surfacing (#31587). + +The skills handlers (create/list/get/delete, sync + async) used to pass the +raw upstream response straight into ``transform_*_skill_response`` without +calling ``raise_for_status()``. When Anthropic returned an error response +(e.g. 401 on an invalid key, 400 on a malformed request), the body shape +``{"type": "error", "error": ...}`` was force-parsed as a success payload +and surfaced as a Pydantic ValidationError with HTTP 500, instead of as a +proper upstream error. + +These tests pin the contract that an error-status response raises +``httpx.HTTPStatusError`` *before* the response transform runs. +""" + +from __future__ import annotations + +import os +import sys +from unittest.mock import MagicMock, patch + +import httpx +import pytest + +sys.path.insert(0, os.path.abspath("../../../../..")) + +from litellm.llms.anthropic.skills.transformation import AnthropicSkillsConfig +from litellm.llms.custom_httpx.llm_http_handler import BaseLLMHTTPHandler +from litellm.types.llms.anthropic_skills import ListSkillsResponse +from litellm.types.router import GenericLiteLLMParams + + +class _RecordingConfig(AnthropicSkillsConfig): + """Wrap the real Anthropic config so abstract methods stay satisfied, but + record whether ``transform_*_skill_response`` actually ran. + + A raise_for_status() failure must skip the transform entirely; if the + transform runs, the bug from #31587 is back. + """ + + def __init__(self) -> None: + self.transform_called = False + + def transform_list_skills_response(self, raw_response, logging_obj): # type: ignore[override] + self.transform_called = True + return ListSkillsResponse(data=[], has_more=False, next_page=None) + + def transform_create_skill_response(self, raw_response, logging_obj): # type: ignore[override] + self.transform_called = True + raise AssertionError("transform_create_skill_response must not run on error response") + + def transform_get_skill_response(self, raw_response, logging_obj): # type: ignore[override] + self.transform_called = True + raise AssertionError("transform_get_skill_response must not run on error response") + + def transform_delete_skill_response(self, raw_response, logging_obj): # type: ignore[override] + self.transform_called = True + raise AssertionError("transform_delete_skill_response must not run on error response") + + +def _error_response(status_code: int, method: str) -> httpx.Response: + """Anthropic-shaped error body — matches the issue's reproduction.""" + return httpx.Response( + status_code=status_code, + json={"type": "error", "error": {"type": "invalid_request_error", "message": "bad key"}}, + request=httpx.Request(method, "https://api.anthropic.com/v1/skills"), + ) + + +def _ok_response(method: str) -> httpx.Response: + return httpx.Response( + status_code=200, + json={"data": [], "has_more": False, "next_page": None}, + request=httpx.Request(method, "https://api.anthropic.com/v1/skills"), + ) + + +def _make_sync_client(response: httpx.Response): + client = MagicMock() + client.get = MagicMock(return_value=response) + client.post = MagicMock(return_value=response) + client.delete = MagicMock(return_value=response) + return client + + +def _make_async_client(response: httpx.Response): + async def _co(*a, **kw): + return response + + client = MagicMock() + client.get = MagicMock(return_value=_co()) + client.post = MagicMock(return_value=_co()) + client.delete = MagicMock(return_value=_co()) + return client + + +def _common_kwargs(): + return dict( + custom_llm_provider="anthropic", + litellm_params=GenericLiteLLMParams(), + logging_obj=MagicMock(), + ) + + +_HANDLER_MODULE = "litellm.llms.custom_httpx.llm_http_handler" + + +# --------------------------------------------------------------------------- +# Sync handlers +# --------------------------------------------------------------------------- + + +def test_list_skills_handler_raises_on_error_response(): + config = _RecordingConfig() + response = _error_response(401, "GET") + handler = BaseLLMHTTPHandler() + with patch(f"{_HANDLER_MODULE}._get_httpx_client", return_value=_make_sync_client(response)): + with pytest.raises(httpx.HTTPStatusError) as exc_info: + handler.list_skills_handler( + url="https://api.anthropic.com/v1/skills", + query_params={}, + skills_api_provider_config=config, + **_common_kwargs(), + ) + + assert exc_info.value.response.status_code == 401 + assert not config.transform_called, "transform must be skipped when raise_for_status fires" + + +def test_create_skills_handler_raises_on_error_response(): + config = _RecordingConfig() + response = _error_response(400, "POST") + handler = BaseLLMHTTPHandler() + with patch(f"{_HANDLER_MODULE}._get_httpx_client", return_value=_make_sync_client(response)): + with pytest.raises(httpx.HTTPStatusError): + handler.create_skill_handler( + url="https://api.anthropic.com/v1/skills", + request_body={"display_title": "x"}, + skills_api_provider_config=config, + **_common_kwargs(), + ) + + assert not config.transform_called + + +def test_get_skills_handler_raises_on_error_response(): + config = _RecordingConfig() + response = _error_response(404, "GET") + handler = BaseLLMHTTPHandler() + with patch(f"{_HANDLER_MODULE}._get_httpx_client", return_value=_make_sync_client(response)): + with pytest.raises(httpx.HTTPStatusError): + handler.get_skill_handler( + url="https://api.anthropic.com/v1/skills/skill_abc", + skills_api_provider_config=config, + **_common_kwargs(), + ) + + assert not config.transform_called + + +def test_delete_skills_handler_raises_on_error_response(): + config = _RecordingConfig() + response = _error_response(403, "DELETE") + handler = BaseLLMHTTPHandler() + with patch(f"{_HANDLER_MODULE}._get_httpx_client", return_value=_make_sync_client(response)): + with pytest.raises(httpx.HTTPStatusError): + handler.delete_skill_handler( + url="https://api.anthropic.com/v1/skills/skill_abc", + skills_api_provider_config=config, + **_common_kwargs(), + ) + + assert not config.transform_called + + +# --------------------------------------------------------------------------- +# Async handlers +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_async_list_skills_handler_raises_on_error_response(): + config = _RecordingConfig() + response = _error_response(401, "GET") + handler = BaseLLMHTTPHandler() + with patch(f"{_HANDLER_MODULE}.get_async_httpx_client", return_value=_make_async_client(response)): + with pytest.raises(httpx.HTTPStatusError): + await handler.async_list_skills_handler( + url="https://api.anthropic.com/v1/skills", + query_params={}, + skills_api_provider_config=config, + **_common_kwargs(), + ) + + assert not config.transform_called + + +@pytest.mark.asyncio +async def test_async_create_skills_handler_raises_on_error_response(): + config = _RecordingConfig() + response = _error_response(400, "POST") + handler = BaseLLMHTTPHandler() + with patch(f"{_HANDLER_MODULE}.get_async_httpx_client", return_value=_make_async_client(response)): + with pytest.raises(httpx.HTTPStatusError): + await handler.async_create_skill_handler( + url="https://api.anthropic.com/v1/skills", + request_body={"display_title": "x"}, + skills_api_provider_config=config, + **_common_kwargs(), + ) + + assert not config.transform_called + + +@pytest.mark.asyncio +async def test_async_get_skills_handler_raises_on_error_response(): + config = _RecordingConfig() + response = _error_response(404, "GET") + handler = BaseLLMHTTPHandler() + with patch(f"{_HANDLER_MODULE}.get_async_httpx_client", return_value=_make_async_client(response)): + with pytest.raises(httpx.HTTPStatusError): + await handler.async_get_skill_handler( + url="https://api.anthropic.com/v1/skills/skill_abc", + skills_api_provider_config=config, + **_common_kwargs(), + ) + + assert not config.transform_called + + +@pytest.mark.asyncio +async def test_async_delete_skills_handler_raises_on_error_response(): + config = _RecordingConfig() + response = _error_response(403, "DELETE") + handler = BaseLLMHTTPHandler() + with patch(f"{_HANDLER_MODULE}.get_async_httpx_client", return_value=_make_async_client(response)): + with pytest.raises(httpx.HTTPStatusError): + await handler.async_delete_skill_handler( + url="https://api.anthropic.com/v1/skills/skill_abc", + skills_api_provider_config=config, + **_common_kwargs(), + ) + + assert not config.transform_called + + +# --------------------------------------------------------------------------- +# Sanity: success response still flows through to transform +# --------------------------------------------------------------------------- + + +def test_list_skills_handler_runs_transform_on_success(): + config = _RecordingConfig() + response = _ok_response("GET") + handler = BaseLLMHTTPHandler() + with patch(f"{_HANDLER_MODULE}._get_httpx_client", return_value=_make_sync_client(response)): + result = handler.list_skills_handler( + url="https://api.anthropic.com/v1/skills", + query_params={}, + skills_api_provider_config=config, + **_common_kwargs(), + ) + + assert config.transform_called + assert isinstance(result, ListSkillsResponse)