Role of `ErrorKind` and intent of the variants?

[epage]

Some early expectations of ErrorKind from nom

• Passed back through FFI to C APIs
• iirc Infer intent from &[ErrorKind], inspired by another parser library (can't find where I saw this written)

Today in Winnow, none of these apply and the default error type drops ErrorKind. It is hard to infer intent from ErrorKind because which one is reported is an implementation detail in a lot of circumstances.

Also, ErrorKind variants are focused on what operation failed, instead of describing the failure

• This is confusing (e.g. I keep wanting to use ErrorKind::Eof when an eof is found but its for when an eof isn't found)
• It doesn't scale beyond built-in parsers and should not be used as a reason to add a built-in parser
• AddContext can serve a similar role, though isn't applied to built-in parsers

Where should ErrorKind go?

[epage]

One idea is to remove them completely.

The other is to shift the focus to describing the failure and to focus on the core principles of why something fails:

enum ErrorKind {
    // For `fail`
    Failed,
    // Use for `one_of`, `take_while`/`repeat` ending early besides eof, `verify`, `try_map`
    Invalid,
    // Used for operations that hit `eof` when data was expected, including `complete_err`
    EofFound,
    // Parser pre-condition was not met
    Assert,
}

[Nukesor]

That's a pretty good question.

In my mind, it kind of depends on how a user wants to use the library.

One scenario where an ErrorKind might be useful would be when one is trying to implement some kind of error recovery strategy.
Having an ErrorKind that tries to explain the reason for a failure might allow you to perform a better guess on where to jump to, to skip the broken input and continue parsing afterwards.

Personally, I don't really need the ErrorKind yet. I simply had to create a backtrack error by hand, which required any ErrorKind. I decided to just use the Verify kind in the end, as non of the existing ones matched my exact usecase.
So if ErrorKind were to be rewritten, the possibility for a user provided ErrorKind in the form of Custom(String) could be awesome, as that could be used to easily provide some context for error handling.

FYI, I'm currently writing parsers for hand-crafted and very poorly structured file formats in the scope of Arch Linux package management. (Over here in case you're interested :D)

My usecase is that those file formats have no "nice" indicators/syntax on when a certain section is finished, which requires me to peek ahead and throw a backtracking error if I find a new section so that I can escape the current repeat parser and continue with the next section.

[Nukesor]

To be honest, error handling in winnow is one of the last things that haven't really clicked for me yet.

I'll probably spend a few hours soon to add good error handling to those libraries, maybe I'll finally understand how to do proper error handling with winnow 😅

[epage]

So if ErrorKind were to be rewritten, the possibility for a user provided ErrorKind in the form of Custom(String) could be awesome, as that could be used to easily provide some context for error handling.

Wouldn't AddContext serves that role?

[Nukesor]

Good point. I forgot about that!

[Nukesor]

Ok. I think I got a pretty good grasp on how to do proper backtracking and error handling now.

In general, I really don't see the need for the current ErrorKind.
The only thing it could be useful for is error recovery, but even than, it's not very descriptive.

---

I took some time and tought about error recovery specifically and how this would look in winnow on a library level. I guess to have something like error recovery, it would need to be properly baked into winnow. For this to be ergonomic, there would need to be some form of error aggregation mechanic and a dedicated "We found an error, seek to this position and continue with the repeat/take_while" error type, combined with some functionality to convert a backtracking error to that recoverable error in combination with a (fallible) recovery strategy (e.g. seek to next }, but not if we encounter a { first.).

Considering that this would likely require a significant restructurization of winnow's internals, I think that the ErrorKind can be dropped.

IMO error recovery would be a really nice touch, but this is a different topic :)

[epage]

We are tracking error recovery in #96 which has an unstable implementation that includes

• Recoverable stream wrapper which collects errors into a Vec
• Parser::resume_after and Parser::retry_after recovery methods
• RecoverableParser::recoverable_parse top-level parse methods

[Nukesor]

Haha, perfect.
In that case, there's really nothing left to desire! Much appreciated.

Let's kick ErrorKind out then :)

[snaggen]

I think the Errors are the most confusing thing about nom/winnow. Some issues I have with it is:
The input type is part of the error, hence methods like parse parse_next throw different errors that are incompatible.

Also, looking at the parser example in https://docs.rs/winnow/latest/winnow/_topic/partial/index.html
you have an Error type E: ParserError<Stream<'i>> + AddContext<Stream<'i>, StrContext> . My first thought was to simplify it by using type, but that doesn't work easily due to reasons. And when you have worked with these for a while, copying and pasting this around you suddenly need to throw an external error, just to realize that E doesn't have from_external_error... so a manual error you think, but then there is no way to provide any information.

And I'm sure there are ways to handle my issues, since I'm quite new at winnow, but the rest of the framework feels quite intuitive so the Errors feels very like a mess.

So, I'm happy to see that you are looking into this area, and I hope you find a way to simplify it. And maybe just some better documentation explaining how the Error handling is meant to work.

[epage]

@snaggen thank you for sharing your experience! Understanding how people are approaching Winnow and dealing with problems is a big help in figuring out how to improve it. Could you start a Discussion on that topic? It looks different enough from this issue and there are enough points to cover, that having a dedicated, threaded conversation would help in finding how to help solve those problems.