mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-21 00:21:49 +00:00
fix(rust): honor environment proxies by default and name the cause in transport errors
Python's aiohttp transport reads HTTP(S)_PROXY on every request unless disable_aiohttp_trust_env is set, so the Rust clients now do the same instead of requiring aiohttp_trust_env. Transport error messages include reqwest's source chain, so a rejected certificate or refused connection is no longer reported as just 'error sending request'
This commit is contained in:
parent
a3aceec2f8
commit
8d2476465f
7 changed files with 79 additions and 13 deletions
|
|
@ -55,7 +55,10 @@ impl HttpClientConfig {
|
|||
force_ipv4: settings.force_ipv4,
|
||||
http2: settings.http2,
|
||||
user_agent: settings.user_agent.clone(),
|
||||
trust_proxy_env: settings.trust_proxy_env || settings.http2 || settings.httpx_transport,
|
||||
trust_proxy_env: !settings.ignore_proxy_env
|
||||
|| settings.trust_proxy_env
|
||||
|| settings.http2
|
||||
|| settings.httpx_transport,
|
||||
connect_timeout: settings.connect_timeout,
|
||||
})
|
||||
}
|
||||
|
|
@ -237,11 +240,21 @@ mod tests {
|
|||
}
|
||||
|
||||
#[rstest]
|
||||
#[case::aiohttp_default(HttpSettings::default(), false)]
|
||||
#[case::aiohttp_trust_env(HttpSettings { trust_proxy_env: true, ..HttpSettings::default() }, true)]
|
||||
#[case::http2_uses_httpx(HttpSettings { http2: true, ..HttpSettings::default() }, true)]
|
||||
#[case::aiohttp_disabled(HttpSettings { httpx_transport: true, ..HttpSettings::default() }, true)]
|
||||
fn environment_proxies_apply_whenever_python_would_use_httpx(
|
||||
#[case::aiohttp_default(HttpSettings::default(), true)]
|
||||
#[case::aiohttp_opted_out(HttpSettings { ignore_proxy_env: true, ..HttpSettings::default() }, false)]
|
||||
#[case::session_trust_env_beats_opt_out(
|
||||
HttpSettings { ignore_proxy_env: true, trust_proxy_env: true, ..HttpSettings::default() },
|
||||
true
|
||||
)]
|
||||
#[case::http2_uses_httpx(
|
||||
HttpSettings { ignore_proxy_env: true, http2: true, ..HttpSettings::default() },
|
||||
true
|
||||
)]
|
||||
#[case::aiohttp_disabled(
|
||||
HttpSettings { ignore_proxy_env: true, httpx_transport: true, ..HttpSettings::default() },
|
||||
true
|
||||
)]
|
||||
fn environment_proxies_apply_unless_the_aiohttp_transport_opts_out(
|
||||
#[case] settings: HttpSettings,
|
||||
#[case] expected: bool,
|
||||
) {
|
||||
|
|
|
|||
|
|
@ -32,6 +32,7 @@ pub struct HttpSettings {
|
|||
pub httpx_transport: bool,
|
||||
pub user_agent: Option<String>,
|
||||
pub trust_proxy_env: bool,
|
||||
pub ignore_proxy_env: bool,
|
||||
pub connect_timeout: Duration,
|
||||
}
|
||||
|
||||
|
|
@ -48,6 +49,7 @@ impl Default for HttpSettings {
|
|||
httpx_transport: false,
|
||||
user_agent: None,
|
||||
trust_proxy_env: false,
|
||||
ignore_proxy_env: false,
|
||||
connect_timeout: Duration::from_secs(5),
|
||||
}
|
||||
}
|
||||
|
|
@ -78,6 +80,7 @@ impl HttpSettings {
|
|||
httpx_transport: self.httpx_transport || enabled("DISABLE_AIOHTTP_TRANSPORT"),
|
||||
user_agent: env("LITELLM_USER_AGENT").or(self.user_agent),
|
||||
trust_proxy_env: self.trust_proxy_env || enabled("AIOHTTP_TRUST_ENV"),
|
||||
ignore_proxy_env: self.ignore_proxy_env || enabled("DISABLE_AIOHTTP_TRUST_ENV"),
|
||||
..self
|
||||
}
|
||||
}
|
||||
|
|
@ -214,14 +217,16 @@ mod tests {
|
|||
#[case("1", false)]
|
||||
fn boolean_switches_only_turn_on_for_true(#[case] value: &'static str, #[case] expected: bool) {
|
||||
let env = move |name: &str| match name {
|
||||
"LITELLM_HTTP2" | "AIOHTTP_TRUST_ENV" | "DISABLE_AIOHTTP_TRANSPORT" => {
|
||||
Some(value.to_string())
|
||||
}
|
||||
"LITELLM_HTTP2"
|
||||
| "AIOHTTP_TRUST_ENV"
|
||||
| "DISABLE_AIOHTTP_TRANSPORT"
|
||||
| "DISABLE_AIOHTTP_TRUST_ENV" => Some(value.to_string()),
|
||||
_ => None,
|
||||
};
|
||||
let settings = HttpSettings::default().with_environment(&env);
|
||||
assert_eq!(settings.http2, expected);
|
||||
assert_eq!(settings.httpx_transport, expected);
|
||||
assert_eq!(settings.trust_proxy_env, expected);
|
||||
assert_eq!(settings.ignore_proxy_env, expected);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -11,7 +11,7 @@ pub enum Error {
|
|||
impl Error {
|
||||
pub fn from_reqwest_before_dispatch(error: reqwest::Error) -> Self {
|
||||
let before_dispatch = !error.is_timeout() && (error.is_connect() || error.is_builder());
|
||||
let message = error.without_url().to_string();
|
||||
let message = describe(error);
|
||||
if before_dispatch {
|
||||
Self::Connect(message)
|
||||
} else {
|
||||
|
|
@ -22,10 +22,18 @@ impl Error {
|
|||
|
||||
impl From<reqwest::Error> for Error {
|
||||
fn from(error: reqwest::Error) -> Self {
|
||||
Self::Network(error.without_url().to_string())
|
||||
Self::Network(describe(error))
|
||||
}
|
||||
}
|
||||
|
||||
fn describe(error: reqwest::Error) -> String {
|
||||
let error = error.without_url();
|
||||
std::iter::successors(std::error::Error::source(&error), |cause| cause.source())
|
||||
.fold(error.to_string(), |message, cause| {
|
||||
format!("{message}: {cause}")
|
||||
})
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
#[tokio::test]
|
||||
|
|
@ -47,6 +55,32 @@ mod tests {
|
|||
assert!(!error.to_string().contains("private"));
|
||||
}
|
||||
|
||||
fn root_cause(error: &dyn std::error::Error) -> Option<String> {
|
||||
match error.source() {
|
||||
Some(cause) => root_cause(cause).or_else(|| Some(cause.to_string())),
|
||||
None => None,
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn network_error_message_names_the_underlying_cause() {
|
||||
let listener = std::net::TcpListener::bind("127.0.0.1:0").expect("bind");
|
||||
let address = listener.local_addr().expect("address");
|
||||
drop(listener);
|
||||
let error = reqwest::Client::builder()
|
||||
.no_proxy()
|
||||
.build()
|
||||
.expect("client")
|
||||
.get(format!("http://{address}/private?api_key=secret"))
|
||||
.send()
|
||||
.await
|
||||
.expect_err("nothing listens on the port");
|
||||
let root_cause = root_cause(&error).expect("reqwest reports a cause");
|
||||
let message = crate::custom_httpx::transport::Error::from(error).to_string();
|
||||
assert!(message.contains(&root_cause), "{message}");
|
||||
assert!(!message.contains("secret"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn request_timeout_is_not_safe_to_retry_as_a_connect_failure() {
|
||||
use std::time::Duration;
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@
|
|||
"force_ipv4",
|
||||
"http2",
|
||||
"aiohttp_trust_env",
|
||||
"disable_aiohttp_trust_env",
|
||||
"disable_aiohttp_transport",
|
||||
"user_agent"
|
||||
]
|
||||
|
|
|
|||
|
|
@ -72,6 +72,7 @@ struct PythonHttpSettings<'py> {
|
|||
force_ipv4: bool,
|
||||
http2: bool,
|
||||
aiohttp_trust_env: bool,
|
||||
disable_aiohttp_trust_env: bool,
|
||||
disable_aiohttp_transport: bool,
|
||||
user_agent: String,
|
||||
}
|
||||
|
|
@ -92,6 +93,7 @@ fn settings(value: &Bound<'_, PyAny>) -> PyResult<HttpSettings> {
|
|||
httpx_transport: python.disable_aiohttp_transport,
|
||||
user_agent: Some(python.user_agent),
|
||||
trust_proxy_env: python.aiohttp_trust_env,
|
||||
ignore_proxy_env: python.disable_aiohttp_trust_env,
|
||||
..HttpSettings::default()
|
||||
})
|
||||
}
|
||||
|
|
@ -133,6 +135,7 @@ defaults = dict(
|
|||
force_ipv4=False,
|
||||
http2=False,
|
||||
aiohttp_trust_env=False,
|
||||
disable_aiohttp_trust_env=False,
|
||||
disable_aiohttp_transport=False,
|
||||
user_agent='litellm/test',
|
||||
)
|
||||
|
|
@ -177,6 +180,7 @@ ssl_ecdh_curve='X25519',
|
|||
force_ipv4=True,
|
||||
http2=True,
|
||||
aiohttp_trust_env=True,
|
||||
disable_aiohttp_trust_env=True,
|
||||
disable_aiohttp_transport=True,
|
||||
user_agent='litellm/9.9.9',
|
||||
",
|
||||
|
|
@ -194,6 +198,7 @@ user_agent='litellm/9.9.9',
|
|||
httpx_transport: true,
|
||||
user_agent: Some("litellm/9.9.9".into()),
|
||||
trust_proxy_env: true,
|
||||
ignore_proxy_env: true,
|
||||
..HttpSettings::default()
|
||||
}
|
||||
);
|
||||
|
|
@ -295,11 +300,15 @@ user_agent='litellm/9.9.9',
|
|||
#[rstest]
|
||||
#[case::asynchronous(true, false)]
|
||||
#[case::synchronous(false, true)]
|
||||
fn synchronous_calls_honor_environment_proxies_like_httpx(
|
||||
fn synchronous_calls_honor_environment_proxies_even_when_aiohttp_opts_out(
|
||||
#[case] asynchronous: bool,
|
||||
#[case] expected: bool,
|
||||
) {
|
||||
let settings = for_call(HttpSettings::default(), None, asynchronous);
|
||||
let opted_out = HttpSettings {
|
||||
ignore_proxy_env: true,
|
||||
..HttpSettings::default()
|
||||
};
|
||||
let settings = for_call(opted_out, None, asynchronous);
|
||||
let config = HttpClientConfig::resolve(&settings).unwrap();
|
||||
assert_eq!(config.trust_proxy_env, expected);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ class HttpSettings:
|
|||
force_ipv4: bool
|
||||
http2: bool
|
||||
aiohttp_trust_env: bool
|
||||
disable_aiohttp_trust_env: bool
|
||||
disable_aiohttp_transport: bool
|
||||
user_agent: str
|
||||
|
||||
|
|
@ -28,6 +29,7 @@ def http_settings() -> HttpSettings:
|
|||
force_ipv4=litellm.force_ipv4,
|
||||
http2=litellm.http2,
|
||||
aiohttp_trust_env=litellm.aiohttp_trust_env,
|
||||
disable_aiohttp_trust_env=litellm.disable_aiohttp_trust_env,
|
||||
disable_aiohttp_transport=litellm.disable_aiohttp_transport,
|
||||
user_agent=default_user_agent(),
|
||||
)
|
||||
|
|
|
|||
|
|
@ -26,6 +26,7 @@ def test_http_settings_reads_the_litellm_globals(monkeypatch: pytest.MonkeyPatch
|
|||
monkeypatch.setattr(litellm, "force_ipv4", True)
|
||||
monkeypatch.setattr(litellm, "http2", True)
|
||||
monkeypatch.setattr(litellm, "aiohttp_trust_env", True)
|
||||
monkeypatch.setattr(litellm, "disable_aiohttp_trust_env", True)
|
||||
monkeypatch.setattr(litellm, "disable_aiohttp_transport", True)
|
||||
|
||||
assert settings.http_settings() == settings.HttpSettings(
|
||||
|
|
@ -36,6 +37,7 @@ def test_http_settings_reads_the_litellm_globals(monkeypatch: pytest.MonkeyPatch
|
|||
force_ipv4=True,
|
||||
http2=True,
|
||||
aiohttp_trust_env=True,
|
||||
disable_aiohttp_trust_env=True,
|
||||
disable_aiohttp_transport=True,
|
||||
user_agent=default_user_agent(),
|
||||
)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue