diff --git a/enterprise/litellm_enterprise/enterprise_callbacks/pagerduty/pagerduty.py b/enterprise/litellm_enterprise/enterprise_callbacks/pagerduty/pagerduty.py index f920aa7ac13..a0c896b7e05 100644 --- a/enterprise/litellm_enterprise/enterprise_callbacks/pagerduty/pagerduty.py +++ b/enterprise/litellm_enterprise/enterprise_callbacks/pagerduty/pagerduty.py @@ -17,6 +17,7 @@ from typing import List, Optional, Union from litellm._logging import verbose_logger from litellm.caching import DualCache +from litellm.exceptions import MissingMessageRoleError from litellm.integrations.SlackAlerting.slack_alerting import SlackAlerting from litellm.llms.custom_httpx.http_handler import ( AsyncHTTPHandler, @@ -86,6 +87,8 @@ class PagerDutyAlerting(SlackAlerting): Record a failure event. Only send an alert to PagerDuty if the configured *failure* threshold is exceeded in the specified window. """ + if isinstance(kwargs.get("exception"), MissingMessageRoleError): + return now = datetime.now(timezone.utc) standard_logging_payload: Optional[StandardLoggingPayload] = kwargs.get( "standard_logging_object" diff --git a/litellm/__init__.py b/litellm/__init__.py index 8a73f3447b8..bfe0e2b5f3e 100644 --- a/litellm/__init__.py +++ b/litellm/__init__.py @@ -1457,6 +1457,7 @@ from .exceptions import ( BadRequestError, ImageFetchError, MalformedRequestFieldError as MalformedRequestFieldError, + MissingMessageRoleError as MissingMessageRoleError, VectorStoreSearchError, NotFoundError, PermissionDeniedError, diff --git a/litellm/exceptions.py b/litellm/exceptions.py index 56a5f136b62..f8c6a50a589 100644 --- a/litellm/exceptions.py +++ b/litellm/exceptions.py @@ -369,6 +369,10 @@ class MalformedRequestFieldError(BadRequestError): pass +class MissingMessageRoleError(MalformedRequestFieldError): + pass + + class UnsupportedEndpointError(BadRequestError): pass diff --git a/litellm/main.py b/litellm/main.py index 2d6cd18162b..df8e8f29483 100644 --- a/litellm/main.py +++ b/litellm/main.py @@ -5660,7 +5660,9 @@ def completion( if model is None: raise ValueError("model param not passed in.") # validate messages - messages = validate_and_fix_openai_messages(messages=messages) + messages = validate_and_fix_openai_messages( + messages=messages, model=model, request_kwargs=_REQUEST_KWARGS_ADAPTER.validate_python(kwargs) + ) tools = validate_and_fix_openai_tools(tools=tools) # validate tool_choice requested_tool_choice: Final = cast( # cast-ok: widening an untyped dict to object is always sound diff --git a/litellm/proxy/utils.py b/litellm/proxy/utils.py index 51f1c8a614d..eac2700551f 100644 --- a/litellm/proxy/utils.py +++ b/litellm/proxy/utils.py @@ -151,6 +151,7 @@ from litellm.caching.dual_cache import LimitedSizeOrderedDict from litellm.caching.in_memory_cache import InMemoryCache from litellm.exceptions import ( GuardrailRaisedException, + MissingMessageRoleError, RejectedRequestError, SensitiveDataRouteException, ) @@ -671,6 +672,8 @@ class _BudgetAlertEmailer(Protocol): def _is_client_error_exception(exc: Exception) -> bool: + if isinstance(exc, MissingMessageRoleError): + return True if isinstance(exc, HTTPException): return exc.status_code < 500 if isinstance(exc, ProxyException): diff --git a/litellm/router_utils/cooldown_handlers.py b/litellm/router_utils/cooldown_handlers.py index 190ef43a3e3..22bec9e1ead 100644 --- a/litellm/router_utils/cooldown_handlers.py +++ b/litellm/router_utils/cooldown_handlers.py @@ -24,7 +24,7 @@ from litellm.constants import ( INTERNAL_CALL_ORIGIN_METADATA_KEY, SINGLE_DEPLOYMENT_TRAFFIC_FAILURE_THRESHOLD, ) -from litellm.exceptions import MidStreamFallbackError +from litellm.exceptions import MidStreamFallbackError, MissingMessageRoleError from litellm.litellm_core_utils.credential_accessor import CredentialAccessor from litellm.litellm_core_utils.litellm_logging import mask_api_base_credentials from litellm.router_deprecated import renamed_router_param @@ -83,8 +83,10 @@ def mark_deployment_selection_failure(exception: BaseException) -> None: def is_failure_outside_selected_deployment(exception: BaseException | None) -> bool: - return is_advisor_orchestration_failure(exception) or bool( - getattr(exception, _DEPLOYMENT_SELECTION_FAILURE_ATTR, False) + return ( + is_advisor_orchestration_failure(exception) + or isinstance(exception, MissingMessageRoleError) + or bool(getattr(exception, _DEPLOYMENT_SELECTION_FAILURE_ATTR, False)) ) diff --git a/litellm/router_utils/handle_error.py b/litellm/router_utils/handle_error.py index a1e6d2b2cad..319959b5cfd 100644 --- a/litellm/router_utils/handle_error.py +++ b/litellm/router_utils/handle_error.py @@ -3,6 +3,7 @@ from typing import TYPE_CHECKING, Any, Final from litellm._logging import redact_secrets, verbose_router_logger from litellm.constants import MAX_EXCEPTION_MESSAGE_LENGTH +from litellm.exceptions import MissingMessageRoleError from litellm.router_deprecated import renamed_router_param from litellm.router_utils.cooldown_handlers import ( async_get_cooldown_deployments_with_debug_info, @@ -46,6 +47,9 @@ async def send_llm_exception_alert( if slack_alerting_logger is None: return + if isinstance(original_exception, MissingMessageRoleError): + return + if "proxy_server_request" in request_kwargs: # Do not send any alert if it's a request from litellm proxy server request # the proxy is already instrumented to send LLM API call failures diff --git a/litellm/utils.py b/litellm/utils.py index 7c73d1cc4d5..7fa9d32ae64 100644 --- a/litellm/utils.py +++ b/litellm/utils.py @@ -534,6 +534,7 @@ from .exceptions import ( ContentPolicyViolationError, ContextWindowExceededError, MalformedRequestFieldError, + MissingMessageRoleError, ModelNotMappedError, NotFoundError, OpenAIError, @@ -9496,16 +9497,22 @@ def convert_list_message_to_dict( return [dict(cleanup_none_field_in_message(message=_as_message_value(message))) for message in messages] -def validate_and_fix_openai_messages(messages: list): +_ASSISTANT_ONLY_MESSAGE_KEYS: Final = frozenset( + {"tool_calls", "function_call", "reasoning_content", "reasoning_items", "thinking_blocks", "refusal", "audio"} +) + + +def validate_and_fix_openai_messages( + messages: list, model: str = "", request_kwargs: Mapping[str, object] | None = None +): """ Ensures all messages are valid OpenAI chat completion messages. - Handles missing role for assistant messages. + Only an assistant turn carries the keys in _ASSISTANT_ONLY_MESSAGE_KEYS, so one sent without a role but with + any of them, even set to null, gets "assistant" (issue #8661). """ new_messages: Final = [] for message in messages: - if not message.get("role"): - message["role"] = "assistant" if message.get("tool_calls"): message["tool_calls"] = jsonify_tools(tools=message["tool_calls"]) @@ -9516,8 +9523,10 @@ def validate_and_fix_openai_messages(messages: list): ), ) cleaned_message = cleanup_none_field_in_message(message=convert_msg_to_dict) + if not cleaned_message.get("role") and not _ASSISTANT_ONLY_MESSAGE_KEYS.isdisjoint(convert_msg_to_dict): + cleaned_message["role"] = "assistant" new_messages.append(cleaned_message) - return validate_chat_completion_user_messages(messages=new_messages) + return validate_chat_completion_user_messages(messages=new_messages, model=model, request_kwargs=request_kwargs) def validate_and_fix_openai_tools(tools: list | None) -> list[dict] | None: @@ -9570,21 +9579,36 @@ def cleanup_none_field_in_message(message: AllMessageValues): return {k: v for k, v in new_message.items() if v is not None} -def validate_chat_completion_user_messages(messages: list[AllMessageValues]): +def validate_chat_completion_user_messages( + messages: list[AllMessageValues], model: str = "", request_kwargs: Mapping[str, object] | None = None +): """ - Ensures all user messages are valid OpenAI chat completion messages. + Ensures every message has a role and all user messages are valid OpenAI chat completion messages. Args: messages: List of message dictionaries + model: Model named on the error raised for a message without a role + request_kwargs: The request's provider settings, read to name the provider on that error message_content_type: Type to validate content against Returns: List[dict]: The validated messages Raises: + MissingMessageRoleError: If a message has no role ValueError: If any message is invalid """ for idx, m in enumerate(messages): + if "role" not in m or not m["role"]: + raise MissingMessageRoleError( + message=f"Missing required parameter: 'messages[{idx}].role'.", + model=model, + llm_provider=_deployment_provider({**(request_kwargs or {}), "model": model}), + body={ + "type": "invalid_request_error", + "param": f"messages[{idx}].role", + }, + ) try: if m["role"] == "user": user_content = m.get("content") @@ -9597,10 +9621,6 @@ def validate_chat_completion_user_messages(messages: list[AllMessageValues]): if item.get("type") not in ValidUserMessageContentTypes: raise Exception(f"invalid content type={item.get('type')}") except Exception as e: - if isinstance(e, KeyError): - raise Exception( - f"Invalid message at index {idx}. Please ensure all messages are valid OpenAI chat completion messages." - ) if "invalid content type" in str(e): raise Exception( f"Invalid user message at index {idx}. Please ensure all user messages are valid OpenAI chat completion messages." diff --git a/tests/unit/enterprise/enterprise_callbacks/test_pagerduty_alerting.py b/tests/unit/enterprise/enterprise_callbacks/test_pagerduty_alerting.py index c26966eeb38..84b4dc0a93e 100644 --- a/tests/unit/enterprise/enterprise_callbacks/test_pagerduty_alerting.py +++ b/tests/unit/enterprise/enterprise_callbacks/test_pagerduty_alerting.py @@ -74,6 +74,51 @@ async def test_failure_alerts_follow_the_configured_threshold( assert recent_errors[0]["error_llm_provider"] == "openai" +@pytest.mark.asyncio +@pytest.mark.parametrize( + ("exception", "request_count"), + [ + ( + litellm.MissingMessageRoleError( + message="Missing required parameter: 'messages[0].role'.", model="gpt-5.6", llm_provider="openai" + ), + 0, + ), + ( + litellm.MalformedRequestFieldError( + message="tools: Input should be a valid list", model="gpt-5.6", llm_provider="openai" + ), + 1, + ), + ], +) +async def test_only_a_role_less_request_stays_out_of_the_failure_count( + monkeypatch: pytest.MonkeyPatch, + respx_mock: respx.MockRouter, + exception: Exception, + request_count: int, +) -> None: + route: Final = respx_mock.post(PAGERDUTY_URL).mock(return_value=httpx.Response(202)) + logger: Final = _pagerduty_logger(monkeypatch) + payload: Final = cast( + StandardLoggingPayload, + { + "error_information": { + "error_class": type(exception).__name__, + "error_code": "400", + "llm_provider": "openai", + }, + "metadata": {}, + }, + ) + + await logger.async_log_failure_event( + {"standard_logging_object": payload, "exception": exception}, None, None, None + ) + + assert route.call_count == request_count + + @pytest.mark.asyncio async def test_hanging_requests_alert_at_the_default_threshold( monkeypatch: pytest.MonkeyPatch, respx_mock: respx.MockRouter diff --git a/tests/unit/proxy/test_proxy_utils_model_creation_and_error_logging.py b/tests/unit/proxy/test_proxy_utils_model_creation_and_error_logging.py index 867259a9b3b..5c399f96e83 100644 --- a/tests/unit/proxy/test_proxy_utils_model_creation_and_error_logging.py +++ b/tests/unit/proxy/test_proxy_utils_model_creation_and_error_logging.py @@ -2007,6 +2007,19 @@ class TestPostCallFailureHookLLMExceptionAlerting: async def test_http_exception_does_not_alert(self): assert (await self._alerted(HTTPException(status_code=400, detail="blocked"))).called is False + @pytest.mark.asyncio + async def test_role_less_message_does_not_alert(self): + with pytest.raises(litellm.MalformedRequestFieldError) as exc_info: + litellm.completion(model="openai/gpt-5.6", messages=[{"content": "hi"}], mock_response="ok") + assert (await self._alerted(exc_info.value)).called is False + + @pytest.mark.asyncio + async def test_malformed_tools_field_still_alerts(self): + exc = litellm.MalformedRequestFieldError( + message="tools: Input should be a valid list", model="openai/gpt-5.6", llm_provider="openai" + ) + assert (await self._alerted(exc)).called is True + @pytest.mark.asyncio async def test_genuine_llm_api_error_still_alerts(self): assert (await self._alerted(Exception("upstream 503"))).called is True diff --git a/tests/unit/router_utils/test_cooldown_handlers.py b/tests/unit/router_utils/test_cooldown_handlers.py index 3a61ca48f26..cc3165a036c 100644 --- a/tests/unit/router_utils/test_cooldown_handlers.py +++ b/tests/unit/router_utils/test_cooldown_handlers.py @@ -3,6 +3,7 @@ import copy import datetime import importlib import itertools +import json import random import time import uuid @@ -37,6 +38,7 @@ from litellm.router_utils.cooldown_handlers import ( async_get_cooldown_deployments, cast_exception_status_to_int, get_cooldown_deployments, + is_failure_outside_selected_deployment, is_response_lookup_not_found, mark_advisor_orchestration_failure, set_cooldown_deployments, @@ -1939,6 +1941,48 @@ async def test_should_cooldown_deployment(testing_litellm_router): ) +@pytest.mark.usefixtures("_vcr_outcome_gate", "setup_and_teardown") +@pytest.mark.asyncio +async def test_role_less_requests_do_not_count_toward_the_deployments_failure_rate(): + """A caller's role-less message is not the deployment failing, so it must not push a healthy one into cooldown.""" + litellm_params: Final = {"model": "house-model", "custom_llm_provider": "openai", "mock_response": "ok"} + router: Final = Router( + model_list=[ + {"model_name": "house", "litellm_params": litellm_params, "model_info": {"id": "d1"}}, + {"model_name": "house", "litellm_params": litellm_params, "model_info": {"id": "d2"}}, + ] + ) + for _ in range(6): + with pytest.raises(litellm.MalformedRequestFieldError): + await router.acompletion(model="d1", messages=json.loads('[{"content": "hi"}]')) + await asyncio.sleep(1) + + assert _should_cooldown_deployment(router, "d1", 500, Exception("Test")) is False + + +@pytest.mark.parametrize( + ("exception", "outside_deployment"), + [ + ( + litellm.MissingMessageRoleError( + message="Missing required parameter: 'messages[0].role'.", model="m", llm_provider="openai" + ), + True, + ), + ( + litellm.MalformedRequestFieldError( + message="tools: Input should be a valid list", model="m", llm_provider="openai" + ), + False, + ), + ], +) +def test_only_a_role_less_request_is_a_failure_outside_the_deployment( + exception: Exception, outside_deployment: bool +) -> None: + assert is_failure_outside_selected_deployment(exception) is outside_deployment + + @pytest.mark.usefixtures("_vcr_outcome_gate", "setup_and_teardown") @pytest.mark.asyncio async def test_should_cooldown_deployment_allowed_fails_set_on_router(): diff --git a/tests/unit/router_utils/test_fallback_event_handlers.py b/tests/unit/router_utils/test_fallback_event_handlers.py index 9c034b77fe7..47d58a3b869 100644 --- a/tests/unit/router_utils/test_fallback_event_handlers.py +++ b/tests/unit/router_utils/test_fallback_event_handlers.py @@ -2503,6 +2503,25 @@ def test_is_bad_request_without_fallback_only_stops_on_a_real_400_when_the_flag_ assert is_bad_request_without_fallback(exception, fallback_on_unsupported_params=True) is False +@pytest.mark.asyncio +async def test_a_role_less_request_is_not_retried_on_the_fallback_group() -> None: + router: Final = Router( + model_list=[ + { + "model_name": "house", + "litellm_params": {"model": "house-model", "custom_llm_provider": "openai", "mock_response": "ok"}, + }, + {"model_name": "backup", "litellm_params": {"model": "openai/gpt-5.6", "mock_response": "ok"}}, + ], + fallbacks=[{"house": ["backup"]}], + fallback_on_unsupported_params=False, + ) + + with pytest.raises(litellm.MalformedRequestFieldError, match="Missing required parameter") as exc_info: + await router.acompletion(model="house", messages=json.loads('[{"content": "hi"}]')) + assert "Fallback to backup" not in str(exc_info.value) + + @pytest.mark.asyncio async def test_run_async_fallback_skips_a_target_the_rate_limit_check_rejects() -> None: checked: Final = [] # mutable-ok: records the targets the check saw diff --git a/tests/unit/router_utils/test_handle_error.py b/tests/unit/router_utils/test_handle_error.py index 22bd6e74190..00350cc357c 100644 --- a/tests/unit/router_utils/test_handle_error.py +++ b/tests/unit/router_utils/test_handle_error.py @@ -1,6 +1,6 @@ import asyncio import importlib -from typing import List +from typing import Final, List from unittest.mock import AsyncMock, MagicMock import pytest @@ -40,6 +40,37 @@ async def test_send_llm_exception_alert_success(): slack_alerting_logger.send_alert.assert_called_once() +@pytest.mark.asyncio +@pytest.mark.parametrize( + ("exception", "alerts_sent"), + [ + ( + litellm.MissingMessageRoleError( + message="Missing required parameter: 'messages[0].role'.", model="gpt-5.6", llm_provider="openai" + ), + 0, + ), + ( + litellm.MalformedRequestFieldError( + message="tools: Input should be a valid list", model="gpt-5.6", llm_provider="openai" + ), + 1, + ), + ], +) +async def test_send_llm_exception_alert_skips_only_a_role_less_request(exception: Exception, alerts_sent: int): + slack_alerting_logger: Final = AsyncMock() + ctx: Final = MagicMock() + ctx.observers.return_value = RoutingObservers(slack_alerting_logger=slack_alerting_logger) + request_kwargs: Final = {"model": "gpt-5.6", "messages": [{"content": "Hello"}]} + + from litellm.router_utils.handle_error import send_llm_exception_alert + + await send_llm_exception_alert(ctx, request_kwargs, "Traceback", exception) + + assert slack_alerting_logger.send_alert.call_count == alerts_sent + + @pytest.mark.asyncio async def test_send_llm_exception_alert_no_logger(): """ diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index 6fece4d8c66..a1400f53ef8 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -57,6 +57,8 @@ from litellm.litellm_core_utils.thread_pool_executor import executor as logging_ from litellm.llms.base_llm.base_model_iterator import MockResponseIterator from litellm.llms.custom_httpx.http_handler import AsyncHTTPHandler, headers from litellm.llms.openai_like.json_loader import JSONProviderRegistry +from litellm.litellm_core_utils.litellm_logging import StandardLoggingPayloadSetup +from litellm.proxy.db.request_error_tracking import status_code_from_metadata from litellm.proxy.utils import is_valid_api_key from litellm.responses.main import aresponses, responses from litellm.types.caching import CachingSupportedCallTypes @@ -119,6 +121,7 @@ from litellm.utils import ( trim_messages, validate_environment, validate_chat_completion_tool_choice, + validate_and_fix_openai_messages, ) from litellm.litellm_core_utils.model_cost_table import publish @@ -8939,6 +8942,57 @@ def test_validate_tool_choice_without_model_is_still_a_400(): assert exc_info.value.model == "" +@pytest.mark.usefixtures("_vcr_outcome_gate", "setup_and_teardown") +@pytest.mark.parametrize( + ("messages", "index"), + [ + ([{"content": "Say hi in two words."}], 0), + ([{"role": "user", "content": "hi"}, {"role": None, "content": "hello"}], 1), + ([{"role": "user", "content": "hi"}, {"role": "", "content": "hello"}], 1), + ], +) +def test_completion_message_without_role_is_a_400(messages: list[dict[str, str | None]], index: int): + """A message with no role is the caller's mistake, rejected before any provider sees it, never sent as assistant.""" + with pytest.raises( + litellm.MissingMessageRoleError, match=re.escape(f"Missing required parameter: 'messages[{index}].role'.") + ) as exc_info: + litellm.completion(model=MODEL, messages=messages, mock_response="Hello there!") + assert exc_info.value.status_code == 400 + assert exc_info.value.model == MODEL + assert exc_info.value.llm_provider == "anthropic" + assert exc_info.value.param == f"messages[{index}].role" + error_information: Final = StandardLoggingPayloadSetup.get_error_information(exc_info.value) + assert status_code_from_metadata({"error_information": error_information}) == 400 + + +@pytest.mark.usefixtures("_vcr_outcome_gate", "setup_and_teardown") +@pytest.mark.parametrize( + "call_turn", + [ + { + "content": "", + "tool_calls": [ + {"id": "call_1", "type": "function", "function": {"name": "get_weather", "arguments": "{}"}} + ], + }, + {"content": None, "function_call": {"name": "get_weather", "arguments": "{}"}}, + {"content": "ok", "tool_calls": []}, + {"content": "ok", "tool_calls": None}, + {"content": "ok", "function_call": None}, + {"content": "Sunny.", "reasoning_content": "The user wants the weather."}, + {"content": "Sunny.", "reasoning_items": []}, + {"content": "Sunny.", "thinking_blocks": [{"type": "thinking", "thinking": "Weather.", "signature": "sig"}]}, + {"content": None, "refusal": "I can't help with that."}, + {"content": None, "audio": {"id": "audio_1"}}, + ], +) +def test_completion_role_less_assistant_turn_is_sent_as_assistant(call_turn: dict[str, object]): + """Only an assistant turn carries these keys, so one replayed without a role still goes upstream as assistant.""" + messages: Final = [{"role": "user", "content": "Weather in Paris?"}, call_turn] + + assert validate_and_fix_openai_messages(messages)[1]["role"] == "assistant" + + @pytest.fixture() def _vcr_outcome_gate_local_testing(request, vcr): install_live_call_probe(request, vcr) @@ -10731,12 +10785,12 @@ def test_dict_to_response_format_helper(): def test_validate_user_messages_invalid_content_type(): from litellm.utils import validate_chat_completion_user_messages - messages = [{"content": [{"type": "invalid_type", "text": "Hello"}]}] + messages = [{"role": "user", "content": [{"type": "invalid_type", "text": "Hello"}]}] - with pytest.raises(Exception, match="Please ensure all messages are valid OpenAI chat completion") as e: + with pytest.raises(Exception, match="Please ensure all user messages are valid OpenAI chat completion") as e: validate_chat_completion_user_messages(messages) - assert "Invalid message" in str(e) + assert "Invalid user message at index 0" in str(e) print(e)