jam1garner / jam1garner/binrw

Make correct error handling easier for manual implementations

Open
#155 0 comments 1 reaction 0 assignees View on GitHub
enhancement
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.