Don't panic with invalid wheel source #4191
Conversation
crates/uv-resolver/src/lock.rs
Outdated
| @@ -1780,6 +1782,9 @@ impl std::fmt::Display for LockError { | |||
| "found distribution `{id}` with development dependency group `{group}` but no base distribution", | |||
| ) | |||
| } | |||
| LockErrorKind::InvalidWheelSource { source } => { | |||
There was a problem hiding this comment.
@BurntSushi Is there are reason why we implement as of those manually instead of deriving them with thiserror?
There was a problem hiding this comment.
No real reason other than my fingers are just very used to typing out the error types. I'm fine if you want to convert over to thiserror.
(I have also been thinking that we might perhaps just want to use an anyhow::Error in most places.)
Remove the panic when there is an invalid wheel source, instead surface the error. This error can only occur when manually editing the lock file, but since it's an external file, we should error and not panic. This change is helpful since the method needs to be able to error for relative path support.
konstin
force-pushed
the
konsti/dont-panic-invalid-source
branch
from
bb8a826 to
6df6b1a
Compare
| Dist::Built(built_dist) | ||
| } | ||
| SourceKind::Git(_) => { | ||
| unreachable!("Wheels cannot come from Git sources") |
There was a problem hiding this comment.
I think the original intent here was that these cases were actually impossible as a contract of constructing a Distribution. I wonder if that got flubbed somehow.
There was a problem hiding this comment.
Yeah this should be impossible.
There was a problem hiding this comment.
I wouldn't know a good way to tell serde about this correspondence, any hints?
There was a problem hiding this comment.
If I can't make the types do it for me, then I usually define a TryFrom impl and point Serde to that. For example:
uv/crates/uv-resolver/src/lock.rs
Line 333 in 04c4da4
Remove the panic when there is an invalid wheel source, instead surface the error. This error can only occur when manually editing the lock file, but since it's an external file, we should error and not panic.
This change is helpful since the method needs to be able to error for relative path support.