Skip to content

Commit

Permalink
Ignore query errors during uv toolchain list (#4382)
Browse files Browse the repository at this point in the history
Closes #4380 

This is the same logic as `should_stop_discovery` but I changed the log
level and duplicated it because I don't really want that method to be
public. Maybe it should be though?
  • Loading branch information
zanieb authored Jun 18, 2024
1 parent 1ce2147 commit 58f53f0
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 25 deletions.
46 changes: 24 additions & 22 deletions crates/uv-toolchain/src/discovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -454,29 +454,31 @@ fn result_satisfies_system_python(
})
}

/// Check if an encountered error should stop discovery.
/// Check if an encountered error is critical and should stop discovery.
///
/// Returns false when an error could be due to a faulty toolchain and we should continue searching for a working one.
fn should_stop_discovery(err: &Error) -> bool {
match err {
// When querying the toolchain interpreter fails, we will only raise errors that demonstrate that something is broken
// If the toolchain interpreter returned a bad response, we'll continue searching for one that works
Error::Query(err) => match err {
InterpreterError::Encode(_)
| InterpreterError::Io(_)
| InterpreterError::SpawnFailed { .. } => true,
InterpreterError::QueryScript { path, .. }
| InterpreterError::UnexpectedResponse { path, .. }
| InterpreterError::StatusCode { path, .. } => {
trace!("Skipping bad interpreter at {}", path.display());
false
}
InterpreterError::NotFound(path) => {
trace!("Skipping missing interpreter at {}", path.display());
false
}
},
_ => true,
impl Error {
pub fn is_critical(&self) -> bool {
match self {
// When querying the toolchain interpreter fails, we will only raise errors that demonstrate that something is broken
// If the toolchain interpreter returned a bad response, we'll continue searching for one that works
Error::Query(err) => match err {
InterpreterError::Encode(_)
| InterpreterError::Io(_)
| InterpreterError::SpawnFailed { .. } => true,
InterpreterError::QueryScript { path, .. }
| InterpreterError::UnexpectedResponse { path, .. }
| InterpreterError::StatusCode { path, .. } => {
debug!("Skipping bad interpreter at {}: {err}", path.display());
false
}
InterpreterError::NotFound(path) => {
trace!("Skipping missing interpreter at {}", path.display());
false
}
},
_ => true,
}
}
}

Expand Down Expand Up @@ -641,7 +643,7 @@ pub(crate) fn find_toolchain(
let mut toolchains = find_toolchains(request, system, sources, cache);
if let Some(result) = toolchains.find(|result| {
// Return the first critical discovery error or toolchain result
result.as_ref().err().map_or(true, should_stop_discovery)
result.as_ref().err().map_or(true, Error::is_critical)
}) {
result
} else {
Expand Down
2 changes: 1 addition & 1 deletion crates/uv-toolchain/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ pub use crate::discovery::{
};
pub use crate::environment::PythonEnvironment;
pub use crate::implementation::ImplementationName;
pub use crate::interpreter::Interpreter;
pub use crate::interpreter::{Error as InterpreterError, Interpreter};
pub use crate::pointer_size::PointerSize;
pub use crate::prefix::Prefix;
pub use crate::python_version::PythonVersion;
Expand Down
10 changes: 8 additions & 2 deletions crates/uv/src/commands/toolchain/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,16 @@ pub(crate) async fn list(
&ToolchainSources::All(PreviewMode::Enabled),
cache,
)
// Raise any errors encountered during discovery
// Raise discovery errors if critical
.filter(|result| {
result
.as_ref()
.err()
.map_or(true, DiscoveryError::is_critical)
})
.collect::<Result<Vec<Result<Toolchain, ToolchainNotFound>>, DiscoveryError>>()?
.into_iter()
// Then drop any "missing" toolchains
// Drop any "missing" toolchains
.filter_map(std::result::Result::ok);

let mut output = BTreeSet::new();
Expand Down

0 comments on commit 58f53f0

Please sign in to comment.