Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: improved msg for network timeouts #1961

Merged
merged 5 commits into from
Feb 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 24 additions & 11 deletions crates/uv-client/src/registry_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -130,6 +135,7 @@ impl RegistryClientBuilder {
connectivity: self.connectivity,
client_raw,
client: CachedClient::new(uncached_client),
timeout,
}
}
}
Expand All @@ -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 {
Expand All @@ -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/)
Expand Down
18 changes: 16 additions & 2 deletions crates/uv-distribution/src/distribution_database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down