mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-15 23:31:29 +00:00
fix(bedrock): retry once, re-signed, when Converse rejects extra tool 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 with `tools.<i>.<variant>.<field>: Extra inputs are not permitted`. Today the only defence is `bedrock_converse_supports_strict_tools` in the pricing map, which means every newly released Claude model is broken for tool calling until a human notices and adds the flag. Converse now drops the fields the provider just named and sends the request again. `parse_rejected_tool_fields` reads the verdict off the error and is shared with azure_ai, which already had this behaviour for the same error string; the Bedrock side owns applying it to the Converse request shape. The retry is single-shot, and a failure that is not an extra-tool-field rejection raises exactly what it raised before. The Bedrock-specific part is signing. SigV4 commits to 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. All four Converse call paths (sync and async, streaming and not) go through one wrapper that takes the transport as a parameter, so the retry logic exists once and each path keeps its own error contract. The two error shapes Converse can raise are both handled: the non-streaming paths convert to BedrockError themselves, the streaming paths surface the transport's MaskedHTTPStatusError. Verified against real Bedrock in us-east-1 on `us.anthropic.claude-sonnet-5`, which currently lacks the pricing-map flag: all four paths returned a real tool call for a request carrying `strict: true` that 400s without this change.
This commit is contained in:
parent
d4d0bf0acc
commit
37c8efde0f
5 changed files with 515 additions and 50 deletions
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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<bracket_index>\d+)\]|\.(?P<dot_index>\d+)\.[A-Za-z_][\w-]*)\.(?P<field>[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:
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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).
|
||||
|
|
|
|||
|
|
@ -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
|
||||
Loading…
Add table
Reference in a new issue