Error Management: switch to `Failure` ? (Disposition: No) about goblin HOT 8 OPEN

I've used failure in a few projects now and it's pretty great. It feels similar to error-chain but with a bit less magic. You could probably start using it now without changing much--just add #[derive(Fail)] to your Error enum as a start, and then you ought to be able to remove the std::error::Error impl, and the From impls. You might want to switch scroll over first so that you get implicit conversion from its errors as well.

You can see a simple example of using failure with a custom error type in this code I wrote:
https://github.com/luser/rust-disasm/blob/0758b8ac2820cd73137284423b7dfaf04c2701bd/src/lib.rs#L20

There are a few things there that are silly because the gimli crates are using error-chain, and interop between failure and error-chain is a little wonky. (boats has a PR open on error-chain with a fix, but it's a breaking change.)

In the meantime, the recommended workaround is to define a new trait that converts error-chain errors, which isn't the best but it works. Here's an example of that:
https://github.com/luser/query-crates-index/blob/26a825fc11ac8d29b32bdf35a92748e3e31dedf4/src/main.rs#L30

You wind up writing whatever().sync()? for things that return error-chain errors, which is workable.

from goblin.

I think we wouldn't want to use failure::Error in this crate, but instead #[derive(Failure)] on a custom error enum.

from goblin.

silly because the gimli crates are using error-chain

Would love to get PRs to move to failure, btw.

from goblin.

I think we wouldn't want to use failure::Error in this crate, but instead #[derive(Failure)] on a custom error enum.

Yeah, that's what I was suggesting, just deriving Fail on the existing Error enum. That should immediately give you some niceties like auto-conversion from other error types, as well as easy error message formatting. (That being said, I'm not sure what the practical benefits of using your own error type vs. failure::Error are, really. Just the ability to pattern-match specific errors, I guess?)

from goblin.

So with the new alloc feature I think just adding derive annotation will work as expected and won't cause any problems in no_std environments, would you agree @philipc ?

from goblin.

Yes, my understanding is that failure::Error can't be used with no_std, but derive is fine.

gimli crates are using error-chain

This was only in addr2line afaik, and that usage has been removed. Adding failure support to gimli would be good though.

from goblin.

Ok I think we should get the error story figured out. I'm leaning towards sticking with current error system, not pulling on another dep, as it's compatible with downstream users using failure.

from goblin.

Failure is unmaintained and deprecated now. https://github.com/rust-lang-nursery/failure

from goblin.