From 109de6f0e315987e43e572f800cebdc583a0d5b3 Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Sat, 5 Sep 2026 22:41:20 -0700 Subject: [PATCH] fix(guardrails): retry a broken dependency and warn once per unscanned route A ModuleNotFoundError from a dependency the install actually has is a broken install, not a lean one, so it now lands in the unavailable set and is retried instead of dropping its handlers for the life of the process. The unscanned warning is cached on the guardrail, route, call type and reason, since most proxy routes have no translation handler and never will, and both remaining skip sites now resolve the call type instead of raising on a route string the enum never had. --- litellm/llms/__init__.py | 21 +++++-- .../unified_guardrail/unified_guardrail.py | 62 ++++++++++++++----- .../test_guardrail_translation_discovery.py | 22 ++++++- .../test_unified_guardrail.py | 44 ++++++++++++- 4 files changed, 124 insertions(+), 25 deletions(-) diff --git a/litellm/llms/__init__.py b/litellm/llms/__init__.py index 90a07222d35..a4a43630ba0 100644 --- a/litellm/llms/__init__.py +++ b/litellm/llms/__init__.py @@ -1,4 +1,5 @@ import importlib +import importlib.util import os from collections.abc import Iterable, Iterator, Mapping from dataclasses import dataclass @@ -115,14 +116,23 @@ class _UnavailablePackage: """ Why one guardrail_translation package could not be imported. - missing_dependency is set when the package asked for a module outside litellm that this install does not - have, which is the one failure that says the package is absent rather than momentarily unimportable. + missing_dependency is set when the package asked for a module this install does not have at all, which is + the one failure that says the package is absent rather than momentarily unimportable. """ reason: str missing_dependency: bool +def _is_absent(module_name: str) -> bool: + """Whether this install has no module of that name, as opposed to one that is present but failed to import.""" + root: Final = module_name.partition(".")[0] + try: + return importlib.util.find_spec(root) is None + except (ImportError, ValueError): + return False + + def _import_guardrail_translations( module_path: str, ) -> Mapping[CallTypes, type["BaseTranslation"]] | _UnavailablePackage: @@ -132,7 +142,7 @@ def _import_guardrail_translations( except Exception as e: # noqa: BLE001 # a package failing at import time for any reason is unavailable, not fatal return _UnavailablePackage( reason=f"{type(e).__name__}: {e}", - missing_dependency=isinstance(e, ModuleNotFoundError) and not (e.name or "").startswith("litellm"), + missing_dependency=isinstance(e, ModuleNotFoundError) and _is_absent(e.name or ""), ) mappings: Final = getattr(module, "guardrail_translation_mappings", None) if not isinstance(mappings, dict): @@ -148,8 +158,9 @@ def _guardrail_translations_from( Import one package's handlers, tolerating an install that does not ship the optional MCP server. litellm ships every package under llms, so a failure there is a gap to retry rather than a fact about the - install. The MCP package instead arrives with the proxy extra, and a dependency it cannot import means this - install serves no MCP endpoints for a guardrail to scan, so there is nothing to retry or report. + install. The MCP package instead arrives with the proxy extra, and a dependency this install does not have + at all means it serves no MCP endpoints for a guardrail to scan, so there is nothing to retry or report. A + dependency that is installed and still fails to import is a broken install, which is reported and retried. """ result: Final = _import_guardrail_translations(module_path) if ( diff --git a/litellm/proxy/guardrails/guardrail_hooks/unified_guardrail/unified_guardrail.py b/litellm/proxy/guardrails/guardrail_hooks/unified_guardrail/unified_guardrail.py index 07573158950..64d5af0f772 100644 --- a/litellm/proxy/guardrails/guardrail_hooks/unified_guardrail/unified_guardrail.py +++ b/litellm/proxy/guardrails/guardrail_hooks/unified_guardrail/unified_guardrail.py @@ -9,6 +9,7 @@ Unified Guardrail, leveraging LiteLLM's /applyGuardrail endpoint import copy import json from collections.abc import AsyncGenerator, AsyncIterable, Awaitable, Callable, Mapping, Sequence +from functools import lru_cache from typing import TYPE_CHECKING, Any, Final, Protocol from fastapi import HTTPException @@ -133,6 +134,9 @@ def _ensure_litellm_metadata(data: dict, user_api_key_dict: UserAPIKeyAuth) -> N data["litellm_metadata"] = user_metadata +_UNSCANNED_WARNING_KEYS: Final = 256 + + def _resolved_call_type(call_type: str | None) -> CallTypes | None: """Return the CallTypes member a route's call type names, or None when the enum has no member for it.""" try: @@ -141,27 +145,51 @@ def _resolved_call_type(call_type: str | None) -> CallTypes | None: return None +@lru_cache(maxsize=_UNSCANNED_WARNING_KEYS) +def _warn_left_unscanned_once( + guardrail_name: str | None, + request_route: str | None, + call_type: str | None, + consequence: str, +) -> None: + if _resolved_call_type(call_type) is not None: + verbose_proxy_logger.warning( + "Guardrail '%s' selected for route '%s' but call type '%s' has no guardrail translation handler; %s.", + guardrail_name, + request_route, + call_type, + consequence, + ) + return + unscannable: Final = ( + f"call type '{call_type}' is not one litellm can scan" if call_type else "its call type could not be resolved" + ) + verbose_proxy_logger.warning( + "Guardrail '%s' selected for route '%s' but %s, so no guardrail can run on that route; %s.", + guardrail_name, + request_route, + unscannable, + consequence, + ) + + def _warn_left_unscanned( guardrail_to_apply: "CustomGuardrail", user_api_key_dict: UserAPIKeyAuth, call_type: str | None, consequence: str, ) -> None: - if call_type is None: - verbose_proxy_logger.warning( - "Guardrail '%s' selected for route '%s' but its call type could not be resolved, so no guardrail " - "can run on that route; %s.", - guardrail_to_apply.guardrail_name, - user_api_key_dict.request_route, - consequence, - ) - return - verbose_proxy_logger.warning( - "Guardrail '%s' selected for route '%s' but call type '%s' has no guardrail translation handler; %s.", - guardrail_to_apply.guardrail_name, - user_api_key_dict.request_route, - call_type, - consequence, + """ + Say that a selected guardrail could not scan this request, once per route and reason. + + Most proxy routes have no translation handler and never will, so a line per request would bury the + outage it is meant to surface, and repeating it adds nothing an operator can act on twice. + """ + _warn_left_unscanned_once( + guardrail_name=guardrail_to_apply.guardrail_name, + request_route=user_api_key_dict.request_route, + call_type=call_type, + consequence=consequence, ) @@ -340,7 +368,7 @@ class UnifiedLLMGuardrails(CustomLogger): call_type = logging_call_type mappings: Final = load_guardrail_translation_mappings() - if call_type is None or CallTypes(call_type) not in mappings: + if _resolved_call_type(call_type) not in mappings: _warn_left_unscanned( guardrail_to_apply=guardrail_to_apply, user_api_key_dict=user_api_key_dict, @@ -1048,7 +1076,7 @@ class UnifiedLLMGuardrails(CustomLogger): call_type = _infer_call_type(call_type=None, completion_response=item) # If call type not supported, just pass through all chunks - if call_type is None or CallTypes(call_type) not in mappings: + if _resolved_call_type(call_type) not in mappings: _warn_left_unscanned( guardrail_to_apply=guardrail_to_apply, user_api_key_dict=user_api_key_dict, diff --git a/tests/test_litellm/llms/test_guardrail_translation_discovery.py b/tests/test_litellm/llms/test_guardrail_translation_discovery.py index 0507a645ab3..3b4446e4da5 100644 --- a/tests/test_litellm/llms/test_guardrail_translation_discovery.py +++ b/tests/test_litellm/llms/test_guardrail_translation_discovery.py @@ -155,7 +155,7 @@ def test_an_mcp_package_that_fails_to_import_is_reported_and_retried(): def test_an_install_without_the_mcp_server_is_discovered_once_and_quietly(caplog): absent = ModuleNotFoundError("No module named 'mcp'", name="mcp") - with capturing(caplog, logging.DEBUG), raising_on_import(MCP_TRANSLATION_MODULE, absent): + with capturing(caplog, logging.DEBUG), unimportable("mcp"), raising_on_import(MCP_TRANSLATION_MODULE, absent): first = llms_package.load_guardrail_translation_mappings() second = llms_package.load_guardrail_translation_mappings() @@ -164,3 +164,23 @@ def test_an_install_without_the_mcp_server_is_discovered_once_and_quietly(caplog assert CallTypes.acompletion in first assert not llms_package.guardrail_translation_discovery.unavailable assert not [record for record in caplog.records if record.levelno >= logging.WARNING] + + +def test_a_broken_mcp_dependency_is_reported_and_retried(caplog): + """An mcp the install has but cannot import is a broken install, not a lean one, so it must be loud.""" + broken = ModuleNotFoundError("No module named 'mcp.types'", name="mcp.types") + + with capturing(caplog, logging.DEBUG), raising_on_import(MCP_TRANSLATION_MODULE, broken): + partial = llms_package.load_guardrail_translation_mappings() + + assert CallTypes.call_mcp_tool not in partial + assert CallTypes.acompletion in partial + assert tuple(llms_package.guardrail_translation_discovery.unavailable) == (MCP_TRANSLATION_MODULE,) + errors = [record for record in caplog.records if record.levelno >= logging.ERROR] + assert len(errors) == 1, [record.getMessage() for record in caplog.records] + assert "mcp.types" in errors[0].getMessage() + + recovered = llms_package.load_guardrail_translation_mappings() + + assert CallTypes.call_mcp_tool in recovered + assert not llms_package.guardrail_translation_discovery.unavailable diff --git a/tests/test_litellm/proxy/guardrails/guardrail_hooks/unified_guardrails/test_unified_guardrail.py b/tests/test_litellm/proxy/guardrails/guardrail_hooks/unified_guardrails/test_unified_guardrail.py index 9f4a0d8683f..65fab6e1270 100644 --- a/tests/test_litellm/proxy/guardrails/guardrail_hooks/unified_guardrails/test_unified_guardrail.py +++ b/tests/test_litellm/proxy/guardrails/guardrail_hooks/unified_guardrails/test_unified_guardrail.py @@ -85,6 +85,14 @@ def _patch_translation_mappings(monkeypatch, mappings): monkeypatch.setattr(unified_module, "load_guardrail_translation_mappings", lambda: mappings) +@pytest.fixture(autouse=True) +def _forget_unscanned_warnings(): + """The unscanned warning fires once per route and reason, so each test starts with nothing remembered.""" + unified_module._warn_left_unscanned_once.cache_clear() + yield + unified_module._warn_left_unscanned_once.cache_clear() + + @pytest.fixture(autouse=True) def _inject_mcp_handler_mapping(monkeypatch): """Inject MCP handler mapping so the unified guardrail can run inside tests.""" @@ -2396,6 +2404,12 @@ class TestUnscannedRequestIsAnnounced: def _warnings(caplog): return [record.getMessage() for record in caplog.records if record.levelno >= logging.WARNING] + @staticmethod + def _distinct_warnings(caplog): + """caplog holds every record twice here, once through the handler above and once through propagation.""" + by_record = {id(record): record for record in caplog.records if record.levelno >= logging.WARNING} + return [record.getMessage() for record in by_record.values()] + @pytest.mark.asyncio async def test_pre_call_warns_when_the_call_type_has_no_translation_handler(self, caplog, monkeypatch): _patch_translation_mappings(monkeypatch, {CallTypes.aembedding: _NoopTranslation}) @@ -2458,7 +2472,7 @@ class TestUnscannedRequestIsAnnounced: assert guardrail.apply_calls == [] assert returned["messages"] == [{"role": "user", "content": "hello world"}] assert any( - "call type 'not_a_call_type' has no guardrail translation handler" in message + "call type 'not_a_call_type' is not one litellm can scan" in message and "skipping pre-call scanning" in message for message in self._warnings(caplog) ), self._warnings(caplog) @@ -2479,11 +2493,37 @@ class TestUnscannedRequestIsAnnounced: assert guardrail.apply_calls == [] assert returned["messages"] == [{"role": "user", "content": "hello world"}] assert any( - "call type 'not_a_call_type' has no guardrail translation handler" in message + "call type 'not_a_call_type' is not one litellm can scan" in message and "skipping during-call scanning" in message for message in self._warnings(caplog) ), self._warnings(caplog) + @pytest.mark.asyncio + async def test_a_route_with_no_handler_warns_once_instead_of_once_per_request(self, caplog, monkeypatch): + _patch_translation_mappings(monkeypatch, {CallTypes.aembedding: _NoopTranslation}) + guardrail = RecordingGuardrail() + + with self._capturing(caplog): + for _ in range(5): + await UnifiedLLMGuardrails().async_moderation_hook( + data=self._request(guardrail), + user_api_key_dict=UserAPIKeyAuth(api_key="test-key", request_route="/v1/responses/resp_123"), + call_type="aget_responses", + ) + await UnifiedLLMGuardrails().async_moderation_hook( + data=self._request(guardrail), + user_api_key_dict=UserAPIKeyAuth(api_key="test-key", request_route="/v1/moderations"), + call_type="aget_responses", + ) + + unscanned = [ + message for message in self._distinct_warnings(caplog) if "skipping during-call scanning" in message + ] + assert len(unscanned) == 2, unscanned + assert "/v1/responses/resp_123" in unscanned[0] + assert "aget_responses" in unscanned[0] + assert "/v1/moderations" in unscanned[1] + @pytest.mark.asyncio async def test_post_call_names_the_call_type_the_route_maps_to(self, caplog, monkeypatch): _patch_translation_mappings(monkeypatch, {CallTypes.aembedding: _NoopTranslation})