Open zardini123 opened 5 months ago
I managed to get snafu's Whatever
to be sendable between threads by adding + Send + Sync
next to all instances of std::error::Error
in Whatever
definition. In addition, a test in snafu's test suite converts a Whatever
to a std::error::Error
, so adding the markers there are required too. The changes to make this work are available in my PR: #448
Snafu compiles and tests run on my machine with this. From my naive testing in one of my personal projects, it successfully allows for Whatever
's to be sent between threads.
I am curious to hear your thoughts. Thanks!
I'm not an expert in send and sync trait I often try to avoid them :p but I think sync is not needed here it's maybe too much requirement, I don't see a case where you would need to share a reference to a Whatever
in multiple thread.
I think the error here is solved with just send that is a perfectly reasonable constraint for a Whatever
error.
It is unclear whether the lack of Send + Sync
was just an oversight, or whether it was deliberately made in order to admit error types which might not be Send
and Sync
. On the side of caution, it would be nice to understand whether someone out there is using Whatever
to encapsulate causes which cannot be sent or shared between threads.
In any case, making the change now is a breaking change, so I will mark it as such in the pull request.
send that is a perfectly reasonable constraint for a
Whatever
error.
Ah, that is true. The error I experienced was only in relation to Send
, not Sync
. You bring up a great point @Stargateur, Send
certainly sounds more reasonable as a constraint than Send + Sync
.
On the side of caution, it would be nice to understand whether someone out there is using
Whatever
to encapsulate causes which cannot be sent or shared between threads.
Great point @Enet4. Would making the error type only Sync
make it any less of a breaking change?
In any case, making the change now is a breaking change, so I will mark it as such in the pull request.
Would there be a better method to allow users to enable Send
or Send + Sync
for Whatever
if they find they need it? Maybe a feature flag of sorts? Something like this could allow for backwards compatibility and full choice of error types while not forcing all users out of using whatever_context
in a threaded context.
Just chiming in to say that there are use cases where Sync
is also necessary. In our case, we needed a cloneable dyn Error
type (using contents as API return values and passing separately to logging system), but could not make all of our error types Clone
. So we are passing an Arc<dyn Error + Send + Sync>
around, instead of a Box<dyn Error + Clone + Send>
.
Hi @shepmaster! Thank you for the wonderful crate!
Problem
I have been switching my project to utilize
snafu
. Previously, I usedanyhow::Error
as a generic error type when I didn't understand how to implement error types. Now I am rewriting my project with proper error types, and in the meantime switching outanyhow::Error
withsnafu::Whatever
.When switching, I ran into a problem. I have a future being spawned via Tokio, where the future has a output type of
Result<(), snafu::Whatever>
. As this is concurrency, the rust compiler complains that thatWhatever
does not haveSend + Sync
markers for its source type. The compile error is pretty indirect about this issue. I have the error included at the bottom of this post. To figure out where this issue was coming from required a bunch of research, as shown below.Research
Searching "snafu thread saftey" on Google returns the project
http-whatever
, where they essentially implement a new Whatever but withSend + Sync
attached to theStdError
:https://github.com/bassmanitram/http-whatever/blob/af4fb2d672011e76a5746c27f31a2fdf65979326/src/lib.rs#L84-L93
Snafu's Whatever:
https://github.com/shepmaster/snafu/blob/073cc389dbdab8ac0dba7497377638f1170b7f89/src/lib.rs#L1557-L1569
I found a snafu issue from January 2022 that asks for implementation details for making a custom snafu error type be async compatible. From there in May 2022 a PR was merged that showed an error type with async/multi-threading as part of the tests. This is part of the tests, but not part of the examples, so finding this example required a bunch of searching as its not part of the doc examples.
In addition,
anyhow::Error
hasSync + Send
markers (source in anyhow code, source in anyhow docs).Question
Why isn't
Whatever
Send + Sync by default?I could implement a new Whatever as done by
http-whatever
and as shown in the snafu test. But, if we considersnafu::Whatever
to be a catch-all error type until the user can make more specific types, shouldn't we expect as a user that it can be used in all places including async and multi-threading? Is there a detail I am missing that makes Whatever not able to be Send + Sync compatible by default?Thank you!
Whatever Send + Sync error report