From db9eda19b4bb7a8c30cd93a037b45efd0b83b6d3 Mon Sep 17 00:00:00 2001 From: ryan-crabbe-berri Date: Mon, 26 Oct 2026 16:04:33 -0700 Subject: [PATCH] fix(http_handler): name the timeout phase and limit that fired instead of "after None seconds" (#48250) * fix(http_handler): name the timeout phase and the limit that fired instead of 'after None seconds' Every HTTP verb on both handlers raised litellm.Timeout with the per-call timeout argument interpolated into the message, which is None whenever the caller relied on the client default, and always said "Connection" even for a read timeout. One helper now reads the limit httpx actually enforced from the failed request and names the phase that fired Fixes #14635 * fix(rust_bridge): name the timeout phase in the vault timeout message to match the httpx handler * fix(http_handler): drop the unreachable timeout fallbacks httpx already rules out * refactor(rust_bridge): drop a doc comment that restated the timeout helper --- .../src/exception_mapping_utils/mod.rs | 65 +++++------ .../src/exception_mapping_utils/original.rs | 14 +++ .../python-bridge/src/secrets/provider.rs | 19 +-- .../crates/python-bridge/src/secrets/vault.rs | 18 ++- litellm/llms/custom_httpx/http_handler.py | 108 ++++++++---------- .../llms/custom_httpx/test_http_handler.py | 83 ++++++++++++++ 6 files changed, 195 insertions(+), 112 deletions(-) diff --git a/litellm-rust/crates/core-utils/src/exception_mapping_utils/mod.rs b/litellm-rust/crates/core-utils/src/exception_mapping_utils/mod.rs index 162d325e4f4..8b63fadf5a6 100644 --- a/litellm-rust/crates/core-utils/src/exception_mapping_utils/mod.rs +++ b/litellm-rust/crates/core-utils/src/exception_mapping_utils/mod.rs @@ -51,7 +51,7 @@ mod rules; mod status; mod vertex_ai; -pub use original::{ExceptionFamily, OriginalException}; +pub use original::{ExceptionFamily, OriginalException, TimeoutPhase}; pub use public::{MappedFailure, PublicError, UpstreamResponse}; use rules::{Rule, contains_any, first_match}; @@ -67,7 +67,6 @@ const TIMEOUT_MARKERS: &[&str] = &[ pub struct ExceptionContext { pub model: String, pub custom_llm_provider: String, - pub asynchronous: bool, pub vertex_project: Option, pub vertex_location: Option, pub model_group: Option, @@ -105,11 +104,12 @@ pub fn exception_type( (None, message.clone(), None) } OriginalException::Timeout { + phase, timeout_seconds, elapsed_seconds, } => ( None, - timeout_message(context.asynchronous, *timeout_seconds, *elapsed_seconds), + timeout_message(*phase, *timeout_seconds, *elapsed_seconds), None, ), }; @@ -161,21 +161,16 @@ fn family_rules(family: ExceptionFamily) -> &'static [Rule] { } } -/// The text the Python HTTP handler's timeout carries: the sync and async handlers word it -/// differently. -fn timeout_message( - asynchronous: bool, +/// The text the Python HTTP handler's timeout carries: the phase that expired and the limit +/// httpx enforced for it, both rendered as Python would print them. +pub fn timeout_message( + phase: TimeoutPhase, timeout_seconds: Option, elapsed_seconds: Option, ) -> String { let timeout = python_float(timeout_seconds); - if asynchronous { - let elapsed = - python_float(elapsed_seconds.map(|seconds| (seconds * 1000.0).round() / 1000.0)); - format!("Connection timed out. Timeout passed={timeout}, time taken={elapsed} seconds") - } else { - format!("Connection timed out after {timeout} seconds.") - } + let elapsed = python_float(elapsed_seconds.map(|seconds| (seconds * 1000.0).round() / 1000.0)); + format!("{phase} timed out. Timeout passed={timeout}, time taken={elapsed} seconds") } fn python_float(value: Option) -> String { @@ -356,6 +351,7 @@ mod tests { #[test] fn a_handler_timeout_is_a_408_without_a_response() { let original = OriginalException::Timeout { + phase: TimeoutPhase::Read, timeout_seconds: Some(0.5), elapsed_seconds: Some(0.5031), }; @@ -363,7 +359,7 @@ mod tests { mapped("mistral", &original), MappedFailure { error: PublicError::Timeout { status: 408 }, - message: "MistralException - Connection timed out after 0.5 seconds.".into(), + message: "MistralException - Read timed out. Timeout passed=0.5, time taken=0.503 seconds".into(), upstream: None, debug_info: DEBUG.into(), } @@ -440,39 +436,44 @@ mod tests { } #[rstest::rstest] - #[case::sync( - false, + #[case::connect(TimeoutPhase::Connect, "Connection timed out.")] + #[case::read(TimeoutPhase::Read, "Read timed out.")] + #[case::write(TimeoutPhase::Write, "Write timed out.")] + #[case::pool(TimeoutPhase::Pool, "Connection pool timed out.")] + fn timeout_text_names_the_phase_that_expired( + #[case] phase: TimeoutPhase, + #[case] expected: &str, + ) { + assert_eq!( + timeout_message(phase, Some(0.5), Some(0.5031)), + format!("{expected} Timeout passed=0.5, time taken=0.503 seconds") + ); + } + + #[rstest::rstest] + #[case::rounds_the_elapsed_time( Some(0.5), Some(0.5031), - "Connection timed out after 0.5 seconds." - )] - #[case::async_rounds_the_elapsed_time( - true, - Some(0.5), - Some(0.5031), - "Connection timed out. Timeout passed=0.5, time taken=0.503 seconds" + "Timeout passed=0.5, time taken=0.503 seconds" )] #[case::whole_seconds_keep_a_decimal( - true, Some(600.0), Some(2.0), - "Connection timed out. Timeout passed=600.0, time taken=2.0 seconds" + "Timeout passed=600.0, time taken=2.0 seconds" )] #[case::unknown_values_render_as_none( - true, None, None, - "Connection timed out. Timeout passed=None, time taken=None seconds" + "Timeout passed=None, time taken=None seconds" )] - fn timeout_text_follows_the_delivery_mode( - #[case] asynchronous: bool, + fn timeout_text_renders_durations_as_python_floats( #[case] timeout_seconds: Option, #[case] elapsed_seconds: Option, #[case] expected: &str, ) { assert_eq!( - timeout_message(asynchronous, timeout_seconds, elapsed_seconds), - expected + timeout_message(TimeoutPhase::Read, timeout_seconds, elapsed_seconds), + format!("Read timed out. {expected}") ); } diff --git a/litellm-rust/crates/core-utils/src/exception_mapping_utils/original.rs b/litellm-rust/crates/core-utils/src/exception_mapping_utils/original.rs index 4de67e6e7b5..6517b093bbd 100644 --- a/litellm-rust/crates/core-utils/src/exception_mapping_utils/original.rs +++ b/litellm-rust/crates/core-utils/src/exception_mapping_utils/original.rs @@ -10,6 +10,7 @@ pub enum OriginalException { message: String, }, Timeout { + phase: TimeoutPhase, timeout_seconds: Option, elapsed_seconds: Option, }, @@ -20,6 +21,19 @@ pub enum OriginalException { }, } +/// The httpx timeout that expired, spelled the way the Python HTTP handler names it. +#[derive(Clone, Copy, Debug, PartialEq, Eq, strum::Display, strum::VariantArray)] +pub enum TimeoutPhase { + #[strum(serialize = "Connection")] + Connect, + #[strum(serialize = "Read")] + Read, + #[strum(serialize = "Write")] + Write, + #[strum(serialize = "Connection pool")] + Pool, +} + /// Which provider-specific text rules apply before the shared status table. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum ExceptionFamily { diff --git a/litellm-rust/crates/python-bridge/src/secrets/provider.rs b/litellm-rust/crates/python-bridge/src/secrets/provider.rs index 568a0cd2228..5f4cd193fcf 100644 --- a/litellm-rust/crates/python-bridge/src/secrets/provider.rs +++ b/litellm-rust/crates/python-bridge/src/secrets/provider.rs @@ -88,20 +88,23 @@ fn aws_context( } fn read_timeout(value: Option<&Bound<'_, PyAny>>) -> PyResult> { - let Some(value) = value.filter(|value| !value.is_none()) else { - return Ok(None); - }; - let seconds = match value.extract::() { - Ok(value) => Some(value), - Err(_) => value.getattr("read")?.extract::>()?, - }; - seconds + read_timeout_seconds(value)? .map(|value| { Duration::try_from_secs_f64(value).map_err(|_| PyValueError::new_err("invalid timeout")) }) .transpose() } +pub(super) fn read_timeout_seconds(value: Option<&Bound<'_, PyAny>>) -> PyResult> { + let Some(value) = value.filter(|value| !value.is_none()) else { + return Ok(None); + }; + match value.extract::() { + Ok(value) => Ok(Some(value)), + Err(_) => value.getattr("read")?.extract::>(), + } +} + fn vault_context(params: Option<&Bound<'_, PyAny>>) -> PyResult { let params = params.and_then(|value| value.cast::().ok()); let nested = params diff --git a/litellm-rust/crates/python-bridge/src/secrets/vault.rs b/litellm-rust/crates/python-bridge/src/secrets/vault.rs index 46b29839fe4..79281662848 100644 --- a/litellm-rust/crates/python-bridge/src/secrets/vault.rs +++ b/litellm-rust/crates/python-bridge/src/secrets/vault.rs @@ -2,10 +2,12 @@ mod operation; pub(super) use operation::{Failure, FailureKind, FailureStage, delete, rotate, write}; +use litellm_core_utils::exception_mapping_utils::{TimeoutPhase, timeout_message}; use litellm_secrets::hashicorp::{Error, RawOperationError}; use pyo3::prelude::*; use super::mutation::{error_value, http_message, json_value}; +use super::provider::read_timeout_seconds; const HTTP_TRANSPORT_MODULE: &str = "litellm.rust_bridge.host.http_transport"; @@ -69,21 +71,17 @@ fn cause_message(py: Python<'_>, kind: FailureKind, context: &ErrorContext) -> P } FailureKind::Native(RawOperationError::Timeout { method, elapsed }) => { if method == "POST" { - let elapsed = py - .import("builtins")? - .call_method1("round", (elapsed.as_secs_f64(), 3))?; let kwargs = pyo3::types::PyDict::new(py); kwargs.set_item( "message", - format!( - "Connection timed out. Timeout passed={}, time taken={} seconds", - context.timeout.as_deref().unwrap_or("None"), - elapsed.str()? + timeout_message( + TimeoutPhase::Read, + context.timeout, + Some(elapsed.as_secs_f64()), ), )?; kwargs.set_item("model", "default-model-name")?; kwargs.set_item("llm_provider", "litellm-httpx-handler")?; - kwargs.set_item("headers", pyo3::types::PyDict::new(py))?; py.import("litellm.exceptions")? .getattr("Timeout")? .call((), Some(&kwargs))? @@ -159,7 +157,7 @@ fn response_text(py: Python<'_>, body: &[u8]) -> PyResult { #[derive(Default)] pub(super) struct ErrorContext { - timeout: Option, + timeout: Option, aiohttp: bool, } @@ -173,7 +171,7 @@ impl ErrorContext { return Ok(Self::default()); } Ok(Self { - timeout: timeout.map(|value| value.str()?.extract()).transpose()?, + timeout: read_timeout_seconds(timeout)?, aiohttp: py .import(HTTP_TRANSPORT_MODULE)? .getattr("uses_aiohttp_transport")? diff --git a/litellm/llms/custom_httpx/http_handler.py b/litellm/llms/custom_httpx/http_handler.py index cffeb44f12c..0b64885df45 100644 --- a/litellm/llms/custom_httpx/http_handler.py +++ b/litellm/llms/custom_httpx/http_handler.py @@ -20,6 +20,7 @@ from aiohttp import ClientSession, DummyCookieJar, TCPConnector from httpx import USE_CLIENT_DEFAULT, AsyncHTTPTransport, HTTPTransport from httpx._types import CertTypes, RequestFiles from httpx._utils import get_environment_proxies +from pydantic import TypeAdapter import litellm from litellm._logging import verbose_logger @@ -566,6 +567,34 @@ async def _raise_masked_async_error(e: httpx.HTTPStatusError, stream: bool) -> N raise MaskedHTTPStatusError(e, message=_text, text=_text) from None +_TIMEOUT_PHASES: Final[Mapping[type[httpx.TimeoutException], tuple[str, str]]] = MappingProxyType( + { + httpx.ConnectTimeout: ("Connection", "connect"), + httpx.ReadTimeout: ("Read", "read"), + httpx.WriteTimeout: ("Write", "write"), + httpx.PoolTimeout: ("Connection pool", "pool"), + } +) +_UNKNOWN_TIMEOUT_PHASE: Final = ("Request", "read") +_TIMEOUT_EXTENSION: Final = TypeAdapter(dict[str, float | None]) + + +def _timeout_phase_seconds(error: httpx.TimeoutException, phase: str) -> float | None: + """The limit httpx enforced for ``phase`` on the request that raised ``error``, from the request itself.""" + return _TIMEOUT_EXTENSION.validate_python(error.request.extensions["timeout"]).get(phase) + + +def _raise_timeout(error: httpx.TimeoutException, started_at: float) -> NoReturn: + label, phase = _TIMEOUT_PHASES.get(type(error), _UNKNOWN_TIMEOUT_PHASE) + seconds: Final = _timeout_phase_seconds(error, phase) + time_taken: Final = round(time.time() - started_at, 3) + raise litellm.Timeout( + message=f"{label} timed out. Timeout passed={seconds}, time taken={time_taken} seconds", + model="default-model-name", + llm_provider="litellm-httpx-handler", + ) + + class HTTPResponseLimitError(ValueError): pass @@ -870,20 +899,7 @@ class AsyncHTTPHandler: finally: await new_client.aclose() except httpx.TimeoutException as e: - end_time: Final = time.time() - time_delta: Final = round(end_time - start_time, 3) - headers = {} - error_response: Final[httpx.Response | None] = getattr(e, "response", None) - if error_response is not None: - for key, value in error_response.headers.items(): - headers[f"response_headers-{key}"] = value - - raise litellm.Timeout( - message=f"Connection timed out. Timeout passed={timeout}, time taken={time_delta} seconds", - model="default-model-name", - llm_provider="litellm-httpx-handler", - headers=headers, - ) + _raise_timeout(e, start_time) except httpx.HTTPStatusError as e: await _raise_masked_async_error(e, stream) except Exception as e: @@ -902,6 +918,7 @@ class AsyncHTTPHandler: follow_redirects: bool | None = None, ): _follow_redirects: Final = follow_redirects if follow_redirects is not None else USE_CLIENT_DEFAULT + start_time: Final = time.time() try: if timeout is None: timeout = self.timeout @@ -946,18 +963,7 @@ class AsyncHTTPHandler: finally: await new_client.aclose() except httpx.TimeoutException as e: - headers = {} - error_response: Final[httpx.Response | None] = getattr(e, "response", None) - if error_response is not None: - for key, value in error_response.headers.items(): - headers[f"response_headers-{key}"] = value - - raise litellm.Timeout( - message=f"Connection timed out after {timeout} seconds.", - model="default-model-name", - llm_provider="litellm-httpx-handler", - headers=headers, - ) + _raise_timeout(e, start_time) except httpx.HTTPStatusError as e: await _raise_masked_async_error(e, stream) except Exception as e: @@ -974,6 +980,7 @@ class AsyncHTTPHandler: stream: bool = False, content: _RequestContent | None = None, ): + start_time: Final = time.time() try: if timeout is None: timeout = self.timeout @@ -1012,18 +1019,7 @@ class AsyncHTTPHandler: finally: await new_client.aclose() except httpx.TimeoutException as e: - headers = {} - error_response: Final[httpx.Response | None] = getattr(e, "response", None) - if error_response is not None: - for key, value in error_response.headers.items(): - headers[f"response_headers-{key}"] = value - - raise litellm.Timeout( - message=f"Connection timed out after {timeout} seconds.", - model="default-model-name", - llm_provider="litellm-httpx-handler", - headers=headers, - ) + _raise_timeout(e, start_time) except httpx.HTTPStatusError as e: await _raise_masked_async_error(e, stream) except Exception as e: @@ -1514,6 +1510,7 @@ class HTTPHandler: content: _RequestContent | None = None, logging_obj: LiteLLMLoggingObject | None = None, ): + start_time: Final = time.time() try: # Prepare data/content parameters to prevent httpx DeprecationWarning (memory leak fix) request_data, request_content = _prepare_request_data_and_content(data, content) @@ -1546,12 +1543,8 @@ class HTTPHandler: _anchor_handler_to(response, self) response.raise_for_status() return response - except httpx.TimeoutException: - raise litellm.Timeout( - message=f"Connection timed out after {timeout} seconds.", - model="default-model-name", - llm_provider="litellm-httpx-handler", - ) + except httpx.TimeoutException as e: + _raise_timeout(e, start_time) except httpx.HTTPStatusError as e: _raise_masked_sync_error(e, stream) except Exception as e: @@ -1568,6 +1561,7 @@ class HTTPHandler: timeout: float | httpx.Timeout | None = None, content: _RequestContent | None = None, ): + start_time: Final = time.time() try: # Prepare data/content parameters to prevent httpx DeprecationWarning (memory leak fix) request_data, request_content = _prepare_request_data_and_content(data, content) @@ -1598,12 +1592,8 @@ class HTTPHandler: _anchor_handler_to(response, self) response.raise_for_status() return response - except httpx.TimeoutException: - raise litellm.Timeout( - message=f"Connection timed out after {timeout} seconds.", - model="default-model-name", - llm_provider="litellm-httpx-handler", - ) + except httpx.TimeoutException as e: + _raise_timeout(e, start_time) except httpx.HTTPStatusError as e: _raise_masked_sync_error(e, stream) except Exception as e: @@ -1620,6 +1610,7 @@ class HTTPHandler: timeout: float | httpx.Timeout | None = None, content: _RequestContent | None = None, ): + start_time: Final = time.time() try: # Prepare data/content parameters to prevent httpx DeprecationWarning (memory leak fix) request_data, request_content = _prepare_request_data_and_content(data, content) @@ -1649,12 +1640,8 @@ class HTTPHandler: if stream: _anchor_handler_to(response, self) return response - except httpx.TimeoutException: - raise litellm.Timeout( - message=f"Connection timed out after {timeout} seconds.", - model="default-model-name", - llm_provider="litellm-httpx-handler", - ) + except httpx.TimeoutException as e: + _raise_timeout(e, start_time) except httpx.HTTPStatusError as e: _raise_masked_sync_error(e, stream) except Exception as e: @@ -1671,6 +1658,7 @@ class HTTPHandler: stream: bool = False, content: _RequestContent | None = None, ): + start_time: Final = time.time() try: # Prepare data/content parameters to prevent httpx DeprecationWarning (memory leak fix) request_data, request_content = _prepare_request_data_and_content(data, content) @@ -1701,12 +1689,8 @@ class HTTPHandler: _anchor_handler_to(response, self) response.raise_for_status() return response - except httpx.TimeoutException: - raise litellm.Timeout( - message=f"Connection timed out after {timeout} seconds.", - model="default-model-name", - llm_provider="litellm-httpx-handler", - ) + except httpx.TimeoutException as e: + _raise_timeout(e, start_time) except httpx.HTTPStatusError as e: _raise_masked_sync_error(e, stream) except Exception as e: diff --git a/tests/unit/llms/custom_httpx/test_http_handler.py b/tests/unit/llms/custom_httpx/test_http_handler.py index bd4ed4c99cc..1607447f7bf 100644 --- a/tests/unit/llms/custom_httpx/test_http_handler.py +++ b/tests/unit/llms/custom_httpx/test_http_handler.py @@ -2172,3 +2172,86 @@ async def test_async_ollama_ssl_verify_false_builds_a_client_that_skips_certific assert route.call_count == 1 assert text == "Sunny" assert _cached_clients_verify_modes(client_cache, AsyncHTTPHandler) == (ssl.CERT_NONE,) + + +_TIMEOUT_CLIENT_LIMITS: Final = httpx.Timeout(42.0, connect=7.0) +_TIMEOUT_PHASES: Final = ( + (httpx.ConnectTimeout, "Connection timed out", "Timeout passed=7.0"), + (httpx.ReadTimeout, "Read timed out", "Timeout passed=42.0"), + (httpx.WriteTimeout, "Write timed out", "Timeout passed=42.0"), + (httpx.PoolTimeout, "Connection pool timed out", "Timeout passed=42.0"), +) + + +def _timing_out(kind: type[httpx.TimeoutException]) -> httpx.MockTransport: + def _raise(request: httpx.Request) -> httpx.Response: + raise kind("simulated", request=request) + + return httpx.MockTransport(_raise) + + +@pytest.mark.parametrize("verb", ["post", "put", "patch", "delete"]) +@pytest.mark.parametrize(("kind", "label", "limit"), _TIMEOUT_PHASES) +def test_sync_timeout_without_a_per_call_timeout_names_the_phase_and_the_limit_that_fired( + verb: str, kind: type[httpx.TimeoutException], label: str, limit: str +) -> None: + """Issue #14635: the handler used to print the caller's absent argument as 'after None seconds'.""" + handler: Final = HTTPHandler() + handler.client.close() + handler.client = httpx.Client(transport=_timing_out(kind), timeout=_TIMEOUT_CLIENT_LIMITS) + try: + with pytest.raises(LitellmTimeout) as raised: + getattr(handler, verb)("https://example.com/v1/embeddings", json={"input": "hi"}) + finally: + handler.close() + + message: Final = str(raised.value) + assert label in message, message + assert limit in message, message + assert "None" not in message, message + + +@pytest.mark.parametrize("verb", ["post", "put", "patch"]) +@pytest.mark.parametrize(("kind", "label", "limit"), _TIMEOUT_PHASES) +async def test_async_timeout_without_a_per_call_timeout_names_the_phase_and_the_limit_that_fired( + verb: str, kind: type[httpx.TimeoutException], label: str, limit: str +) -> None: + handler: Final = AsyncHTTPHandler(timeout=_TIMEOUT_CLIENT_LIMITS) + await handler.client.aclose() + handler.client = httpx.AsyncClient(transport=_timing_out(kind), timeout=_TIMEOUT_CLIENT_LIMITS) + try: + with pytest.raises(LitellmTimeout) as raised: + await getattr(handler, verb)("https://example.com/v1/embeddings", json={"input": "hi"}) + finally: + await handler.close() + + message: Final = str(raised.value) + assert label in message, message + assert limit in message, message + assert "None" not in message, message + + +def test_sync_timeout_reports_the_per_call_timeout_as_a_float_when_one_is_given() -> None: + handler: Final = HTTPHandler() + handler.client.close() + handler.client = httpx.Client(transport=_timing_out(httpx.ReadTimeout), timeout=_TIMEOUT_CLIENT_LIMITS) + try: + with pytest.raises(LitellmTimeout) as raised: + handler.post("https://example.com/v1/embeddings", json={"input": "hi"}, timeout=30) + finally: + handler.close() + + assert "Read timed out. Timeout passed=30.0, time taken=" in str(raised.value), str(raised.value) + + +async def test_async_timeout_reports_the_per_call_timeout_as_a_float_when_one_is_given() -> None: + handler: Final = AsyncHTTPHandler(timeout=_TIMEOUT_CLIENT_LIMITS) + await handler.client.aclose() + handler.client = httpx.AsyncClient(transport=_timing_out(httpx.ReadTimeout), timeout=_TIMEOUT_CLIENT_LIMITS) + try: + with pytest.raises(LitellmTimeout) as raised: + await handler.post("https://example.com/v1/embeddings", json={"input": "hi"}, timeout=30) + finally: + await handler.close() + + assert "Read timed out. Timeout passed=30.0, time taken=" in str(raised.value), str(raised.value)