mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-10 22:41:41 +00:00
fix(router): honor litellm_settings.request_timeout as an independent per-attempt timeout (#31119)
request_timeout was shadowed by router_settings.timeout: Router stored a single slot via `self.timeout = timeout or litellm.request_timeout`, so when a router timeout was set the configured request_timeout was never used. Provider calls with no per-model timeout (Bedrock especially) then fell back to the hardcoded 600s httpx client default. Mirrors PR #25701 and completes it on top of the CompletionTimeout work already on this branch. - Router: add an independent self.request_timeout and prefer it over router_settings.timeout in both _get_non_stream_timeout and _get_stream_timeout - http_handler: cached default clients now fall back to request_timeout instead of a hardcoded 600s - Replace the brittle `== 6000` default-detection heuristic with a single get_configured_request_timeout() resolver backed by an explicit request_timeout_explicitly_set sentinel (set from REQUEST_TIMEOUT env and litellm_settings), keeping the value-differs fallback for SDK assignment. This also fixes an explicit request_timeout of 6000 being coerced to 600 - CompletionTimeout no longer second-guesses the package default; the caller passes the explicitly-configured value or None Regression for LIT-2369.
This commit is contained in:
parent
0a17c7c39f
commit
f12c9bec48
12 changed files with 292 additions and 22 deletions
|
|
@ -80,6 +80,7 @@ from litellm.constants import (
|
|||
WANDB_MODELS,
|
||||
REPEATED_STREAMING_CHUNK_LIMIT,
|
||||
request_timeout,
|
||||
request_timeout_explicitly_set as request_timeout_explicitly_set,
|
||||
open_ai_embedding_models,
|
||||
cohere_embedding_models,
|
||||
bedrock_embedding_models,
|
||||
|
|
|
|||
|
|
@ -468,6 +468,7 @@ HTTP_HANDLER_CONNECT_TIMEOUT_SECONDS: float = 5.0
|
|||
request_timeout: float = float(
|
||||
os.getenv("REQUEST_TIMEOUT", str(int(DEFAULT_REQUEST_TIMEOUT_SECONDS)))
|
||||
)
|
||||
request_timeout_explicitly_set: bool = "REQUEST_TIMEOUT" in os.environ
|
||||
DEFAULT_A2A_AGENT_TIMEOUT: float = float(
|
||||
os.getenv("DEFAULT_A2A_AGENT_TIMEOUT", 6000)
|
||||
) # 10 minutes
|
||||
|
|
|
|||
|
|
@ -6,10 +6,7 @@ from typing import Callable, Optional, Union
|
|||
|
||||
import httpx
|
||||
|
||||
from litellm.constants import (
|
||||
COMPLETION_HTTP_FALLBACK_SECONDS,
|
||||
DEFAULT_REQUEST_TIMEOUT_SECONDS,
|
||||
)
|
||||
from litellm.constants import COMPLETION_HTTP_FALLBACK_SECONDS
|
||||
|
||||
|
||||
class CompletionTimeout:
|
||||
|
|
@ -22,17 +19,13 @@ class CompletionTimeout:
|
|||
"""
|
||||
Used when ``model_timeout`` and kwargs timeouts are all unset.
|
||||
|
||||
``global_timeout`` is :attr:`litellm.request_timeout` (numeric / string), not
|
||||
:class:`httpx.Timeout`.
|
||||
|
||||
If it equals :data:`~litellm.constants.DEFAULT_REQUEST_TIMEOUT_SECONDS` (6000),
|
||||
return :data:`~litellm.constants.COMPLETION_HTTP_FALLBACK_SECONDS`. Same if
|
||||
``None``. Otherwise return ``float(global_timeout)``.
|
||||
``global_timeout`` is the explicitly-configured ``litellm.request_timeout``
|
||||
(numeric / string) or ``None`` when it was never set. ``None`` falls back to
|
||||
:data:`~litellm.constants.COMPLETION_HTTP_FALLBACK_SECONDS`; any explicit value
|
||||
(including ``6000``) is honored.
|
||||
"""
|
||||
if global_timeout is None:
|
||||
return COMPLETION_HTTP_FALLBACK_SECONDS
|
||||
if float(global_timeout) == float(DEFAULT_REQUEST_TIMEOUT_SECONDS):
|
||||
return COMPLETION_HTTP_FALLBACK_SECONDS
|
||||
return float(global_timeout)
|
||||
|
||||
@staticmethod
|
||||
|
|
@ -50,11 +43,10 @@ class CompletionTimeout:
|
|||
1. ``model_timeout`` (call argument / merged ``litellm_params``)
|
||||
2. ``kwargs["timeout"]``
|
||||
3. ``kwargs["request_timeout"]``
|
||||
4. Fallback from ``global_timeout`` (:attr:`litellm.request_timeout`) — if it is
|
||||
the package default (6000), use 600 instead.
|
||||
4. ``global_timeout`` (the explicitly-configured ``litellm.request_timeout``),
|
||||
or 600 when nothing was configured.
|
||||
|
||||
Coerce :class:`httpx.Timeout` when the provider does not support it.
|
||||
Explicit ``6000`` on the model or in kwargs is kept as ``6000``.
|
||||
"""
|
||||
resolved: Union[float, str, httpx.Timeout]
|
||||
if model_timeout is not None:
|
||||
|
|
|
|||
29
litellm/litellm_core_utils/request_timeout_resolver.py
Normal file
29
litellm/litellm_core_utils/request_timeout_resolver.py
Normal file
|
|
@ -0,0 +1,29 @@
|
|||
"""Single source of truth for whether ``litellm.request_timeout`` was configured.
|
||||
|
||||
``litellm.request_timeout`` always holds a value (the package default,
|
||||
:data:`~litellm.constants.DEFAULT_REQUEST_TIMEOUT_SECONDS`), so a bare read can't
|
||||
tell "user asked for this" from "nobody set it". This resolver answers that:
|
||||
|
||||
* ``request_timeout_explicitly_set`` is the authoritative signal, set when the
|
||||
value comes from the ``REQUEST_TIMEOUT`` env var or ``litellm_settings``.
|
||||
* A runtime value that differs from the package default (e.g. ``litellm.request_timeout
|
||||
= 300`` in SDK code) is also treated as explicit, for backwards compatibility.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import Optional
|
||||
|
||||
from litellm.constants import DEFAULT_REQUEST_TIMEOUT_SECONDS
|
||||
|
||||
|
||||
def get_configured_request_timeout() -> Optional[float]:
|
||||
"""Return the explicitly-configured ``litellm.request_timeout``, else ``None``."""
|
||||
import litellm
|
||||
|
||||
timeout = float(litellm.request_timeout)
|
||||
if litellm.request_timeout_explicitly_set:
|
||||
return timeout
|
||||
if timeout != float(DEFAULT_REQUEST_TIMEOUT_SECONDS):
|
||||
return timeout
|
||||
return None
|
||||
|
|
@ -42,6 +42,9 @@ from litellm.constants import (
|
|||
HTTP_HANDLER_CONNECT_TIMEOUT_SECONDS,
|
||||
)
|
||||
from litellm.litellm_core_utils.logging_utils import track_llm_api_timing
|
||||
from litellm.litellm_core_utils.request_timeout_resolver import (
|
||||
get_configured_request_timeout,
|
||||
)
|
||||
from litellm.types.llms.custom_http import *
|
||||
|
||||
if TYPE_CHECKING:
|
||||
|
|
@ -134,6 +137,18 @@ _DEFAULT_TIMEOUT = httpx.Timeout(
|
|||
timeout=COMPLETION_HTTP_FALLBACK_SECONDS,
|
||||
connect=HTTP_HANDLER_CONNECT_TIMEOUT_SECONDS,
|
||||
)
|
||||
|
||||
|
||||
def _default_cached_client_timeout() -> httpx.Timeout:
|
||||
"""Timeout for cached default httpx clients; honors an explicit litellm.request_timeout."""
|
||||
configured = get_configured_request_timeout()
|
||||
if configured is None:
|
||||
return _DEFAULT_TIMEOUT
|
||||
return httpx.Timeout(
|
||||
timeout=configured, connect=HTTP_HANDLER_CONNECT_TIMEOUT_SECONDS
|
||||
)
|
||||
|
||||
|
||||
_STREAMING_ERROR_BODY_READ_TIMEOUT_SECONDS = 5.0
|
||||
_STREAMING_ERROR_BODY_READ_EXECUTOR = concurrent.futures.ThreadPoolExecutor(
|
||||
max_workers=50,
|
||||
|
|
@ -1379,7 +1394,7 @@ def get_async_httpx_client(
|
|||
_new_client = AsyncHTTPHandler(**handler_params)
|
||||
else:
|
||||
_new_client = AsyncHTTPHandler(
|
||||
timeout=_DEFAULT_TIMEOUT,
|
||||
timeout=_default_cached_client_timeout(),
|
||||
shared_session=shared_session,
|
||||
)
|
||||
|
||||
|
|
@ -1428,7 +1443,7 @@ def _get_httpx_client(params: Optional[dict] = None) -> HTTPHandler:
|
|||
}
|
||||
_new_client = HTTPHandler(**handler_params)
|
||||
else:
|
||||
_new_client = HTTPHandler(timeout=_DEFAULT_TIMEOUT)
|
||||
_new_client = HTTPHandler(timeout=_default_cached_client_timeout())
|
||||
|
||||
cache.set_cache(
|
||||
key=_cache_key_name,
|
||||
|
|
|
|||
|
|
@ -89,6 +89,9 @@ from litellm.litellm_core_utils.audio_utils.utils import (
|
|||
get_audio_file_for_health_check,
|
||||
)
|
||||
from litellm.litellm_core_utils.completion_timeout import CompletionTimeout
|
||||
from litellm.litellm_core_utils.request_timeout_resolver import (
|
||||
get_configured_request_timeout,
|
||||
)
|
||||
from litellm.litellm_core_utils.get_litellm_params import OPTIONAL_KWARGS_KEYS
|
||||
from litellm.litellm_core_utils.dd_tracing import tracer
|
||||
from litellm.litellm_core_utils.get_provider_specific_headers import (
|
||||
|
|
@ -5315,7 +5318,7 @@ def completion( # type: ignore
|
|||
timeout,
|
||||
kwargs,
|
||||
custom_llm_provider,
|
||||
global_timeout=getattr(litellm, "request_timeout", None),
|
||||
global_timeout=get_configured_request_timeout(),
|
||||
supports_httpx_timeout=supports_httpx_timeout,
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -4463,6 +4463,8 @@ class ProxyConfig:
|
|||
f"{blue_color_code} setting litellm.{key}={value}{reset_color_code}"
|
||||
)
|
||||
setattr(litellm, key, value)
|
||||
if key == "request_timeout":
|
||||
litellm.request_timeout_explicitly_set = True
|
||||
if key in {"s3_audit_callback_params", "s3_callback_params"}:
|
||||
from litellm.integrations.s3_v2 import S3Logger as S3V2Logger
|
||||
from litellm.litellm_core_utils.litellm_logging import (
|
||||
|
|
|
|||
|
|
@ -61,6 +61,9 @@ from litellm.constants import (
|
|||
)
|
||||
from litellm.integrations.custom_logger import CustomLogger
|
||||
from litellm.litellm_core_utils.asyncify import run_async_function
|
||||
from litellm.litellm_core_utils.request_timeout_resolver import (
|
||||
get_configured_request_timeout,
|
||||
)
|
||||
from litellm.litellm_core_utils.core_helpers import (
|
||||
_get_parent_otel_span_from_kwargs,
|
||||
get_metadata_variable_name_from_kwargs,
|
||||
|
|
@ -565,6 +568,12 @@ class Router:
|
|||
|
||||
self._explicit_timeout = timeout # None when user did not pass timeout
|
||||
self.timeout = timeout or litellm.request_timeout
|
||||
# Per-attempt request_timeout, independent of router_settings.timeout.
|
||||
# Only stored when a router timeout is also set, since otherwise
|
||||
# request_timeout already flows through self.timeout above.
|
||||
self.request_timeout = (
|
||||
get_configured_request_timeout() if timeout is not None else None
|
||||
)
|
||||
self.stream_timeout = stream_timeout
|
||||
|
||||
self.retry_after = retry_after
|
||||
|
|
@ -3391,6 +3400,7 @@ class Router:
|
|||
"stream_timeout", None
|
||||
) # timeout set on litellm_params for this deployment
|
||||
or self.stream_timeout # timeout set on router
|
||||
or self.request_timeout # litellm_settings.request_timeout (per-attempt)
|
||||
or self.default_litellm_params.get("stream_timeout", None)
|
||||
)
|
||||
|
||||
|
|
@ -3407,7 +3417,8 @@ class Router:
|
|||
or data.get(
|
||||
"request_timeout", None
|
||||
) # timeout set on litellm_params for this deployment
|
||||
or self.timeout # timeout set on router
|
||||
or self.request_timeout # litellm_settings.request_timeout (per-attempt)
|
||||
or self.timeout # timeout set on router (router_settings.timeout)
|
||||
or self.default_litellm_params.get("timeout", None)
|
||||
)
|
||||
return timeout
|
||||
|
|
|
|||
|
|
@ -0,0 +1,58 @@
|
|||
"""Unit tests for litellm.litellm_core_utils.request_timeout_resolver.
|
||||
|
||||
The resolver decides whether ``litellm.request_timeout`` was *explicitly configured*
|
||||
(env REQUEST_TIMEOUT / litellm_settings, or a non-default runtime value) versus left
|
||||
at the package default. This is what lets request_timeout act as an independent
|
||||
per-attempt timeout instead of being indistinguishable from "nobody set it".
|
||||
"""
|
||||
|
||||
import os
|
||||
import sys
|
||||
|
||||
import pytest
|
||||
|
||||
sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), "../../..")))
|
||||
|
||||
import litellm
|
||||
from litellm.constants import DEFAULT_REQUEST_TIMEOUT_SECONDS
|
||||
from litellm.litellm_core_utils.request_timeout_resolver import (
|
||||
get_configured_request_timeout,
|
||||
)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def restore_request_timeout():
|
||||
original_value = litellm.request_timeout
|
||||
original_flag = litellm.request_timeout_explicitly_set
|
||||
try:
|
||||
yield
|
||||
finally:
|
||||
litellm.request_timeout = original_value
|
||||
litellm.request_timeout_explicitly_set = original_flag
|
||||
|
||||
|
||||
def test_default_value_without_flag_is_unset(restore_request_timeout):
|
||||
litellm.request_timeout = DEFAULT_REQUEST_TIMEOUT_SECONDS
|
||||
litellm.request_timeout_explicitly_set = False
|
||||
assert get_configured_request_timeout() is None
|
||||
|
||||
|
||||
def test_explicit_flag_returns_value(restore_request_timeout):
|
||||
litellm.request_timeout = 300
|
||||
litellm.request_timeout_explicitly_set = True
|
||||
assert get_configured_request_timeout() == 300.0
|
||||
|
||||
|
||||
def test_explicit_flag_preserves_value_equal_to_default(restore_request_timeout):
|
||||
# The case the bare ``!= default`` heuristic gets wrong: a user who explicitly
|
||||
# configures the default value still means it explicitly.
|
||||
litellm.request_timeout = DEFAULT_REQUEST_TIMEOUT_SECONDS
|
||||
litellm.request_timeout_explicitly_set = True
|
||||
assert get_configured_request_timeout() == float(DEFAULT_REQUEST_TIMEOUT_SECONDS)
|
||||
|
||||
|
||||
def test_non_default_runtime_value_treated_as_explicit(restore_request_timeout):
|
||||
# SDK users assigning litellm.request_timeout directly (no flag) must keep working.
|
||||
litellm.request_timeout = 300
|
||||
litellm.request_timeout_explicitly_set = False
|
||||
assert get_configured_request_timeout() == 300.0
|
||||
|
|
@ -851,3 +851,56 @@ async def test_async_get_forwards_per_request_timeout():
|
|||
}
|
||||
finally:
|
||||
await handler.close()
|
||||
|
||||
|
||||
class TestDefaultCachedClientTimeoutHonorsRequestTimeout:
|
||||
"""Cached default httpx clients must fall back to an explicit litellm.request_timeout.
|
||||
|
||||
Regression for LIT-2369: get_async_httpx_client / _get_httpx_client hardcoded a
|
||||
600s default and never consulted litellm.request_timeout, so provider calls with
|
||||
no per-model timeout (e.g. Bedrock) hung for 600s.
|
||||
"""
|
||||
|
||||
@pytest.fixture
|
||||
def restore_request_timeout(self):
|
||||
original_value = litellm.request_timeout
|
||||
original_flag = litellm.request_timeout_explicitly_set
|
||||
try:
|
||||
yield
|
||||
finally:
|
||||
litellm.request_timeout = original_value
|
||||
litellm.request_timeout_explicitly_set = original_flag
|
||||
|
||||
def test_default_when_request_timeout_unset(self, restore_request_timeout):
|
||||
from litellm.llms.custom_httpx.http_handler import (
|
||||
_DEFAULT_TIMEOUT,
|
||||
_default_cached_client_timeout,
|
||||
)
|
||||
|
||||
litellm.request_timeout = litellm.constants.DEFAULT_REQUEST_TIMEOUT_SECONDS
|
||||
litellm.request_timeout_explicitly_set = False
|
||||
assert _default_cached_client_timeout() is _DEFAULT_TIMEOUT
|
||||
|
||||
def test_uses_explicit_request_timeout(self, restore_request_timeout):
|
||||
from litellm.llms.custom_httpx.http_handler import (
|
||||
_default_cached_client_timeout,
|
||||
)
|
||||
|
||||
litellm.request_timeout = 300
|
||||
litellm.request_timeout_explicitly_set = True
|
||||
resolved = _default_cached_client_timeout()
|
||||
assert resolved.read == 300.0
|
||||
assert resolved.connect == 5.0
|
||||
|
||||
def test_cached_async_client_built_with_explicit_request_timeout(
|
||||
self, restore_request_timeout
|
||||
):
|
||||
from litellm.caching.llm_caching_handler import LLMClientCache
|
||||
from litellm.llms.custom_httpx.http_handler import get_async_httpx_client
|
||||
from litellm.types.utils import LlmProviders
|
||||
|
||||
litellm.request_timeout = 300
|
||||
litellm.request_timeout_explicitly_set = True
|
||||
litellm.in_memory_llm_clients_cache = LLMClientCache()
|
||||
client = get_async_httpx_client(llm_provider=LlmProviders.BEDROCK)
|
||||
assert client.timeout.read == 300.0
|
||||
|
|
|
|||
|
|
@ -63,8 +63,9 @@ def test_global_timeout_from_litellm_settings():
|
|||
)
|
||||
|
||||
|
||||
def test_global_timeout_package_default_coerced_to_600_for_completion():
|
||||
"""Package default 6000s → 600s for completion-only path."""
|
||||
def test_explicit_global_timeout_6000_is_preserved():
|
||||
"""The caller passes the explicitly-configured value (or None); an explicit
|
||||
6000 must be honored, not silently coerced to 600."""
|
||||
assert (
|
||||
CompletionTimeout.resolve(
|
||||
None,
|
||||
|
|
@ -73,7 +74,7 @@ def test_global_timeout_package_default_coerced_to_600_for_completion():
|
|||
global_timeout=6000.0,
|
||||
supports_httpx_timeout=supports_httpx_timeout,
|
||||
)
|
||||
== 600.0
|
||||
== 6000.0
|
||||
)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -4901,3 +4901,107 @@ def test_is_deployment_blocked_static_helper_reflects_blocked_flag():
|
|||
)
|
||||
is True
|
||||
)
|
||||
|
||||
|
||||
class TestRouterRequestTimeoutPropagation:
|
||||
"""litellm_settings.request_timeout must act as an independent per-attempt timeout.
|
||||
|
||||
Regression for LIT-2369: request_timeout was shadowed by router_settings.timeout,
|
||||
so Bedrock (and other provider) calls fell back to the hardcoded 600s httpx
|
||||
default instead of the configured value.
|
||||
"""
|
||||
|
||||
def _make_router(self, timeout=None, stream_timeout=None):
|
||||
return litellm.Router(
|
||||
model_list=[
|
||||
{
|
||||
"model_name": "test-model",
|
||||
"litellm_params": {
|
||||
"model": "openai/gpt-4",
|
||||
"api_key": "sk-test",
|
||||
},
|
||||
}
|
||||
],
|
||||
timeout=timeout,
|
||||
stream_timeout=stream_timeout,
|
||||
)
|
||||
|
||||
@pytest.fixture
|
||||
def explicit_request_timeout(self):
|
||||
original_value = litellm.request_timeout
|
||||
original_flag = litellm.request_timeout_explicitly_set
|
||||
litellm.request_timeout = 300
|
||||
litellm.request_timeout_explicitly_set = True
|
||||
try:
|
||||
yield 300
|
||||
finally:
|
||||
litellm.request_timeout = original_value
|
||||
litellm.request_timeout_explicitly_set = original_flag
|
||||
|
||||
def test_request_timeout_stored_independently_when_both_set(
|
||||
self, explicit_request_timeout
|
||||
):
|
||||
router = self._make_router(timeout=330)
|
||||
assert router.timeout == 330
|
||||
assert router.request_timeout == 300
|
||||
|
||||
def test_request_timeout_none_when_not_explicitly_configured(self):
|
||||
original_value = litellm.request_timeout
|
||||
original_flag = litellm.request_timeout_explicitly_set
|
||||
litellm.request_timeout = litellm.constants.DEFAULT_REQUEST_TIMEOUT_SECONDS
|
||||
litellm.request_timeout_explicitly_set = False
|
||||
try:
|
||||
router = self._make_router(timeout=330)
|
||||
assert router.timeout == 330
|
||||
assert router.request_timeout is None
|
||||
finally:
|
||||
litellm.request_timeout = original_value
|
||||
litellm.request_timeout_explicitly_set = original_flag
|
||||
|
||||
def test_non_stream_prefers_request_timeout_over_router_timeout(
|
||||
self, explicit_request_timeout
|
||||
):
|
||||
router = self._make_router(timeout=330)
|
||||
assert router._get_non_stream_timeout(kwargs={}, data={}) == 300
|
||||
|
||||
def test_stream_prefers_request_timeout_over_router_timeout(
|
||||
self, explicit_request_timeout
|
||||
):
|
||||
router = self._make_router(timeout=330)
|
||||
# stream=True resolves through _get_stream_timeout; request_timeout must win.
|
||||
assert router._get_timeout(kwargs={"stream": True}, data={}) == 300
|
||||
|
||||
def test_explicit_stream_timeout_still_wins_over_request_timeout(
|
||||
self, explicit_request_timeout
|
||||
):
|
||||
router = self._make_router(timeout=330, stream_timeout=45)
|
||||
assert router._get_stream_timeout(kwargs={}, data={}) == 45
|
||||
|
||||
def test_non_stream_falls_through_to_router_timeout_without_request_timeout(self):
|
||||
original_value = litellm.request_timeout
|
||||
original_flag = litellm.request_timeout_explicitly_set
|
||||
litellm.request_timeout = litellm.constants.DEFAULT_REQUEST_TIMEOUT_SECONDS
|
||||
litellm.request_timeout_explicitly_set = False
|
||||
try:
|
||||
router = self._make_router(timeout=330)
|
||||
assert router._get_non_stream_timeout(kwargs={}, data={}) == 330
|
||||
finally:
|
||||
litellm.request_timeout = original_value
|
||||
litellm.request_timeout_explicitly_set = original_flag
|
||||
|
||||
def test_per_deployment_timeout_overrides_request_timeout(
|
||||
self, explicit_request_timeout
|
||||
):
|
||||
router = self._make_router(timeout=330)
|
||||
assert router._get_non_stream_timeout(kwargs={}, data={"timeout": 120}) == 120
|
||||
|
||||
def test_per_request_timeout_overrides_request_timeout(
|
||||
self, explicit_request_timeout
|
||||
):
|
||||
router = self._make_router(timeout=330)
|
||||
assert (
|
||||
router._get_non_stream_timeout(
|
||||
kwargs={"timeout": 60}, data={"timeout": 120}
|
||||
)
|
||||
== 60
|
||||
)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue