Skip to content
This repository has been archived by the owner on Aug 16, 2021. It is now read-only.

Any plans for clonable errors? #148

Closed
c410-f3r opened this issue Feb 2, 2018 · 5 comments
Closed

Any plans for clonable errors? #148

c410-f3r opened this issue Feb 2, 2018 · 5 comments

Comments

@c410-f3r
Copy link

c410-f3r commented Feb 2, 2018

Related: 121, 134 and 24135.

@withoutboats
Copy link
Contributor

First, nothing stops you from implementing Clone for a type that implements Fail. Errors in general can be Clone just like any other type, because unlike error-chain we don't inject a trait object into your error type.

However, the Error type provided by failure itself is not Clone, because it is essentially a trait object. It has occurred to me that we could make it into an Arc instead of a Box, but we can't do that backwards compatibly because we exposed the downcast_mut method on `Error. I don't feel great about this because I think mutating an error is an anti-pattern, whereas cloning it is not. But I don't know what to do.

@Celti
Copy link

Celti commented Feb 3, 2018

If the issue is "we can't make breaking changes to Error &c. because then things using old-failure won't be using the same types as things using new-failure" ...I am probably completely ignorant of how this sort of thing actually works, and if so I will apologise and continue to watch and learn things — but could you instead of the dtolnay trick, do something like making new-failure import the type from old-failure and impl From<old-failure::Error> for Error?

@withoutboats
Copy link
Contributor

I think coherence won't let us provide that impl. That would solve the most annoying problems but not all of them, e.g. if Error is a part of the signature of a trait method and its being implemented by another crate using the other Error type.

Currently grepping the source of all reverse dependencies to find out if anyone actually uses downcast_mut ever.

@withoutboats
Copy link
Contributor

Actually, nevermind. This would also invalidate downcast (by value). I don't want to give up the ability to turn an Error into a concrete value again.

In theory, Clone will some day be object-safe using alloca. Unfortunately, we can't then add Clone as a supertrait on Fail without breaking everything. So I think Error will never be Clone.

Fortunately, there are several work arounds:

  • Rc<Error> and Arc<Error>.
  • Using your own type that implements Clone and Fail.

@c410-f3r
Copy link
Author

c410-f3r commented Feb 4, 2018

Thanks @withoutboats for the great explanation! The Arc and Rc approach is a nice workaround for this problem.

@c410-f3r c410-f3r closed this as completed Feb 4, 2018
hdevalence added a commit to ZcashFoundation/zebra that referenced this issue Oct 3, 2019
Unfortunately the generic `failure::Error` type is not `Clone` because
of its internals: rust-lang-deprecated/failure#148

The fix is to either switch to `Rc<Error>` or `Arc<Error>`, or to use a
custom `Fail` type.  In this case using a custom `Fail` might be best
but this commit just stubs out references to e.clone() until then.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants