From e6949b81e2512f91468943a5cc7fc283c3363e74 Mon Sep 17 00:00:00 2001 From: konstin Date: Mon, 27 Oct 2025 16:06:47 +0100 Subject: [PATCH] Review --- crates/uv-client/src/error.rs | 4 +++- crates/uv-client/src/registry_client.rs | 20 ++++++++++++-------- crates/uv-publish/src/lib.rs | 20 +++++++++++--------- crates/uv-resolver/src/resolver/provider.rs | 2 +- crates/uv/src/commands/pip/latest.rs | 2 +- 5 files changed, 28 insertions(+), 20 deletions(-) diff --git a/crates/uv-client/src/error.rs b/crates/uv-client/src/error.rs index b2ec2ef8f..67ba19312 100644 --- a/crates/uv-client/src/error.rs +++ b/crates/uv-client/src/error.rs @@ -3,6 +3,8 @@ use std::ops::Deref; use async_http_range_reader::AsyncHttpRangeReaderError; use async_zip::error::ZipError; +use http::StatusCode; +use rustc_hash::FxHashSet; use serde::Deserialize; use uv_distribution_filename::{WheelFilename, WheelFilenameError}; @@ -278,7 +280,7 @@ pub enum ErrorKind { /// Make sure the package name is spelled correctly and that you've /// configured the right registry to fetch it from. #[error("Package `{0}` was not found in the registry")] - StatusCodeError(PackageName), + StatusCodeError(PackageName, FxHashSet), /// The package was not found in the local (file-based) index. #[error("Package `{0}` was not found in the local index")] diff --git a/crates/uv-client/src/registry_client.rs b/crates/uv-client/src/registry_client.rs index 3c9b476b1..d5644910d 100644 --- a/crates/uv-client/src/registry_client.rs +++ b/crates/uv-client/src/registry_client.rs @@ -3,7 +3,6 @@ use std::fmt::Debug; use std::path::PathBuf; use std::str::FromStr; use std::sync::Arc; -use std::sync::atomic::{AtomicBool, Ordering}; use std::time::Duration; use async_http_range_reader::AsyncHttpRangeReader; @@ -11,7 +10,7 @@ use futures::{FutureExt, StreamExt, TryStreamExt}; use http::{HeaderMap, StatusCode}; use itertools::Either; use reqwest::{Proxy, Response}; -use rustc_hash::FxHashMap; +use rustc_hash::{FxHashMap, FxHashSet}; use tokio::sync::{Mutex, Semaphore}; use tracing::{Instrument, debug, info_span, instrument, trace, warn}; use url::Url; @@ -325,7 +324,7 @@ impl RegistryClient { }; let mut results = Vec::new(); - let any_status_code_error = AtomicBool::new(false); + let status_code_errors = Arc::new(Mutex::new(FxHashSet::default())); match self.index_strategy_for(package_name) { // If we're searching for the first index that contains the package, fetch serially. @@ -357,7 +356,7 @@ impl RegistryClient { debug!( "Indexes search failed because of status code failure: {status_code}" ); - any_status_code_error.store(true, Ordering::Relaxed); + status_code_errors.lock().await.insert(status_code); break; } } @@ -394,8 +393,8 @@ impl RegistryClient { { SimpleMetadataSearchOutcome::Found(metadata) => Some(metadata), SimpleMetadataSearchOutcome::NotFound => None, - SimpleMetadataSearchOutcome::StatusCodeFailure(_) => { - any_status_code_error.store(true, Ordering::Relaxed); + SimpleMetadataSearchOutcome::StatusCodeFailure(status_code) => { + status_code_errors.lock().await.insert(status_code); None } }; @@ -422,8 +421,13 @@ impl RegistryClient { if results.is_empty() { return match self.connectivity { Connectivity::Online => { - if any_status_code_error.load(Ordering::Relaxed) { - Err(ErrorKind::StatusCodeError(package_name.clone()).into()) + let status_code_errors = status_code_errors.lock().await; + if !status_code_errors.is_empty() { + Err(ErrorKind::StatusCodeError( + package_name.clone(), + status_code_errors.clone(), + ) + .into()) } else { Err(ErrorKind::PackageNotFound(package_name.clone()).into()) } diff --git a/crates/uv-publish/src/lib.rs b/crates/uv-publish/src/lib.rs index abedd1f2f..db3994fef 100644 --- a/crates/uv-publish/src/lib.rs +++ b/crates/uv-publish/src/lib.rs @@ -563,16 +563,18 @@ pub async fn check_url( ); Ok(false) } - uv_client::ErrorKind::StatusCodeError(_) => { + uv_client::ErrorKind::StatusCodeError(_, status_code_error) => { + // TODO(konsti): We currently can't track that this must be exactly one status + // error with check URL. + debug_assert!( + status_code_error.len() == 1, + "Check URL must only check a single URL" + ); + let status_code = status_code_error.iter().next(); // The package may or may not exist, there was an authentication failure. - // TODO(konsti): We should show the real index error instead. - let status_code_detail = if index_capabilities.unauthorized(index_url) { - "401 Unauthorized" - } else if index_capabilities.forbidden(index_url) { - "403 Forbidden" - } else { - "Status code error" - }; + let status_code_detail = status_code + .map(ToString::to_string) + .unwrap_or("Status code error".to_string()); warn!( "Package not found in the registry; skipping upload check for {filename}" ); diff --git a/crates/uv-resolver/src/resolver/provider.rs b/crates/uv-resolver/src/resolver/provider.rs index 7566b0f04..84478fda0 100644 --- a/crates/uv-resolver/src/resolver/provider.rs +++ b/crates/uv-resolver/src/resolver/provider.rs @@ -201,7 +201,7 @@ impl ResolverProvider for DefaultResolverProvider<'_, Con )), Err(err) => match err.kind() { uv_client::ErrorKind::PackageNotFound(_) - | uv_client::ErrorKind::StatusCodeError(_) => { + | uv_client::ErrorKind::StatusCodeError(..) => { if let Some(flat_index) = flat_index .and_then(|flat_index| flat_index.get(package_name)) .cloned() diff --git a/crates/uv/src/commands/pip/latest.rs b/crates/uv/src/commands/pip/latest.rs index 09d53db91..95760dabb 100644 --- a/crates/uv/src/commands/pip/latest.rs +++ b/crates/uv/src/commands/pip/latest.rs @@ -47,7 +47,7 @@ impl LatestClient<'_> { Err(err) => { return match err.kind() { uv_client::ErrorKind::PackageNotFound(_) - | uv_client::ErrorKind::StatusCodeError(_) + | uv_client::ErrorKind::StatusCodeError(..) | uv_client::ErrorKind::NoIndex(_) | uv_client::ErrorKind::Offline(_) => Ok(None), _ => Err(err),