An invalid manifest file should not result in a panic!#172
Merged
Conversation
bradhe
requested review from
Copilot,
jo-sm,
konstantinoscs,
sammuti and
socksy
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes a panic in the package loading logic by changing from_unpacked_path to return a Result instead of unwrapping errors when reading manifest files. This allows callers to handle invalid manifest files gracefully rather than crashing.
Changes:
- Modified
from_unpacked_pathto returnResult<Self, Error>instead ofSelf - Replaced
.unwrap()with?operator for proper error propagation
Comments suppressed due to low confidence (1)
crates/tower-package/src/lib.rs:1
- Corrected spelling of 'shoulnd't' to 'shouldn't'.
use config::Towerfile;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
jo-sm
reviewed
Jan 16, 2026
| parameters.push(Parameter { | ||
| name: p.name.clone(), | ||
| description: p.description.clone(), | ||
| description: Some(p.description.clone()), |
Member
There was a problem hiding this comment.
Will this cause issues if p.description is None?
Contributor
Author
There was a problem hiding this comment.
Nope, it's not really used. Compiler told me so.
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.
Title says it all, we shoulnd't just blindly panic when there's an invalid manifest file in a package. This makes the caller decide what happened.