mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-11-06 05:17:15 +00:00
fix(bedrock): apply configured request timeouts to Bedrock clients
providers.bedrock.request_timeout_seconds and providers.bedrock.models.<model>.timeout_seconds resolved correctly through hermes_cli.timeouts.get_provider_request_timeout, but no Bedrock client was built with the result. The cached boto3 bedrock-runtime client (ambient and profile-scoped) was created with no botocore Config, so it kept botocore's 60s read / 60s connect, and the AnthropicBedrock SDK client was built with _client_timeout(None), i.e. a fixed 900s read. The resolver (3143d32330) landed after the cached Converse client (0cb8c51fa5) and was wired only into the OpenAI-wire and native Anthropic builders;611657487fdocumented Bedrock as not covered instead of wiring it. - _get_bedrock_runtime_client(region, model) resolves the model's timeout (model over provider) and passes Config(read_timeout=..., connect_timeout=60) only when one is configured; unset or invalid builds the exact no-Config client used before. - Both client caches (ambient and per-profile-home) key on the effective read timeout, so models with different overrides in one region never share a client, and the same timeout reuses one client. Stale-connection eviction drops every timeout slot of the region. - converse/converse_stream dispatch picks the client from the request's modelId, so a /model switch or a fallback to another Bedrock model uses that model's timeout. call_converse and the context probe pass their model. - AnthropicBedrock: build_anthropic_bedrock_client takes the timeout; the bound client and the per-request client key carry the active model's value (connect stays 10s, unset stays 900s). - Docs: replace "not currently wired for AWS Bedrock" with what is wired. This closes a configuration gap only. A socket read timeout bounds each read, not the whole call; retry policy and the stale detector are unchanged.
This commit is contained in:
1 parent
0bb556f3a9
commit
a8b987aaa1
10 files changed
+345
-45
No files matched your search
@@ -520,13 +520,14 @@ def build_anthropic_client(api_key, base_url: str | None = None, timeout: float
|
||||
return _new_sdk_client(sdk, kwargs, headers, route=base_url)
|
||||
|
||||
|
||||
def build_anthropic_bedrock_client(region: str):
|
||||
def build_anthropic_bedrock_client(region: str, timeout: float | None = None):
|
||||
"""AnthropicBedrock client for Bedrock Claude models (boto3 default credential chain). The
|
||||
SDK's native Bedrock adapter gives full Claude feature parity (prompt caching, thinking
|
||||
budgets, adaptive thinking, fast mode) that Converse lacks. The common betas plus
|
||||
``context-1m-2025-08-07`` are attached: without the latter Bedrock caps Opus 4.6/4.7 at 200K.
|
||||
A configured ``bedrock.guardrail`` rides as InvokeModel headers so every client built here
|
||||
(primary, auxiliary, per-request rebuild) enforces it."""
|
||||
(primary, auxiliary, per-request rebuild) enforces it. ``timeout`` is the resolved
|
||||
``providers.bedrock`` request timeout; None keeps the 900s read / 10s connect default."""
|
||||
from agent.bedrock_adapter import bedrock_guardrail_headers, scoped_aws_session_kwargs
|
||||
sdk = _require_sdk("the Bedrock provider")
|
||||
if not hasattr(sdk, "AnthropicBedrock"):
|
||||
@@ -537,7 +538,7 @@ def build_anthropic_bedrock_client(region: str):
|
||||
aws_kwargs = {"aws_access_key": scoped.get("aws_access_key_id"), "aws_secret_key": scoped.get("aws_secret_access_key"),
|
||||
"aws_session_token": scoped.get("aws_session_token"), "aws_profile": scoped.get("profile_name")}
|
||||
return sdk.AnthropicBedrock(
|
||||
aws_region=region, timeout=_client_timeout(None), **{k: v for k, v in aws_kwargs.items() if v},
|
||||
aws_region=region, timeout=_client_timeout(timeout), **{k: v for k, v in aws_kwargs.items() if v},
|
||||
max_retries=0, # retry belongs to hermes's outer loop (honors Retry-After)
|
||||
default_headers={**_beta_header([*_COMMON_BETAS, _CONTEXT_1M_BETA]), **bedrock_guardrail_headers()},
|
||||
)
|
||||
|
||||
+56
-24
@@ -25,11 +25,16 @@ from agent.errors import EmptyStreamError
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
_bedrock_runtime_client_cache: dict[str, Any] = {}
|
||||
_bedrock_control_client_cache: dict[str, Any] = {}
|
||||
# Routed multiplex profiles: one client per (profile home, region). boto3 freezes the credential
|
||||
# chain into the client at construction, so a region-only slot would sign profile B's calls with A's keys.
|
||||
_bedrock_clients_by_home: dict[tuple[str, str, str], Any] = {}
|
||||
# Keyed by (region, read_timeout): boto3 freezes its Config into the client, so two models with
|
||||
# different ``timeout_seconds`` overrides in one region need two clients (None = botocore default).
|
||||
_bedrock_runtime_client_cache: dict[tuple[str, Optional[float]], Any] = {}
|
||||
_bedrock_control_client_cache: dict[tuple[str, Optional[float]], Any] = {}
|
||||
# Routed multiplex profiles: one client per (profile home, service, region, read_timeout). boto3 freezes
|
||||
# the credential chain into the client too, so a region-only slot would sign profile B's calls with A's keys.
|
||||
_bedrock_clients_by_home: dict[tuple[str, str, str, Optional[float]], Any] = {}
|
||||
# botocore's own connect default, set explicitly so a long read budget never stretches the TCP/TLS
|
||||
# connect wait: a 600s ``request_timeout_seconds`` is for slow generations, not unreachable endpoints.
|
||||
_BOTO_CONNECT_TIMEOUT_SECONDS = 60
|
||||
|
||||
# botocore session kwarg <- profile .env variable (the explicit sources of the default chain).
|
||||
_AWS_SCOPED_CREDENTIAL_VARS: tuple[tuple[str, str], ...] = (
|
||||
@@ -123,28 +128,46 @@ def _require_boto3():
|
||||
return boto3
|
||||
|
||||
|
||||
def _cached_client(cache: dict[str, Any], service: str, region: str):
|
||||
"""Get or create a per-region boto3 client. Unscoped: the default credential chain, one client per
|
||||
region. Routed profile: one client per (home, service, region), built from that profile's scoped
|
||||
``AWS_*`` (falling back to the default chain only for what the profile does not set)."""
|
||||
def _new_client(factory: Any, service: str, region: str, read_timeout: Optional[float]):
|
||||
"""``factory.client(...)``; no ``Config`` at all when no timeout is configured, so the unset
|
||||
case builds exactly the client botocore always built (60s read / 60s connect)."""
|
||||
if read_timeout is None:
|
||||
return factory.client(service, region_name=region)
|
||||
from botocore.config import Config
|
||||
config = Config(read_timeout=read_timeout, connect_timeout=_BOTO_CONNECT_TIMEOUT_SECONDS)
|
||||
return factory.client(service, region_name=region, config=config)
|
||||
|
||||
|
||||
def _cached_client(cache: dict[tuple[str, Optional[float]], Any], service: str, region: str, *,
|
||||
read_timeout: Optional[float] = None):
|
||||
"""Get or create a boto3 client per (region, read timeout). Unscoped: the default credential
|
||||
chain. Routed profile: one client per (home, service, region, read timeout), built from that
|
||||
profile's scoped ``AWS_*`` (falling back to the default chain only for what the profile does not set)."""
|
||||
from hermes_constants import get_hermes_home_override, hermes_home_key
|
||||
if get_hermes_home_override() is None:
|
||||
if region not in cache:
|
||||
cache[region] = _require_boto3().client(service, region_name=region)
|
||||
return cache[region]
|
||||
key = (hermes_home_key(), service, region)
|
||||
client = _bedrock_clients_by_home.get(key)
|
||||
key = (region, read_timeout)
|
||||
if key not in cache:
|
||||
cache[key] = _new_client(_require_boto3(), service, region, read_timeout)
|
||||
return cache[key]
|
||||
home_key = (hermes_home_key(), service, region, read_timeout)
|
||||
client = _bedrock_clients_by_home.get(home_key)
|
||||
if client is None:
|
||||
# Scope check first: a cred-less multiplex profile must hit the ambient-chain
|
||||
# refusal, not a boto3 ImportError on hosts that lack the package.
|
||||
kwargs = scoped_aws_session_kwargs()
|
||||
client = _require_boto3().Session(**kwargs).client(service, region_name=region)
|
||||
_bedrock_clients_by_home[key] = client
|
||||
client = _new_client(_require_boto3().Session(**kwargs), service, region, read_timeout)
|
||||
_bedrock_clients_by_home[home_key] = client
|
||||
return client
|
||||
|
||||
|
||||
def _get_bedrock_runtime_client(region: str):
|
||||
return _cached_client(_bedrock_runtime_client_cache, "bedrock-runtime", region)
|
||||
def _get_bedrock_runtime_client(region: str, model: Optional[str] = None):
|
||||
"""The ``bedrock-runtime`` client for *region* whose read timeout is *model*'s configured
|
||||
request timeout (``providers.bedrock.models.<model>.timeout_seconds`` over
|
||||
``providers.bedrock.request_timeout_seconds``). Resolved per call, so a model switch or a
|
||||
fallback to another Bedrock model gets that model's budget, not the one the session started with."""
|
||||
from hermes_cli.timeouts import get_provider_request_timeout
|
||||
return _cached_client(_bedrock_runtime_client_cache, "bedrock-runtime", region,
|
||||
read_timeout=get_provider_request_timeout("bedrock", model))
|
||||
|
||||
|
||||
def _get_bedrock_control_client(region: str):
|
||||
@@ -161,11 +184,18 @@ def reset_client_cache():
|
||||
|
||||
|
||||
def invalidate_runtime_client(region: str) -> bool:
|
||||
"""Evict one region's cached ``bedrock-runtime`` client (stale HTTP pool); True if evicted."""
|
||||
"""Evict one region's cached ``bedrock-runtime`` clients, every read timeout (stale HTTP pool);
|
||||
True if any was evicted."""
|
||||
from hermes_constants import get_hermes_home_override, hermes_home_key
|
||||
if get_hermes_home_override() is not None:
|
||||
return _bedrock_clients_by_home.pop((hermes_home_key(), "bedrock-runtime", region), None) is not None
|
||||
return _bedrock_runtime_client_cache.pop(region, None) is not None
|
||||
cache: dict = _bedrock_clients_by_home
|
||||
prefix = (hermes_home_key(), "bedrock-runtime", region)
|
||||
else:
|
||||
cache, prefix = _bedrock_runtime_client_cache, (region,)
|
||||
stale = [key for key in list(cache) if key[:len(prefix)] == prefix]
|
||||
for key in stale:
|
||||
cache.pop(key, None)
|
||||
return bool(stale)
|
||||
|
||||
|
||||
|
||||
@@ -470,7 +500,9 @@ def bind_bedrock_runtime(agent, base_url: str, api_mode: str) -> None:
|
||||
agent._is_anthropic_oauth = False
|
||||
if api_mode == "anthropic_messages":
|
||||
from agent.anthropic_adapter import build_anthropic_bedrock_client
|
||||
agent._anthropic_client = build_anthropic_bedrock_client(agent._bedrock_region)
|
||||
from hermes_cli.timeouts import get_provider_request_timeout
|
||||
agent._anthropic_client = build_anthropic_bedrock_client(
|
||||
agent._bedrock_region, timeout=get_provider_request_timeout("bedrock", getattr(agent, "model", None)))
|
||||
else:
|
||||
agent._anthropic_client = None
|
||||
|
||||
@@ -1326,7 +1358,7 @@ def call_converse(
|
||||
"""Non-streaming Converse call → OpenAI-compatible response. Retries once without a rejected cachePoint
|
||||
placement; evicts the cached client on stale-connection errors."""
|
||||
from agent.prompt_caching import is_one_shot_request
|
||||
client = _get_bedrock_runtime_client(region)
|
||||
client = _get_bedrock_runtime_client(region, model)
|
||||
# Auxiliary-only entry point: a one-shot's cachePoint is a write no later call reads.
|
||||
kwargs = build_converse_kwargs(model, messages, tools, max_tokens, temperature, top_p, stop_sequences, guardrail_config,
|
||||
prompt_cache=not is_one_shot_request(messages, tools))
|
||||
@@ -1494,7 +1526,7 @@ def probe_bedrock_context_length(model_id: str, region: str) -> Optional[int]:
|
||||
probe costs nothing. An accepted tier is a safe lower bound; None (no creds/network/unparseable) → static table."""
|
||||
from agent.model_metadata import parse_context_limit_from_error
|
||||
try:
|
||||
client = _get_bedrock_runtime_client(region)
|
||||
client = _get_bedrock_runtime_client(region, model_id)
|
||||
except Exception as exc: # boto3 missing / credential resolution failure
|
||||
logger.debug("Bedrock context probe skipped for %s: %s", model_id, exc)
|
||||
return None
|
||||
|
||||
@@ -739,7 +739,7 @@ def _bedrock_converse_call(api_kwargs: dict, *, stream: bool, on_stream_denied=N
|
||||
recover_from_cache_point_rejection, recover_from_redacted_reasoning_rejection)
|
||||
region = api_kwargs.pop("__bedrock_region__", "us-east-1")
|
||||
api_kwargs.pop("__bedrock_converse__", None)
|
||||
client = _get_bedrock_runtime_client(region)
|
||||
client = _get_bedrock_runtime_client(region, api_kwargs.get("modelId")) # the active model's timeout
|
||||
method = client.converse_stream if stream else client.converse
|
||||
finish = (lambda raw: raw.get("stream", [])) if stream else normalize_converse_response
|
||||
try:
|
||||
|
||||
@@ -489,7 +489,8 @@ class ClientLifecycleMixin:
|
||||
def _request_anthropic_client_key(self) -> tuple:
|
||||
"""Cache key over everything forcing a fresh client: credential, base URL/region, timeout, 1M-beta flag."""
|
||||
if getattr(self, "provider", None) == "bedrock":
|
||||
return ("bedrock", getattr(self, "_bedrock_region", "us-east-1") or "us-east-1")
|
||||
return ("bedrock", getattr(self, "_bedrock_region", "us-east-1") or "us-east-1",
|
||||
get_provider_request_timeout(self.provider, self.model))
|
||||
return (
|
||||
"direct", self._anthropic_api_key, getattr(self, "_anthropic_base_url", None),
|
||||
get_provider_request_timeout(self.provider, self.model), bool(getattr(self, "_oauth_1m_beta_disabled", False)),
|
||||
@@ -508,7 +509,7 @@ class ClientLifecycleMixin:
|
||||
def _build_anthropic_client_for_key(self, key: tuple) -> Any:
|
||||
from agent.anthropic_adapter import build_anthropic_bedrock_client, build_anthropic_client
|
||||
if key[0] == "bedrock":
|
||||
return build_anthropic_bedrock_client(key[1])
|
||||
return build_anthropic_bedrock_client(key[1], timeout=key[2])
|
||||
return build_anthropic_client(key[1], key[2], timeout=key[3], drop_context_1m_beta=key[4])
|
||||
|
||||
def _create_request_anthropic_client(self, *, reason: str) -> Any:
|
||||
|
||||
@@ -288,8 +288,10 @@ model:
|
||||
# implicit non-stream stale detector is auto-disabled for local endpoints
|
||||
# and can scale upward for very large contexts.
|
||||
#
|
||||
# Not currently wired for AWS Bedrock (bedrock_converse + AnthropicBedrock
|
||||
# SDK paths) — those use boto3 with its own timeout configuration.
|
||||
# AWS Bedrock (providers.bedrock): the resolved value is the socket read
|
||||
# timeout of the boto3 Converse client and of the AnthropicBedrock SDK
|
||||
# client, chosen per request for the active model. Connect stays fixed
|
||||
# (botocore 60s / SDK 10s); unset keeps botocore's 60s read on Converse.
|
||||
#
|
||||
# providers:
|
||||
# ollama-local:
|
||||
|
||||
@@ -495,7 +495,7 @@ def test_bedrock_silent_nonstream_request_is_isolated_without_close_wakeup() ->
|
||||
release = threading.Event()
|
||||
runtime_client = _BedrockRuntimeClient(started, release)
|
||||
reset_client_cache()
|
||||
_bedrock_runtime_client_cache["us-test-1"] = runtime_client
|
||||
_bedrock_runtime_client_cache[("us-test-1", None)] = runtime_client
|
||||
client = aux.BedrockAuxiliaryClient("us-test-1", "bedrock-test")
|
||||
try:
|
||||
exc, elapsed = _cancel_silent_request(client, started, _invoke_generic)
|
||||
|
||||
@@ -1391,14 +1391,16 @@ class TestInvalidateRuntimeClient:
|
||||
reset_client_cache,
|
||||
)
|
||||
reset_client_cache()
|
||||
_bedrock_runtime_client_cache["us-east-1"] = "dead-client"
|
||||
_bedrock_runtime_client_cache["us-west-2"] = "live-client"
|
||||
# Keyed by (region, read_timeout): every timeout variant of the region shares the dead pool.
|
||||
_bedrock_runtime_client_cache[("us-east-1", None)] = "dead-client"
|
||||
_bedrock_runtime_client_cache[("us-east-1", 300.0)] = "dead-client-300"
|
||||
_bedrock_runtime_client_cache[("us-west-2", None)] = "live-client"
|
||||
|
||||
evicted = invalidate_runtime_client("us-east-1")
|
||||
|
||||
assert evicted is True
|
||||
assert "us-east-1" not in _bedrock_runtime_client_cache
|
||||
assert _bedrock_runtime_client_cache["us-west-2"] == "live-client"
|
||||
assert not [key for key in _bedrock_runtime_client_cache if key[0] == "us-east-1"]
|
||||
assert _bedrock_runtime_client_cache[("us-west-2", None)] == "live-client"
|
||||
|
||||
def test_returns_false_when_region_not_cached(self):
|
||||
from agent.bedrock_adapter import invalidate_runtime_client, reset_client_cache
|
||||
@@ -1463,7 +1465,7 @@ class TestCallConverseInvalidatesOnStaleError:
|
||||
error_response={"Error": {"Code": "ValidationException", "Message": "bad"}},
|
||||
operation_name="Converse",
|
||||
)
|
||||
_bedrock_runtime_client_cache["us-east-1"] = live_client
|
||||
_bedrock_runtime_client_cache[("us-east-1", None)] = live_client
|
||||
|
||||
with pytest.raises(ClientError):
|
||||
call_converse(
|
||||
@@ -1472,7 +1474,7 @@ class TestCallConverseInvalidatesOnStaleError:
|
||||
messages=[{"role": "user", "content": "hi"}],
|
||||
)
|
||||
|
||||
assert _bedrock_runtime_client_cache.get("us-east-1") is live_client, (
|
||||
assert _bedrock_runtime_client_cache.get(("us-east-1", None)) is live_client, (
|
||||
"validation errors do not indicate a dead connection — keep the client"
|
||||
)
|
||||
|
||||
@@ -1542,7 +1544,7 @@ class TestAgentBedrockStreamRecovery:
|
||||
"stopReason": "end_turn",
|
||||
"usage": {"inputTokens": 1, "outputTokens": 1, "totalTokens": 2},
|
||||
}
|
||||
_bedrock_runtime_client_cache["us-east-1"] = client
|
||||
_bedrock_runtime_client_cache[("us-east-1", None)] = client
|
||||
agent = SimpleNamespace(_disable_streaming=False, _safe_print=MagicMock(), model="m", provider="bedrock")
|
||||
stream = _BedrockStream(agent, dict(self._KW), on_first_delta=None)
|
||||
|
||||
@@ -1554,7 +1556,7 @@ class TestAgentBedrockStreamRecovery:
|
||||
assert agent._disable_streaming is True
|
||||
assert "InvokeModelWithResponseStream" in agent._safe_print.call_args.args[0]
|
||||
# Not a stale connection — client stays cached.
|
||||
assert _bedrock_runtime_client_cache.get("us-east-1") is client
|
||||
assert _bedrock_runtime_client_cache.get(("us-east-1", None)) is client
|
||||
|
||||
@pytest.mark.parametrize("stream", [True, False])
|
||||
@pytest.mark.parametrize("retry_fails", [False, True])
|
||||
@@ -1625,14 +1627,14 @@ class TestAgentBedrockStreamRecovery:
|
||||
reset_client_cache()
|
||||
dead_client = MagicMock()
|
||||
dead_client.converse_stream.side_effect = ConnectionClosedError(endpoint_url="https://bedrock.example")
|
||||
_bedrock_runtime_client_cache["us-east-1"] = dead_client
|
||||
_bedrock_runtime_client_cache[("us-east-1", None)] = dead_client
|
||||
denied = MagicMock()
|
||||
|
||||
with pytest.raises(ConnectionClosedError):
|
||||
_bedrock_converse_call(dict(self._KW), stream=True, on_stream_denied=denied)
|
||||
|
||||
denied.assert_not_called()
|
||||
assert "us-east-1" not in _bedrock_runtime_client_cache
|
||||
assert ("us-east-1", None) not in _bedrock_runtime_client_cache
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -0,0 +1,262 @@
|
||||
"""Bedrock request timeouts reach the clients that serve the call.
|
||||
|
||||
``providers.bedrock.request_timeout_seconds`` and ``providers.bedrock.models.<id>.timeout_seconds``
|
||||
resolved correctly but never reached a client: the boto3 Converse client kept botocore's 60s read
|
||||
timeout and the AnthropicBedrock SDK client its 900s default. These tests read the real botocore /
|
||||
httpx client config (dummy credentials, IMDS off) and intercept the botocore API call itself, so no
|
||||
request leaves the process.
|
||||
"""
|
||||
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
pytest.importorskip("boto3")
|
||||
pytest.importorskip("botocore.client")
|
||||
|
||||
import boto3
|
||||
import botocore.client
|
||||
|
||||
from agent import bedrock_adapter
|
||||
from agent.bedrock_adapter import _get_bedrock_runtime_client, invalidate_runtime_client
|
||||
|
||||
REGION = "us-east-1"
|
||||
RUNTIME_URL = f"https://bedrock-runtime.{REGION}.amazonaws.com"
|
||||
CLAUDE = "us.anthropic.claude-sonnet-4-5-20250929-v1:0" # model override 300
|
||||
NOVA = "us.amazon.nova-pro-v1:0" # no model override -> provider 600
|
||||
LLAMA = "us.meta.llama4-maverick-17b-instruct-v1:0" # model override 120
|
||||
OVERRIDES = {"providers": {"bedrock": {
|
||||
"request_timeout_seconds": 600,
|
||||
"models": {CLAUDE: {"timeout_seconds": 300}, LLAMA: {"timeout_seconds": 120}},
|
||||
}}}
|
||||
BOTOCORE_DEFAULT_TIMEOUT = 60
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def aws_env(monkeypatch):
|
||||
monkeypatch.setenv("AWS_ACCESS_KEY_ID", "AKIA-AMBIENT")
|
||||
monkeypatch.setenv("AWS_SECRET_ACCESS_KEY", "ambient-secret")
|
||||
monkeypatch.setenv("AWS_EC2_METADATA_DISABLED", "true")
|
||||
for var in ("AWS_PROFILE", "AWS_SESSION_TOKEN", "AWS_BEARER_TOKEN_BEDROCK"):
|
||||
monkeypatch.delenv(var, raising=False)
|
||||
bedrock_adapter.reset_client_cache()
|
||||
yield
|
||||
bedrock_adapter.reset_client_cache()
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def config(monkeypatch):
|
||||
"""Install ``cfg`` as the loaded config (the operator report's offline technique)."""
|
||||
def install(cfg):
|
||||
monkeypatch.setattr("hermes_cli.config.load_config_readonly", lambda: cfg)
|
||||
install({})
|
||||
return install
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def botocore_calls(monkeypatch):
|
||||
"""Intercept every botocore API call: record (operation, serving client, its read timeout)."""
|
||||
calls = []
|
||||
|
||||
def fake_make_api_call(client, operation_name, params):
|
||||
calls.append((operation_name, client, client.meta.config.read_timeout, params.get("modelId")))
|
||||
if operation_name == "ConverseStream":
|
||||
return {"stream": []}
|
||||
return {"output": {"message": {"role": "assistant", "content": [{"text": "ok"}]}},
|
||||
"stopReason": "end_turn", "usage": {"inputTokens": 1, "outputTokens": 1, "totalTokens": 2}}
|
||||
monkeypatch.setattr(botocore.client.BaseClient, "_make_api_call", fake_make_api_call)
|
||||
return calls
|
||||
|
||||
|
||||
def _converse_kwargs(model):
|
||||
return {"__bedrock_converse__": True, "__bedrock_region__": REGION, "modelId": model,
|
||||
"messages": [{"role": "user", "content": [{"text": "hi"}]}]}
|
||||
|
||||
|
||||
# --- client construction ----------------------------------------------------------------------
|
||||
|
||||
def test_unset_config_builds_exactly_the_botocore_default_client(config):
|
||||
baseline = boto3.client("bedrock-runtime", region_name=REGION).meta.config
|
||||
|
||||
built = _get_bedrock_runtime_client(REGION, CLAUDE).meta.config
|
||||
|
||||
assert (built.read_timeout, built.connect_timeout) == (BOTOCORE_DEFAULT_TIMEOUT, BOTOCORE_DEFAULT_TIMEOUT)
|
||||
assert (built.read_timeout, built.connect_timeout, built.retries, built.max_pool_connections) == (
|
||||
baseline.read_timeout, baseline.connect_timeout, baseline.retries, baseline.max_pool_connections)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("model, expected", [(NOVA, 600), (CLAUDE, 300)], ids=["provider", "model-wins"])
|
||||
def test_configured_timeout_is_the_client_read_timeout_and_connect_stays_bounded(config, model, expected):
|
||||
config(OVERRIDES)
|
||||
|
||||
client_config = _get_bedrock_runtime_client(REGION, model).meta.config
|
||||
|
||||
assert client_config.read_timeout == expected
|
||||
assert client_config.connect_timeout == BOTOCORE_DEFAULT_TIMEOUT, "a long read budget must not stretch connect"
|
||||
assert client_config.retries == boto3.client("bedrock-runtime", region_name=REGION).meta.config.retries
|
||||
|
||||
|
||||
@pytest.mark.parametrize("provider_value, model_value", [("soon", -5), (0, "later"), ("", None)])
|
||||
def test_invalid_values_behave_like_unset(config, provider_value, model_value):
|
||||
config({"providers": {"bedrock": {"request_timeout_seconds": provider_value,
|
||||
"models": {CLAUDE: {"timeout_seconds": model_value}}}}})
|
||||
|
||||
client = _get_bedrock_runtime_client(REGION, CLAUDE)
|
||||
|
||||
assert client.meta.config.read_timeout == BOTOCORE_DEFAULT_TIMEOUT
|
||||
assert client is _get_bedrock_runtime_client(REGION), "invalid must share the unset client, not a new one"
|
||||
|
||||
|
||||
def test_models_with_different_overrides_never_share_a_client_in_one_region(config):
|
||||
config(OVERRIDES)
|
||||
|
||||
claude, nova, llama = (_get_bedrock_runtime_client(REGION, m) for m in (CLAUDE, NOVA, LLAMA))
|
||||
|
||||
assert [c.meta.config.read_timeout for c in (claude, nova, llama)] == [300, 600, 120]
|
||||
assert len({id(claude), id(nova), id(llama)}) == 3
|
||||
# Bounded: same region + same effective timeout reuses one client.
|
||||
assert _get_bedrock_runtime_client(REGION, CLAUDE) is claude
|
||||
assert _get_bedrock_runtime_client(REGION, "us.amazon.nova-lite-v1:0") is nova # provider-level 600 too
|
||||
|
||||
config({}) # unset must not collide with any configured slot
|
||||
unset = _get_bedrock_runtime_client(REGION, CLAUDE)
|
||||
assert unset not in (claude, nova, llama)
|
||||
assert unset.meta.config.read_timeout == BOTOCORE_DEFAULT_TIMEOUT
|
||||
|
||||
|
||||
def test_profile_scoped_clients_carry_their_own_profile_timeouts(tmp_path, monkeypatch):
|
||||
"""A routed profile's clients come from its own config.yaml and own cache slot, per timeout."""
|
||||
from agent import secret_scope
|
||||
from hermes_constants import reset_hermes_home_override, set_hermes_home_override
|
||||
|
||||
home = tmp_path / "profile-home"
|
||||
home.mkdir()
|
||||
(home / ".env").write_text("AWS_ACCESS_KEY_ID=AKIA-PROFILE\nAWS_SECRET_ACCESS_KEY=profile-secret\n")
|
||||
(home / "config.yaml").write_text(
|
||||
"providers:\n bedrock:\n request_timeout_seconds: 600\n"
|
||||
f" models:\n {CLAUDE}:\n timeout_seconds: 300\n")
|
||||
monkeypatch.setattr(secret_scope, "_MULTIPLEX_ACTIVE", True)
|
||||
|
||||
def in_profile(fn):
|
||||
h_tok = set_hermes_home_override(str(home))
|
||||
s_tok = secret_scope.set_secret_scope(secret_scope.build_profile_secret_scope(home))
|
||||
try:
|
||||
return fn()
|
||||
finally:
|
||||
secret_scope.reset_secret_scope(s_tok)
|
||||
reset_hermes_home_override(h_tok)
|
||||
|
||||
claude, nova, claude_again = in_profile(lambda: (
|
||||
_get_bedrock_runtime_client(REGION, CLAUDE), _get_bedrock_runtime_client(REGION, NOVA),
|
||||
_get_bedrock_runtime_client(REGION, CLAUDE)))
|
||||
|
||||
assert (claude.meta.config.read_timeout, nova.meta.config.read_timeout) == (300, 600)
|
||||
assert claude.meta.config.connect_timeout == nova.meta.config.connect_timeout == BOTOCORE_DEFAULT_TIMEOUT
|
||||
assert claude is claude_again and claude is not nova
|
||||
assert claude._request_signer._credentials.access_key == "AKIA-PROFILE"
|
||||
# The launch context (no override, its own unset config) keeps botocore's default client.
|
||||
ambient = _get_bedrock_runtime_client(REGION, CLAUDE)
|
||||
assert ambient is not claude and ambient.meta.config.read_timeout == BOTOCORE_DEFAULT_TIMEOUT
|
||||
# Stale-pool eviction drops every timeout slot of the profile's region, never the launch context's.
|
||||
assert in_profile(lambda: invalidate_runtime_client(REGION)) is True
|
||||
assert in_profile(lambda: _get_bedrock_runtime_client(REGION, CLAUDE)) is not claude
|
||||
assert _get_bedrock_runtime_client(REGION, CLAUDE) is ambient
|
||||
|
||||
|
||||
# --- dispatch ---------------------------------------------------------------------------------
|
||||
|
||||
def test_converse_and_converse_stream_are_served_with_the_requested_models_timeout(config, botocore_calls):
|
||||
from agent.chat_completion_helpers import _BedrockStream, _dispatch_nonstreaming_api_request
|
||||
config(OVERRIDES)
|
||||
|
||||
agent = SimpleNamespace(api_mode="bedrock_converse", provider="bedrock", model=CLAUDE,
|
||||
_disable_streaming=False, _safe_print=MagicMock())
|
||||
_dispatch_nonstreaming_api_request(agent, _converse_kwargs(CLAUDE), make_client=None)
|
||||
_BedrockStream(agent, _converse_kwargs(NOVA), on_first_delta=None)._open_stream(_converse_kwargs(NOVA))
|
||||
|
||||
assert [(op, timeout, model) for op, _client, timeout, model in botocore_calls] == [
|
||||
("Converse", 300, CLAUDE), ("ConverseStream", 600, NOVA)]
|
||||
|
||||
|
||||
def test_auxiliary_call_converse_uses_the_models_timeout(config, botocore_calls):
|
||||
from agent.bedrock_adapter import call_converse
|
||||
config(OVERRIDES)
|
||||
|
||||
call_converse(region=REGION, model=LLAMA, messages=[{"role": "user", "content": "hi"}])
|
||||
|
||||
assert [(op, timeout) for op, _c, timeout, _m in botocore_calls] == [("Converse", 120)]
|
||||
|
||||
|
||||
def _agent():
|
||||
from agent.context_compressor import ContextCompressor
|
||||
from run_agent import AIAgent
|
||||
agent = AIAgent.__new__(AIAgent)
|
||||
agent.model, agent.provider = "claude-opus-4.8", "anthropic"
|
||||
agent.base_url, agent.api_key, agent.api_mode = "https://api.anthropic.com", "sk-ant", "anthropic_messages"
|
||||
agent.client, agent._anthropic_client, agent._client_kwargs = None, MagicMock(), {}
|
||||
agent.quiet_mode, agent._config_context_length, agent._primary_runtime = True, None, {}
|
||||
agent.max_tokens = None
|
||||
agent.context_compressor = ContextCompressor(
|
||||
model=agent.model, threshold_percent=0.5, base_url=agent.base_url, api_key="sk-ant",
|
||||
provider="anthropic", quiet_mode=True, config_context_length=None,
|
||||
)
|
||||
return agent
|
||||
|
||||
|
||||
def _dispatch_turn(agent):
|
||||
from agent.chat_completion_helpers import _build_bedrock_kwargs, _dispatch_nonstreaming_api_request
|
||||
kwargs = _build_bedrock_kwargs(agent, [{"role": "user", "content": "hi"}], None, None)
|
||||
_dispatch_nonstreaming_api_request(agent, kwargs, make_client=None)
|
||||
|
||||
|
||||
def test_model_switch_and_fallback_dispatch_with_the_new_models_timeout(config, botocore_calls, monkeypatch):
|
||||
from agent.client_lifecycle import _swap_fallback_clients
|
||||
monkeypatch.setattr("agent.model_metadata.get_model_context_length", lambda *a, **k: 200_000)
|
||||
config(OVERRIDES)
|
||||
agent = _agent()
|
||||
|
||||
agent.switch_model(LLAMA, "bedrock", api_key="aws-sdk", base_url=RUNTIME_URL, api_mode="bedrock_converse")
|
||||
_dispatch_turn(agent)
|
||||
# Fallback onto another Bedrock model, bound the way route_binding.bind_route_entry binds it.
|
||||
agent.model, agent.provider, agent.base_url, agent.api_mode = NOVA, "bedrock", RUNTIME_URL, "bedrock_converse"
|
||||
_swap_fallback_clients(agent, MagicMock(base_url=RUNTIME_URL, api_key="aws-sdk"), "bedrock", NOVA,
|
||||
RUNTIME_URL, "bedrock_converse")
|
||||
_dispatch_turn(agent)
|
||||
agent.switch_model(LLAMA, "bedrock", api_key="aws-sdk", base_url=RUNTIME_URL, api_mode="bedrock_converse")
|
||||
_dispatch_turn(agent)
|
||||
|
||||
assert [(timeout, model) for _op, _c, timeout, model in botocore_calls] == [
|
||||
(120, LLAMA), (600, NOVA), (120, LLAMA)]
|
||||
first, second, third = (client for _op, client, _t, _m in botocore_calls)
|
||||
assert first is third and first is not second
|
||||
|
||||
|
||||
# --- AnthropicBedrock (Claude via the anthropic SDK) ------------------------------------------
|
||||
|
||||
def test_anthropic_bedrock_client_takes_the_timeout_and_keeps_default_when_unset():
|
||||
pytest.importorskip("anthropic")
|
||||
from agent.anthropic_adapter import build_anthropic_bedrock_client
|
||||
|
||||
configured = build_anthropic_bedrock_client(REGION, timeout=300.0).timeout
|
||||
unset = build_anthropic_bedrock_client(REGION).timeout
|
||||
|
||||
assert (configured.read, configured.connect) == (300.0, 10.0)
|
||||
assert (unset.read, unset.connect) == (900.0, 10.0)
|
||||
|
||||
|
||||
def test_anthropic_bedrock_request_client_follows_the_active_model(config, monkeypatch):
|
||||
pytest.importorskip("anthropic")
|
||||
monkeypatch.setattr("agent.model_metadata.get_model_context_length", lambda *a, **k: 200_000)
|
||||
other_claude = "us.anthropic.claude-haiku-4-5-20251001-v1:0" # provider-level 600
|
||||
config(OVERRIDES)
|
||||
agent = _agent()
|
||||
|
||||
agent.switch_model(CLAUDE, "bedrock", api_key="aws-sdk", base_url=RUNTIME_URL, api_mode="anthropic_messages")
|
||||
shared = agent._anthropic_client.timeout
|
||||
first = agent._create_request_anthropic_client(reason="test").timeout
|
||||
agent.model = other_claude # e.g. a fallback onto another Bedrock Claude in the same region
|
||||
second = agent._create_request_anthropic_client(reason="test").timeout
|
||||
|
||||
assert (shared.read, first.read, second.read) == (300.0, 300.0, 600.0)
|
||||
assert first.connect == second.connect == 10.0
|
||||
@@ -323,7 +323,7 @@ def test_bedrock_reasoning_delta_reaches_plugin_only_observer(monkeypatch):
|
||||
monkeypatch.setattr("hermes_cli.config.cfg_get", lambda *_args, **_kwargs: True)
|
||||
monkeypatch.setattr(
|
||||
"agent.bedrock_adapter._get_bedrock_runtime_client",
|
||||
lambda _region: SimpleNamespace(converse_stream=lambda **_kwargs: {"stream": []}),
|
||||
lambda *_args: SimpleNamespace(converse_stream=lambda **_kwargs: {"stream": []}),
|
||||
)
|
||||
monkeypatch.setattr("agent.bedrock_adapter.is_stale_connection_error", lambda _exc: False)
|
||||
monkeypatch.setattr("agent.bedrock_adapter.is_streaming_access_denied_error", lambda _exc: False)
|
||||
|
||||
@@ -167,7 +167,7 @@ You can set `providers.<id>.request_timeout_seconds` for a provider-wide request
|
||||
|
||||
You can also set `providers.<id>.stale_timeout_seconds` for the non-streaming stale-call detector, plus `providers.<id>.models.<model>.stale_timeout_seconds` for a model-specific override. This wins over the legacy `HERMES_API_CALL_STALE_TIMEOUT` env var. The same key is the streaming stale-stream deadline: an explicit value is used as-is — the implicit context-size tiers (240s above 50k tokens, 300s above 100k) and the reasoning-model floors apply only to the 180s default, so an explicit value can shorten how long a hung stream is tolerated.
|
||||
|
||||
Leaving these unset keeps the legacy defaults (`HERMES_API_TIMEOUT=1800`s, `HERMES_API_CALL_STALE_TIMEOUT=90`s, native Anthropic 900s). The non-streaming stale detector is auto-disabled for local endpoints when left implicit and can scale upward for very large contexts. Not currently wired for AWS Bedrock (both `bedrock_converse` and AnthropicBedrock SDK paths use boto3 with its own timeout configuration). See the commented example in [`cli-config.yaml.example`](https://github.com/NousResearch/hermes-agent/blob/main/cli-config.yaml.example).
|
||||
Leaving these unset keeps the legacy defaults (`HERMES_API_TIMEOUT=1800`s, `HERMES_API_CALL_STALE_TIMEOUT=90`s, native Anthropic 900s). The non-streaming stale detector is auto-disabled for local endpoints when left implicit and can scale upward for very large contexts. On AWS Bedrock (`providers.bedrock`), the resolved request timeout is the socket read timeout of the boto3 Converse client (`bedrock_converse`) and of the AnthropicBedrock SDK client, picked per request for the active model so a `/model` switch or fallback uses that model's value. The connect timeout stays fixed (botocore's 60s on Converse, 10s on AnthropicBedrock), and a read timeout bounds each socket read, not the whole call. Unset keeps botocore's 60s read on Converse and 900s on AnthropicBedrock. See the commented example in [`cli-config.yaml.example`](https://github.com/NousResearch/hermes-agent/blob/main/cli-config.yaml.example).
|
||||
|
||||
## Update Behavior
|
||||
|
||||
|
||||
Reference in new issue
Block a user