Make correct error handling easier for manual implementations
- Dominant language
- Rust
- Stars
- 853
- Forks
- 56
- PR merge metrics
- No merged PRs in 30d
Description
Copied from #9:
Error recovery by rewinding the stream on parse failure is implemented only in the derive and the default impls. Requiring manual implementations to handle this themselves is error-prone, remembering to consistently implement it in BinRead itself is error-prone, and manually tracking and reporting the position of the field/struct/variant which failed to be read successfully is error-prone. Consider something like splitting the API so `read_options` becomes something like:
```rust
fn read_options(reader: &mut R, options: &ReadOptions, args: Self::Args) -> BinResult {
let pos = reader.seek(SeekFrom::Current(0))?;
read_options_impl(reader, options, args).map_err(|mut err| {
if let Some(seek_error) = reader.seek(SeekFrom::Start(pos)) {
err.append(seek_error);
}
err
})
}
```
…and then `read_options_impl` (bikeshed name) is where the actual implementation goes, so any error recovery can be fully uniform (and more options could be made available if someone requests them, e.g. if someone prefers to have a stream end in an undefined state or receive no position information on error in exchange for fewer seeks).
This may actually be impossible to fully realise because it would require calls from _inside_ another read_options to become read_options_impl or whatever and only callees that are not read_options would call through a wrapper? At the least some helper functions can exist to make this job easier, because I’ve already written them for myself.
---
The other part of this is to make it easier to actually generate errors, since right now it is a bit verbose and could probably be easier. A helper that records the position and makes it easy to call an error probably. Similar to `syn::parse::Lookahead1` maybe?; any thing which checkpoints positions and has some easy method to call that bails with the error.
Contributor guide
Research direction
Start by tracing BinRead's read_options implementations in the derive and default impls, then compare them with the proposed read_options_impl wrapper. Define the smallest uniform API for stream rewind and position-aware errors, including the helper or checkpoint behavior; done means manual implementations no longer need to duplicate that recovery and error-reporting logic.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- tooling
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100