<R: Read> Iterator for Saphyr parser input cannot communicate the IO error.

[bourumir-wyngs]

I understand this would be breaking change, and maybe worth discussing how this could be done, but currently if I want to put a buffered reader as the source for Saphyr parser (0.0.6), I finally need to implement T: Iterator<Item = char>. My source is T in this your code:

pub struct BufferedInput<T: Iterator<Item = char>> {
    /// The iterator source,
    input: T,
    /// Buffer for the next characters to consume.
    buffer: ArrayDeque<char, BUFFER_LEN>,
}

This means my T must provide the method

fn next(&mut self) -> Option<char>

The problem with this approach is:

• What should I do if I encounter IO error? The value None is reserved for EOF. As YAML end of document marker is optional, returning EOF on IO error may silently pass the wrong (truncated) YAML document as valid, potentially with far reaching consequences. panic! I also do not like.
• Same if my budget checker concludes that our raw input is already too long (this is how I actually discovered the limitation). While String can be checked for the total length before parsing, the Reader not.

This can be fixed by changing the signature to return either Result<char, Error> where Error is the enum with one value reserved for EOF, or returning Result<Option<char>, io::Error> when None in place of the char would mean EOF has been reached while error should also be reported.

Finally, "non breaking" but still not without concerns idea I am currently attempting is to return FFFD that is invalid Unicode char. This also forces parser to stop, but the only way to report the reason why I did so is eprintln!() that is also not very elegant.

I suggest to think about possibility of error reporting. The parser itself could return this error as any other kind of ScanError, this functionality on the event listening is already implemented.

[emmatyping]

In my use case, I would like to update my yaml parser for Python to be based on saphyr-parser, but I cannot bubble up exceptions to Python because Input needs to be infallible.

The best idea I've had so far to work around this is to panic then use catch_unwind around the parser to convert the panic into a Python exception, but that won't work if panic is set to abort and is pretty hacky.

I personally like the Result<Option<char>, io::Error> solution, as it makes it easy to differentiate between EOF and an actual error in I/O, but I think since the parser will be driving the Input, it's really up to the maintainers.

[bourumir-wyngs]

The workaround I made in serde-saphyr 0.0.8 is that the reader receives and retains the shared reference (Rust's Rc). When it encounters an IO error, it places the error there and returns EOF (so the parser stops), but the parsing code (which retained that Rc as well) also checks for the error record first. I can do this because the Saphyr parser is not in the user-exposed part, so I can use shared references without placing a burden on the user to allocate and later check that Rc. This is however, does not look like a recommended solution. If it is crucial to avoid a major compatibility break, we could have two functions to fetch from the parser, maybe one deprecated.

[chirizxc]

In my use case, I would like to update my yaml parser for Python to be based on saphyr-parser, but I cannot bubble up exceptions to Python because Input needs to be infallible.

The best idea I've had so far to work around this is to panic then use catch_unwind around the parser to convert the panic into a Python exception, but that won't work if panic is set to abort and is pretty hacky.

I personally like the Result<Option<char>, io::Error> solution, as it makes it easy to differentiate between EOF and an actual error in I/O, but I think since the parser will be driving the Input, it's really up to the maintainers.

Hmm, I haven't seen your implementation before, so I wrote my own, thanks, I'll add it to benchmarks and check if it passes the tests from the yaml repository!

[emmatyping]

@chirizxc would be happy to collaborate on ryaml if you'd like! It's in need of a revamp, which is why I wanted to move to saphyr and wanted to implement Input for Python files. If you're interested please feel free to comment on emmatyping/ryaml#10