fix: propagate error in sort_paths instead of panicking#2188
Open
suhr25 wants to merge 2 commits intoconda:mainfrom
Open
fix: propagate error in sort_paths instead of panicking#2188suhr25 wants to merge 2 commits intoconda:mainfrom
suhr25 wants to merge 2 commits intoconda:mainfrom
Conversation
… under base_path Signed-off-by: suhr25 <suhridmarwah07@gmail.com>
suhr25
changed the title
- Gate `StreamExt` and `AsyncReadExt` imports with `#[cfg(feature = "reqwest")]` since they are only used in reqwest-gated code paths - Rename `|_|` to `|_e|` in map_err to satisfy clippy::map_err_ignore Signed-off-by: suhr25 <suhridmarwah07@gmail.com>
Contributor
Author
|
Hi @baszalmstra, |
|
Just wanted to left a comment since the pr interested me, but using a loop by replacing an iterator is not a correct approach in my opinion. You could just use collect and still use the simple iterator. |
baszalmstra
reviewed
Mar 11, 2026
Comment on lines
+79
to
+91
| for p in paths { | ||
| let rel = p.strip_prefix(base_path).map_err(|_e| { | ||
| io::Error::new( | ||
| io::ErrorKind::InvalidInput, | ||
| format!( | ||
| "path '{}' is not under base path '{}'", | ||
| p.display(), | ||
| base_path.display() | ||
| ), | ||
| ) | ||
| })?; | ||
| relative_paths.push(rel.to_path_buf()); | ||
| } |
Collaborator
There was a problem hiding this comment.
Introducing a for loop is not better.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
sort_pathspreviously used.unwrap()onPath::strip_prefix, which could panic if a path was not underbase_path. This change replaces the panic with proper error propagation by returning anio::Error. The two call sites now propagate the error using?, aligning the implementation with the documented behavior of the package writing APIs.Fixes
.unwrap()onstrip_prefixwith proper error handlingsort_pathsto returnResult<(Vec<PathBuf>, Vec<PathBuf>), io::Error>?base_pathHow Has This Been Tested?
Tested by passing a path that is not under the provided base_path. Before the fix the code panicked due to .unwrap(). After the fix it correctly returns an io::Error.
AI Disclosure
Tools: Chatgpt, Claude
Checklist