Use thiserror rather than thiserror-core

[tamird]

Manually shim std::error::Error using the core-error crate.

This has the benefit of using the upstream thiserror crate, which is
more actively maintained. In particular, thiserror has migrated to syn@2
whereas thiserror-core has not, which results in dependents of this
crate sometimes compiling syn twice.

[tamird]

@KillingSpark could you TAL please?

[KillingSpark]

Hi sorry, I saw this but forgot to respond. I'm still thinking about whether it's worth keeping the error trait impl at all. Because if not this could all just be solved by going with the 'derive_more' crate (as proposed by @tomaka in #47 ).

I'm a bit busy this and next week, is this something you need resolved urgently?

[tamird]

Perhaps we could merge this and pursue other solutions later? It's not urgent, but I do think it's a simple and clear improvement.

[tomaka]

I do think it's a simple and clear improvement.

I'm not the maintainer of this crate, but personally it took me a good minute to even understand what this PR was doing. I think this change is the opposite of simple and clear.

[KillingSpark]

Yeah I had another look I don't think I want to depend on a 0.0.0 crate that has this warning in its Readme

Warning

This is a pre-release meant to allow experimentation and integration into the various error handling crates. Please do not use it yet! 1.0.0 is right around the corner.

Especially if "right around the corner" was apparently 3 years ago and no more progress has been made.

Would you be willing to make a PR instead that switches this crate from thiserror to using derive_more for the formatting of the errors? After giving this some more thought, I think this is the right move. It would make the whole no_std support easier in the long term.

[tamird]

That warning is overstating the case: the core-error crate simply doesn't do very much - it provides a copy of std::error::Error. See core-error/core-error#15. The author didn't release a proper version because they didn't feel they had enough documentation about the crate's cargo features, none of which are used here.

[KillingSpark]

cargo features, none of which are used here.

Users could - unintentionally - enable these features if they themselves use the core-error crate though right?

[tamird]

Yeah, that's true, but I still don't quite understand how that would affect the code used here. Did you have a specific hazard in mind?

[KillingSpark]

I guess I am just weary of depending on that project. It might be fine to use it as you say, but I have to think about the long time effects of that decision.

Depending on thiserror-core turned out to be a not-so-long-term-solution, the thiserror-core repo is quite a bit behind on the promise of keeping up with the upstream project. And making people compile syn twice is not really a good look. This was fine as a mid-term solution and it's my bad for letting it linger for long enough that it has turned into an issue. When this change was done I was under the impression thiserror proper would introduce the no_std changes so we could just revert back to it. It doesnt seem like that is the case.

OTOH if someone were looking at using this crate and its dependencies, the core-error crate would rightfully raise a red flag. Which isn't a real improvement over the current situation. It seems like a change that will prompt new issues like this current one down the road.

As an outsider, it seems like the no_std community just doesnt want/care about the Error trait, based on a few observations:

1. thiserror doesn't support it.
2. thiserror-core doesn't really keep up with upstream
3. core-error didn't go anywhere
4. It's still not in stable libcore

Additionally @tomaka makes a valid argument that it's not really necessary for the types of errors this library returns. thiserror was more or less just nice for the formatting of the error messages. Supporting the error trait does keep bringing up issues like the current one, so it's not only not really all that useful, it means constant changes to the code base.

With all of this in mind I'd favor a solution that justworks(TM) with no_std and do so longterm. The suggested derive_more crate would provide all of that AFAICT and as soon as the error trait is in libcore we can think about deriving the error trait through that library.

I'll have a look if there are any other crates that provide easy formatting that might be better suited but derive_more seems like a widely used option.

[tamird]

That's fine with me. I think you're making perfect the enemy of good here, but that's your decision to make.

[KillingSpark]

Thanks for the effort and the input, it's very much appreciated! It's definitly given me the needed push to resolve this situation for good.