PasrseError and error Trait

[darrell-roberts]

Hi,

I was wondering why the ParseError struct does not implement the std::error::Error trait? I was hoping to wrap it in a custom error enum type and use the "?" operator when calling parse functions. From what I've tried so far has not allowed me to do so.

[SimonSapin]

I don’t think there’s a reason not to have that impl, it’s just that nobody bothered to add it so far. I personally find the Error trait not to be useful. Note that the ? operator does not involve the Error trait at al. It relies on the From trait to convert between error types. So the fix to your issue might be to add impl From<cssparser::ParseError> for YourErrorType {…}.

That said, I’d accept a PR to add impls of Error for ParseError and BasicParseError.

[darrell-roberts]

Hi,

Your right that it does not need to implement the Error trait for the ? operator. What I did in my client code is create an generalized error enum with variants for all the different crate errors I would encounter. This enum then implements the error trait and delegates to the variants for display and error traits (description, cause). That way I have a common result type I create that would return my wrapper error enum for the Result Err variant.

I'll checkout the cssparser code and add the Error trait and see if I can get that working and then I'll send off the PR.

[Boscop]

Please add the impl of std::error::Error for ParseError :)
I'm using this crate with anyhow btw, and ? isn't working because this impl doesn't exist.

[jdm]

@Boscop #247 (comment) pointed out that Error is not needed for ? to work.

[Boscop]

@jdm Yes but it requires a custom error type and a From impl. I'd prefer if ParseError worked out of the box with anyhow::Result and ?.

[Boscop]

Even with a custom From impl, I still have to call .map_err(MyError::from)?, just ? isn't enough :(
And the ParseError only impls Debug, not Display, and doesn't seem to contain the selector that it errored on. So my From impl doesn't have that as context..

So it could be improved by implementing Display, std::error::Error and making its Display impl also print the selector that caused the error.
(Like reqwest includes the url that caused an error in the error's Display output, e.g. error sending request for url (https://...): connection closed before message completed)

[SimonSapin]

print the selector that caused the error.

What do you mean by that? This library mostly only knows about tokens, not CSS Selectors.

[Boscop]

@SimonSapin Sorry, I'm using scraper: https://docs.rs/scraper/0.11.0/scraper/selector/struct.Selector.html#method.parse

So please ignore that point. The others are still valid though :)

[koutoftimer]

Why can't you just use thiserror which de facto is a standard? Issue originates in 2019 a lot have been changed since that time.