Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support wrapping error_chain Error's ? / document that Send+Sync are required on top of std::error::Error #72

Closed
rbtcollins opened this issue Mar 15, 2020 · 4 comments

Comments

@rbtcollins
Copy link

rbtcollins commented Mar 15, 2020

Anyhow claims to support wrapping any implementor of std::error::Error, but error_chain's impl seems incompatible:

error[E0277]: `(dyn std::error::Error + std::marker::Send + 'static)` cannot be shared between threads safely
  --> src/cli/main.rs:66:25
   |
66 |     utils::current_dir()?;
   |                         ^ `(dyn std::error::Error + std::marker::Send + 'static)` cannot be shared between threads safely
   |
   = help: the trait `std::marker::Sync` is not implemented for `(dyn std::error::Error + std::marker::Send + 'static)`
   = note: required because of the requirements on the impl of `std::marker::Sync` for `std::ptr::Unique<(dyn std::error::Error + std::marker::Send + 'static)>`
   = note: required because it appears within the type `std::boxed::Box<(dyn std::error::Error + std::marker::Send + 'static)>`
   = note: required because it appears within the type `std::option::Option<std::boxed::Box<(dyn std::error::Error + std::marker::Send + 'static)>>`
   = note: required because it appears within the type `error_chain::State`
   = note: required because it appears within the type `rustup::errors::Error`
   = note: required because of the requirements on the impl of `std::convert::From<rustup::errors::Error>` for `anyhow::Error`
   = note: required by `std::convert::From::from`

rust-lang-deprecated/error-chain#240 is presumably the underlying cause, but perhaps anyhow could do something on it's end via specialisation or using ?Send and ?Sync or something?

@dtolnay
Copy link
Owner

dtolnay commented Mar 15, 2020

I would prefer not to make a change for this; my best recommendation would be to not use error-chain. Non-sync errors are extremely unusual and it's a bug in error-chain that their errors are not sync. They decided more than 3 years ago to fix it and make sync errors but the library is not maintained enough to actually make that happen.

@dtolnay dtolnay closed this as completed Mar 15, 2020
@rbtcollins
Copy link
Author

rbtcollins commented Mar 15, 2020

So the issue is how to deal with it in two cases; for migrating a big-patch approach can be done, which is painful but fine. But for using anyhow with some other library that does use error-chain this then becomes a transitive problem. Similar bug was filed on failure rust-lang-deprecated/failure#284 :/. I agree with you that not using error-chain is ideal, but providing some migration path off of error-chain is desirable no? Right now if there are two crates A and B that both use error chain, and one uses the other, neither of them can convert to anyhow, because error-chain cannot convert from anyhow, and anyhow cannot convert from error-chain.

... or maybe using failure.SyncFailure is the thing to do?

@dtolnay
Copy link
Owner

dtolnay commented Mar 15, 2020

For migrating, they should patch in the PR from 3 years ago that makes the error-chain errors Sync. ;)

@rbtcollins
Copy link
Author

OH! rust-lang-deprecated/error-chain#241 ta. Now to figure out the process to make that happen :).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants