avoid panic when opening FK files in convert-fk (propagate IO error)#687
Merged
ChristopherRabotin merged 1 commit intonyx-space:masterfrom Apr 20, 2026
Merged
Conversation
Contributor
There was a problem hiding this comment.
Code Review
This pull request improves error handling by replacing panics with proper error propagation when opening files and converting FK datasets. Specifically, it updates the anise-cli and the KPL parser to return Result types instead of using unwrap or panic!, and adds an integration test to verify that missing files are handled gracefully. I have no feedback to provide.
Member
|
Thanks for your contribution! |
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
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.
Summary
Fix a user-triggerable panic in the
convert-fkCLI command when the input FK file is missing or cannot be opened.Architectural Changes
None.
New Features
None.
Improvements
convert-fkby propagating errors instead of panicking.convert-fkbehavior with existing CLI patterns (e.g.,convert-tpc).Bug Fixes
Fixes a panic caused by:
unwrap()in the CLI layer (main.rs)panic!on file open failure in the parser (parse_file)Missing or unreadable FK files now return a structured error instead of crashing the process.
Reproduction
Before
unwrap()/panic!After
Root Cause
parse_file) panicked whenFile::openfailed.convert-fk) unconditionally unwrapped the result, propagating the panic to users.Fix
unwrap()inconvert-fkwith proper error propagation using.context(...) ?panic!on file open in the parser withDataSetError::IOTesting and Validation
Added an integration test:
Manual validation:
Scope
convert-fkpath and its underlying parser behaviourImpact
Notes
convert-fkpath.