diff --git a/litellm/llms/azure_ai/chat/transformation.py b/litellm/llms/azure_ai/chat/transformation.py index 707ddc9e12b..d1854276e90 100644 --- a/litellm/llms/azure_ai/chat/transformation.py +++ b/litellm/llms/azure_ai/chat/transformation.py @@ -269,7 +269,9 @@ class AzureAIStudioConfig(OpenAIConfig): return super().should_retry_llm_api_inside_llm_translation_on_http_error(e=e, litellm_params=litellm_params) def _error_has_tool_level_extra_fields(self, error_text: str) -> bool: - return bool(re.search(r"tools\[\d+\]\.", error_text)) + from litellm.llms.base_llm.base_utils import parse_rejected_tool_fields + + return bool(parse_rejected_tool_fields(error_text)) @property def max_retry_on_unprocessable_entity_error(self) -> int: @@ -290,7 +292,9 @@ class AzureAIStudioConfig(OpenAIConfig): return data def _drop_tool_level_extra_fields(self, request_data: dict, error_text: str) -> dict: - fields_to_drop = set(re.findall(r"tools\[\d+\]\.([\w-]+)", error_text)) + from litellm.llms.base_llm.base_utils import parse_rejected_tool_fields + + fields_to_drop = frozenset().union(*parse_rejected_tool_fields(error_text).values() or (frozenset(),)) tools = request_data.get("tools") if fields_to_drop and isinstance(tools, list): for tool in tools: diff --git a/litellm/llms/base_llm/base_utils.py b/litellm/llms/base_llm/base_utils.py index 789990e6c78..c199602a4d2 100644 --- a/litellm/llms/base_llm/base_utils.py +++ b/litellm/llms/base_llm/base_utils.py @@ -4,7 +4,10 @@ Utility functions for base LLM classes. import copy import json +import re from abc import ABC, abstractmethod +from collections.abc import Mapping +from types import MappingProxyType from typing import Any from openai.lib import _parsing, _pydantic @@ -103,6 +106,41 @@ class BaseLLMModelInfo(ABC): return None +_EXTRA_INPUTS_NOT_PERMITTED = "Extra inputs are not permitted" +_NO_REJECTED_TOOL_FIELDS: Mapping[int, frozenset[str]] = MappingProxyType({}) +_REJECTED_TOOL_FIELD_RE = re.compile( + r"tools(?:\[(?P\d+)\]|\.(?P\d+)\.[A-Za-z_][\w-]*)\.(?P[A-Za-z_][\w-]*)" +) + + +def parse_rejected_tool_fields(error_text: str) -> Mapping[int, frozenset[str]]: + """ + Parse a provider's "extra inputs" rejection into ``{tool index: rejected field names}``. + + Several providers validate tool definitions against a narrower schema than the API + documents and reject the surplus members by presence, naming each one in the error. + Two spellings are in circulation and both appear here: + + - ``tools[0].strict`` (Azure AI Foundry) + - ``tools.0.custom.strict`` (Bedrock Converse, where the middle segment is the + Anthropic tool union member rather than a key in the request body) + + Returns an empty mapping when the message is not one of these rejections, which + callers treat as "not retryable". A field named here is by definition one the + provider does not accept, so removing it can never strip something required. + """ + if _EXTRA_INPUTS_NOT_PERMITTED not in error_text: + return _NO_REJECTED_TOOL_FIELDS + + rejected = tuple( + (int(match.group("bracket_index") or match.group("dot_index")), match.group("field")) + for match in _REJECTED_TOOL_FIELD_RE.finditer(error_text) + ) + return MappingProxyType( + {index: frozenset(field for other, field in rejected if other == index) for index, _ in rejected} + ) + + def _convert_tool_response_to_message( tool_calls: list[ChatCompletionToolCallChunk], ) -> Message | None: diff --git a/litellm/llms/bedrock/chat/converse_handler.py b/litellm/llms/bedrock/chat/converse_handler.py index 2309965dbe5..cc00528097f 100644 --- a/litellm/llms/bedrock/chat/converse_handler.py +++ b/litellm/llms/bedrock/chat/converse_handler.py @@ -1,5 +1,6 @@ import json -from typing import Any +from collections.abc import Awaitable, Callable, Mapping +from typing import Any, TypeVar import httpx @@ -18,9 +19,28 @@ from litellm.types.utils import ModelResponse from litellm.utils import CustomStreamWrapper from ..base_aws_llm import BaseAWSLLM, Credentials -from ..common_utils import BedrockError, _get_all_bedrock_regions +from ..common_utils import ( + BedrockError, + _get_all_bedrock_regions, + drop_bedrock_rejected_tool_fields, +) from .invoke_handler import AWSEventStreamDecoder, MockResponseIterator, make_call +_SendResultT = TypeVar("_SendResultT") + + +def _provider_error_text(err: BedrockError | httpx.HTTPStatusError) -> str: + """ + Read the provider's error body off either shape Converse raises. + + The non-streaming paths convert to ``BedrockError`` themselves, while the streaming + paths surface the transport's ``httpx.HTTPStatusError`` (in practice a + ``MaskedHTTPStatusError``, which keeps the response body but redacts the URL). + """ + if isinstance(err, BedrockError): + return str(err.message) + return err.response.text + def make_sync_call( client: HTTPHandler | None, @@ -81,6 +101,110 @@ class BedrockConverseLLM(BaseAWSLLM): def __init__(self) -> None: super().__init__() + def _resign_without_rejected_tool_fields( + self, + *, + request_data: Mapping[str, Any], + error_text: str, + credentials: Credentials, + aws_region_name: str, + extra_headers: Mapping[str, str] | None, + endpoint_url: str, + headers: Mapping[str, str], + api_key: str | None, + ) -> tuple[str, Mapping[str, str]] | None: + """ + Build a re-signed payload with the ``toolSpec`` members Bedrock just rejected removed. + + SigV4 signs a hash of the body, so a retry that edits the body has to be signed + again; reusing the original headers would fail as ``SignatureDoesNotMatch`` rather + than succeed. Returns ``None`` when the error is not a rejection of extra tool + fields, which callers treat as "surface the original error". + """ + retry_data = drop_bedrock_rejected_tool_fields(request_data, error_text) + if retry_data is None: + return None + + data = json.dumps(retry_data) + prepped = self.get_request_headers( + credentials=credentials, + aws_region_name=aws_region_name, + extra_headers=extra_headers, + endpoint_url=endpoint_url, + data=data, + headers=headers, + api_key=api_key, + ) + return data, prepped.headers + + async def _asend_retrying_rejected_tool_fields( + self, + *, + send: Callable[[str, Mapping[str, str]], Awaitable[_SendResultT]], + request_data: Mapping[str, Any], + data: str, + headers: Mapping[str, str], + credentials: Credentials, + aws_region_name: str, + caller_headers: Mapping[str, str], + endpoint_url: str, + api_key: str | None, + ) -> _SendResultT: + """ + Send once, and if Bedrock rejects extra ``toolSpec`` members, drop them and send again. + + ``send`` owns the transport and the provider-error contract, so a request that + fails for any other reason raises exactly what it raised before. The retry is + single-shot: a second rejection surfaces rather than looping. + """ + try: + return await send(data, headers) + except (BedrockError, httpx.HTTPStatusError) as err: + retry = self._resign_without_rejected_tool_fields( + request_data=request_data, + error_text=_provider_error_text(err), + credentials=credentials, + aws_region_name=aws_region_name, + extra_headers=caller_headers, + endpoint_url=endpoint_url, + headers=caller_headers, + api_key=api_key, + ) + if retry is None: + raise + return await send(*retry) + + def _send_retrying_rejected_tool_fields( + self, + *, + send: Callable[[str, Mapping[str, str]], _SendResultT], + request_data: Mapping[str, Any], + data: str, + headers: Mapping[str, str], + credentials: Credentials, + aws_region_name: str, + caller_headers: Mapping[str, str], + endpoint_url: str, + api_key: str | None, + ) -> _SendResultT: + """Synchronous twin of ``_asend_retrying_rejected_tool_fields``.""" + try: + return send(data, headers) + except (BedrockError, httpx.HTTPStatusError) as err: + retry = self._resign_without_rejected_tool_fields( + request_data=request_data, + error_text=_provider_error_text(err), + credentials=credentials, + aws_region_name=aws_region_name, + extra_headers=caller_headers, + endpoint_url=endpoint_url, + headers=caller_headers, + api_key=api_key, + ) + if retry is None: + raise + return send(*retry) + async def async_streaming( self, model: str, @@ -132,17 +256,30 @@ class BedrockConverseLLM(BaseAWSLLM): }, ) - completion_stream = await make_call( - client=client, - api_base=api_base, - headers=dict(prepped.headers), + async def _send(body: str, request_headers: Mapping[str, str]): + return await make_call( + client=client, + api_base=api_base, + headers=request_headers, + data=body, + model=model, + messages=messages, + logging_obj=logging_obj, + fake_stream=fake_stream, + json_mode=json_mode, + stream_chunk_size=stream_chunk_size, + ) + + completion_stream = await self._asend_retrying_rejected_tool_fields( + send=_send, + request_data=request_data, data=data, - model=model, - messages=messages, - logging_obj=logging_obj, - fake_stream=fake_stream, - json_mode=json_mode, - stream_chunk_size=stream_chunk_size, + headers=prepped.headers, + credentials=credentials, + aws_region_name=litellm_params.get("aws_region_name") or "us-west-2", + caller_headers=headers, + endpoint_url=api_base, + api_key=api_key, ) streaming_response = CustomStreamWrapper( completion_stream=completion_stream, @@ -200,6 +337,7 @@ class BedrockConverseLLM(BaseAWSLLM): }, ) + caller_headers = headers headers = dict(prepped.headers) if client is None or not isinstance(client, AsyncHTTPHandler): _params = {} @@ -211,19 +349,32 @@ class BedrockConverseLLM(BaseAWSLLM): else: client = client # type: ignore - try: - response = await client.post( - url=api_base, - headers=headers, - data=data, - logging_obj=logging_obj, - ) # type: ignore - response.raise_for_status() - except httpx.HTTPStatusError as err: - error_code = err.response.status_code - raise BedrockError(status_code=error_code, message=err.response.text) - except httpx.TimeoutException: - raise BedrockError(status_code=408, message="Timeout error occurred.") + async def _send(body: str, request_headers: Mapping[str, str]) -> httpx.Response: + try: + sent = await client.post( # type: ignore[union-attr] + url=api_base, + headers=request_headers, + data=body, + logging_obj=logging_obj, + ) + sent.raise_for_status() + return sent + except httpx.HTTPStatusError as err: + raise BedrockError(status_code=err.response.status_code, message=err.response.text) + except httpx.TimeoutException: + raise BedrockError(status_code=408, message="Timeout error occurred.") + + response = await self._asend_retrying_rejected_tool_fields( + send=_send, + request_data=request_data, + data=data, + headers=headers, + credentials=credentials, + aws_region_name=litellm_params.get("aws_region_name") or "us-west-2", + caller_headers=caller_headers, + endpoint_url=api_base, + api_key=api_key, + ) return litellm.AmazonConverseConfig()._transform_response( model=model, @@ -440,17 +591,31 @@ class BedrockConverseLLM(BaseAWSLLM): client = client if stream is not None and stream is True: - completion_stream = make_sync_call( - client=(client if client is not None and isinstance(client, HTTPHandler) else None), - api_base=proxy_endpoint_url, - headers=prepped.headers, # type: ignore + + def _send_stream(body: str, request_headers: Mapping[str, str]): + return make_sync_call( + client=(client if client is not None and isinstance(client, HTTPHandler) else None), + api_base=proxy_endpoint_url, + headers=request_headers, + data=body, + model=model, + messages=messages, + logging_obj=logging_obj, + json_mode=json_mode, + fake_stream=fake_stream, + stream_chunk_size=stream_chunk_size, + ) + + completion_stream = self._send_retrying_rejected_tool_fields( + send=_send_stream, + request_data=_data, data=data, - model=model, - messages=messages, - logging_obj=logging_obj, - json_mode=json_mode, - fake_stream=fake_stream, - stream_chunk_size=stream_chunk_size, + headers=prepped.headers, + credentials=credentials, + aws_region_name=aws_region_name, + caller_headers=headers, + endpoint_url=proxy_endpoint_url, + api_key=api_key, ) streaming_response = CustomStreamWrapper( completion_stream=completion_stream, @@ -463,19 +628,32 @@ class BedrockConverseLLM(BaseAWSLLM): ### COMPLETION - try: - response = client.post( - url=proxy_endpoint_url, - headers=prepped.headers, - data=data, - logging_obj=logging_obj, - ) # type: ignore - response.raise_for_status() - except httpx.HTTPStatusError as err: - error_code = err.response.status_code - raise BedrockError(status_code=error_code, message=err.response.text) - except httpx.TimeoutException: - raise BedrockError(status_code=408, message="Timeout error occurred.") + def _send(body: str, request_headers: Mapping[str, str]) -> httpx.Response: + try: + sent = client.post( # type: ignore[union-attr] + url=proxy_endpoint_url, + headers=request_headers, + data=body, + logging_obj=logging_obj, + ) + sent.raise_for_status() + return sent + except httpx.HTTPStatusError as err: + raise BedrockError(status_code=err.response.status_code, message=err.response.text) + except httpx.TimeoutException: + raise BedrockError(status_code=408, message="Timeout error occurred.") + + response = self._send_retrying_rejected_tool_fields( + send=_send, + request_data=_data, + data=data, + headers=prepped.headers, + credentials=credentials, + aws_region_name=aws_region_name, + caller_headers=headers, + endpoint_url=proxy_endpoint_url, + api_key=api_key, + ) return litellm.AmazonConverseConfig()._transform_response( model=model, diff --git a/litellm/llms/bedrock/common_utils.py b/litellm/llms/bedrock/common_utils.py index 03bb6fe1fbf..db7a58dcd2e 100644 --- a/litellm/llms/bedrock/common_utils.py +++ b/litellm/llms/bedrock/common_utils.py @@ -164,6 +164,50 @@ def remove_custom_field_from_tools(request_body: dict) -> None: tool.pop("custom", None) +def drop_bedrock_rejected_tool_fields(request_data: Mapping[str, Any], error_text: str) -> dict[str, Any] | None: + """ + Rebuild ``request_data`` without the ``toolSpec`` members Bedrock just rejected. + + Bedrock Converse validates some Claude models through an Anthropic-compatible + validator that accepts a narrower ``toolSpec`` than the Converse API documents, and + rejects the extras by presence. ``parse_rejected_tool_fields`` owns reading which + fields those are; this function applies the verdict to the Converse request shape. + + Returns a copy with those fields removed, or ``None`` when the error is not a + rejection of extra tool fields or names nothing this request actually carries. + """ + from litellm.llms.base_llm.base_utils import parse_rejected_tool_fields + + rejected = parse_rejected_tool_fields(error_text) + if not rejected: + return None + + tool_config = request_data.get("toolConfig") + tools = tool_config.get("tools") if isinstance(tool_config, Mapping) else None + if not isinstance(tools, list): + return None + + rebuilt = tuple(_tool_spec_without(tool, rejected.get(index, frozenset())) for index, tool in enumerate(tools)) + if all(new is old for new, old in zip(rebuilt, tools)): + return None + + retried_config = {**tool_config, "tools": list(rebuilt)} # mutable-ok: json.dumps needs real dicts + return {**request_data, "toolConfig": retried_config} # mutable-ok: outbound Converse request body + + +def _tool_spec_without(tool: object, rejected_fields: frozenset[str]) -> object: + """Return ``tool`` minus the named ``toolSpec`` members, or ``tool`` itself if none apply.""" + if not rejected_fields or not isinstance(tool, dict): + return tool + tool_spec = tool.get("toolSpec") + if not isinstance(tool_spec, dict): + return tool + surviving = {k: v for k, v in tool_spec.items() if k not in rejected_fields} # mutable-ok: serialized body + if len(surviving) == len(tool_spec): + return tool + return {**tool, "toolSpec": surviving} # mutable-ok: outbound Converse request body + + def normalize_json_schema_custom_types_to_object(schema: dict) -> None: """ In-place: replace JSON Schema ``type: \"custom\"`` with ``\"object\"`` (iterative walk). diff --git a/tests/test_litellm/llms/bedrock/chat/test_converse_rejected_tool_field_retry.py b/tests/test_litellm/llms/bedrock/chat/test_converse_rejected_tool_field_retry.py new file mode 100644 index 00000000000..61ab0a6365f --- /dev/null +++ b/tests/test_litellm/llms/bedrock/chat/test_converse_rejected_tool_field_retry.py @@ -0,0 +1,201 @@ +"""Bedrock Converse retries once, re-signed, when the provider rejects extra toolSpec fields. + +Bedrock validates some Claude models through an Anthropic-compatible validator that +accepts a narrower ``toolSpec`` than the Converse API documents and rejects the surplus +members by presence. Retrying without those members only works if the retry is signed +again, because SigV4 commits to a hash of the body. See BerriAI/litellm#33193. +""" + +import httpx +import pytest + +from litellm.llms.base_llm.base_utils import parse_rejected_tool_fields +from litellm.llms.bedrock.chat.converse_handler import BedrockConverseLLM +from litellm.llms.bedrock.common_utils import BedrockError, drop_bedrock_rejected_tool_fields + +_STRICT_REJECTION = ( + '{"message":"The model returned the following errors: ' + 'tools.0.custom.strict: Extra inputs are not permitted"}' +) + +_REQUEST_DATA = { + "messages": [{"role": "user", "content": [{"text": "hi"}]}], + "toolConfig": { + "tools": [ + { + "toolSpec": { + "name": "get_weather", + "description": "Get the weather for a city", + "inputSchema": {"json": {"type": "object", "properties": {}}}, + "strict": False, + } + } + ] + }, +} + + +def _credentials(): + from botocore.credentials import Credentials + + return Credentials(access_key="AKIAEXAMPLE", secret_key="secret", token=None) + + +def _retry_kwargs(): + return { + "request_data": _REQUEST_DATA, + "data": "original-body", + "headers": {"Authorization": "signature-over-original"}, + "credentials": _credentials(), + "aws_region_name": "us-east-1", + "caller_headers": {"Content-Type": "application/json"}, + "endpoint_url": "https://bedrock-runtime.us-east-1.amazonaws.com/model/m/converse", + "api_key": None, + } + + +@pytest.mark.parametrize( + "error_text, expected", + [ + ("tools.0.custom.strict: Extra inputs are not permitted", {0: frozenset({"strict"})}), + ("tools[0].strict: Extra inputs are not permitted", {0: frozenset({"strict"})}), + ( + "tools.0.custom.strict: Extra inputs are not permitted, " + "tools.2.custom.defer_loading: Extra inputs are not permitted", + {0: frozenset({"strict"}), 2: frozenset({"defer_loading"})}, + ), + ("tools.0.custom.input_schema.type: Input should be 'object'", {}), + ("ThrottlingException: rate exceeded", {}), + ("", {}), + ], +) +def test_parse_rejected_tool_fields(error_text: str, expected: dict) -> None: + """Both provider spellings parse; anything that is not an extra-inputs rejection is ignored.""" + assert dict(parse_rejected_tool_fields(error_text)) == expected + + +def test_drop_bedrock_rejected_tool_fields_removes_only_the_named_field() -> None: + result = drop_bedrock_rejected_tool_fields(_REQUEST_DATA, _STRICT_REJECTION) + assert result is not None + tool_spec = result["toolConfig"]["tools"][0]["toolSpec"] + assert "strict" not in tool_spec + assert tool_spec["name"] == "get_weather" + assert tool_spec["inputSchema"] == {"json": {"type": "object", "properties": {}}} + + +def test_drop_bedrock_rejected_tool_fields_does_not_mutate_the_original() -> None: + """The caller still needs the original payload to raise its untouched error on failure.""" + drop_bedrock_rejected_tool_fields(_REQUEST_DATA, _STRICT_REJECTION) + assert _REQUEST_DATA["toolConfig"]["tools"][0]["toolSpec"]["strict"] is False + + +@pytest.mark.parametrize( + "error_text", + [ + "ThrottlingException: rate exceeded", + "tools.9.custom.strict: Extra inputs are not permitted", + "tools.0.custom.nonexistent_field: Extra inputs are not permitted", + ], +) +def test_drop_bedrock_rejected_tool_fields_returns_none_when_nothing_applies(error_text: str) -> None: + """Unrelated errors, out-of-range indices and fields the request never carried are all no-ops.""" + assert drop_bedrock_rejected_tool_fields(_REQUEST_DATA, error_text) is None + + +def _http_status_error(body: str) -> httpx.HTTPStatusError: + request = httpx.Request("POST", "https://bedrock-runtime.us-east-1.amazonaws.com/model/m/converse-stream") + return httpx.HTTPStatusError("400", request=request, response=httpx.Response(400, text=request and body)) + + +@pytest.mark.parametrize( + "raised", + [ + BedrockError(status_code=400, message=_STRICT_REJECTION), + _http_status_error(_STRICT_REJECTION), + ], + ids=["non-streaming raises BedrockError", "streaming raises HTTPStatusError"], +) +def test_sync_retry_resends_without_the_rejected_field_and_resigns(raised: Exception) -> None: + """Both error shapes Converse can raise trigger the retry, and the retry is signed afresh.""" + attempts: list[tuple[str, dict]] = [] + + def send(body: str, headers: dict) -> str: + attempts.append((body, headers)) + if len(attempts) == 1: + raise raised + return "ok" + + result = BedrockConverseLLM()._send_retrying_rejected_tool_fields(send=send, **_retry_kwargs()) + + assert result == "ok" + assert len(attempts) == 2 + + first_body, first_headers = attempts[0] + retry_body, retry_headers = attempts[1] + assert first_body == "original-body" + assert '"strict"' not in retry_body + assert '"get_weather"' in retry_body + assert retry_headers["Authorization"] != first_headers["Authorization"] + assert retry_headers["Authorization"].startswith("AWS4-HMAC-SHA256") + + +def test_sync_retry_leaves_unrelated_errors_alone() -> None: + """A failure that is not an extra-tool-field rejection is sent once and raises as-is.""" + attempts: list[str] = [] + + def send(body: str, headers: dict) -> str: + attempts.append(body) + raise BedrockError(status_code=429, message="ThrottlingException: rate exceeded") + + with pytest.raises(BedrockError) as excinfo: + BedrockConverseLLM()._send_retrying_rejected_tool_fields(send=send, **_retry_kwargs()) + + assert excinfo.value.status_code == 429 + assert len(attempts) == 1 + + +def test_sync_retry_is_single_shot() -> None: + """A second rejection surfaces instead of looping.""" + attempts: list[str] = [] + + def send(body: str, headers: dict) -> str: + attempts.append(body) + raise BedrockError(status_code=400, message=_STRICT_REJECTION) + + with pytest.raises(BedrockError): + BedrockConverseLLM()._send_retrying_rejected_tool_fields(send=send, **_retry_kwargs()) + + assert len(attempts) == 2 + + +@pytest.mark.asyncio +async def test_async_retry_resends_without_the_rejected_field_and_resigns() -> None: + attempts: list[tuple[str, dict]] = [] + + async def send(body: str, headers: dict) -> str: + attempts.append((body, headers)) + if len(attempts) == 1: + raise BedrockError(status_code=400, message=_STRICT_REJECTION) + return "ok" + + result = await BedrockConverseLLM()._asend_retrying_rejected_tool_fields(send=send, **_retry_kwargs()) + + assert result == "ok" + assert len(attempts) == 2 + assert '"strict"' not in attempts[1][0] + assert attempts[1][1]["Authorization"].startswith("AWS4-HMAC-SHA256") + + +@pytest.mark.asyncio +async def test_async_retry_leaves_unrelated_errors_alone() -> None: + attempts: list[str] = [] + + async def send(body: str, headers: dict) -> str: + attempts.append(body) + raise BedrockError(status_code=500, message="InternalServerException") + + with pytest.raises(BedrockError) as excinfo: + await BedrockConverseLLM()._asend_retrying_rejected_tool_fields(send=send, **_retry_kwargs()) + + assert excinfo.value.status_code == 500 + assert len(attempts) == 1