feat: improved msg for network timeouts (#1961)
## Summary Closes #1922 When a timeout occurs, it hints to the user to configure the `UV_HTTP_TIMEOUT` env var. Before ``` error: Failed to download distributions Caused by: Failed to fetch wheel: torch==2.2.0 Caused by: Failed to extract source distribution Caused by: request or response body error: operation timed out Caused by: operation timed out ``` After ``` error: Failed to download distributions Caused by: Failed to fetch wheel: torch==2.2.0 Caused by: Failed to extract source distribution Caused by: Failed to download distribution due to network timeout. Try increasing UV_HTTP_TIMEOUT. ``` ## Test Plan <!-- How was it tested? --> Wasn't sure if we'd want a test. If we do, is there a existing mechanism or preferred approach to force a timeout to occur in tests? Maybe set the timeout to 1 and add torch as an install check (although it's possible that could become flaky)?
This commit is contained in:
@@ -88,19 +88,24 @@ impl RegistryClientBuilder {
|
||||
}
|
||||
|
||||
pub fn build(self) -> RegistryClient {
|
||||
let client_raw = self.client.unwrap_or_else(|| {
|
||||
// Timeout options, matching https://doc.rust-lang.org/nightly/cargo/reference/config.html#httptimeout
|
||||
// `UV_REQUEST_TIMEOUT` is provided for backwards compatibility with v0.1.6
|
||||
let default_timeout = 5 * 60;
|
||||
let timeout = env::var("UV_HTTP_TIMEOUT").or_else(|_| env::var("UV_REQUEST_TIMEOUT")).or_else(|_| env::var("HTTP_TIMEOUT")).and_then(|value| {
|
||||
// Timeout options, matching https://doc.rust-lang.org/nightly/cargo/reference/config.html#httptimeout
|
||||
// `UV_REQUEST_TIMEOUT` is provided for backwards compatibility with v0.1.6
|
||||
let default_timeout = 5 * 60;
|
||||
let timeout = env::var("UV_HTTP_TIMEOUT")
|
||||
.or_else(|_| env::var("UV_REQUEST_TIMEOUT"))
|
||||
.or_else(|_| env::var("HTTP_TIMEOUT"))
|
||||
.and_then(|value| {
|
||||
value.parse::<u64>()
|
||||
.or_else(|_| {
|
||||
// On parse error, warn and use the default timeout
|
||||
warn_user_once!("Ignoring invalid value from environment for UV_REQUEST_TIMEOUT. Expected integer number of seconds, got \"{value}\".");
|
||||
// On parse error, warn and use the default timeout
|
||||
warn_user_once!("Ignoring invalid value from environment for UV_HTTP_TIMEOUT. Expected integer number of seconds, got \"{value}\".");
|
||||
Ok(default_timeout)
|
||||
})
|
||||
}).unwrap_or(default_timeout);
|
||||
debug!("Using registry request timeout of {}s", timeout);
|
||||
})
|
||||
.unwrap_or(default_timeout);
|
||||
debug!("Using registry request timeout of {}s", timeout);
|
||||
|
||||
let client_raw = self.client.unwrap_or_else(|| {
|
||||
// Disallow any connections.
|
||||
let client_core = ClientBuilder::new()
|
||||
.user_agent("uv")
|
||||
@@ -130,6 +135,7 @@ impl RegistryClientBuilder {
|
||||
connectivity: self.connectivity,
|
||||
client_raw,
|
||||
client: CachedClient::new(uncached_client),
|
||||
timeout,
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -141,13 +147,15 @@ pub struct RegistryClient {
|
||||
index_urls: IndexUrls,
|
||||
/// The underlying HTTP client.
|
||||
client: CachedClient,
|
||||
/// Don't use this client, it only exists because `async_http_range_reader` needs
|
||||
/// Don't use this client, it only exists because `async_http_range_reader` needs.
|
||||
/// [`reqwest::Client] instead of [`reqwest_middleware::Client`]
|
||||
client_raw: Client,
|
||||
/// Used for the remote wheel METADATA cache
|
||||
/// Used for the remote wheel METADATA cache.
|
||||
cache: Cache,
|
||||
/// The connectivity mode to use.
|
||||
connectivity: Connectivity,
|
||||
/// Configured client timeout, in seconds.
|
||||
timeout: u64,
|
||||
}
|
||||
|
||||
impl RegistryClient {
|
||||
@@ -161,6 +169,11 @@ impl RegistryClient {
|
||||
self.connectivity
|
||||
}
|
||||
|
||||
/// Return the timeout this client is configured with, in seconds.
|
||||
pub fn timeout(&self) -> u64 {
|
||||
self.timeout
|
||||
}
|
||||
|
||||
/// Fetch a package from the `PyPI` simple API.
|
||||
///
|
||||
/// "simple" here refers to [PEP 503 – Simple Repository API](https://peps.python.org/pep-0503/)
|
||||
|
||||
@@ -73,6 +73,20 @@ impl<'a, Context: BuildContext + Send + Sync> DistributionDatabase<'a, Context>
|
||||
}
|
||||
}
|
||||
|
||||
/// Handle a specific `reqwest` error, and convert it to [`io::Error`].
|
||||
fn handle_response_errors(&self, err: reqwest::Error) -> io::Error {
|
||||
if err.is_timeout() {
|
||||
io::Error::new(
|
||||
io::ErrorKind::TimedOut,
|
||||
format!(
|
||||
"Failed to download distribution due to network timeout. Try increasing UV_HTTP_TIMEOUT (current value: {}s).", self.client.timeout()
|
||||
),
|
||||
)
|
||||
} else {
|
||||
io::Error::new(io::ErrorKind::Other, err)
|
||||
}
|
||||
}
|
||||
|
||||
/// Either fetch the wheel or fetch and build the source distribution
|
||||
///
|
||||
/// If `no_remote_wheel` is set, the wheel will be built from a source distribution
|
||||
@@ -150,7 +164,7 @@ impl<'a, Context: BuildContext + Send + Sync> DistributionDatabase<'a, Context>
|
||||
async {
|
||||
let reader = response
|
||||
.bytes_stream()
|
||||
.map_err(|err| io::Error::new(io::ErrorKind::Other, err))
|
||||
.map_err(|err| self.handle_response_errors(err))
|
||||
.into_async_read();
|
||||
|
||||
// Download and unzip the wheel to a temporary directory.
|
||||
@@ -212,7 +226,7 @@ impl<'a, Context: BuildContext + Send + Sync> DistributionDatabase<'a, Context>
|
||||
async {
|
||||
let reader = response
|
||||
.bytes_stream()
|
||||
.map_err(|err| io::Error::new(io::ErrorKind::Other, err))
|
||||
.map_err(|err| self.handle_response_errors(err))
|
||||
.into_async_read();
|
||||
|
||||
// Download and unzip the wheel to a temporary directory.
|
||||
|
||||
Reference in New Issue
Block a user