frozenlib/cancellation-token-rs

Make Cancel error compatible with anyhow

Closed this issue · 7 comments

Make Cancel error compatible with anyhow: https://crates.io/crates/anyhow

::anyhow::anyhow!(::cancellation_token::Canceled)
error[E0599]: the method `anyhow_kind` exists for reference `&Canceled`, but its trait bounds were not satisfied
  --> dest/src/utils/mod.rs:28:5
   |
28 |     ::anyhow::anyhow!(::cancellation_token::Canceled)
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ method cannot be called on `&Canceled` due to unsatisfied trait bounds
  --> .cargo/registry/src/index.crates.io-6f17d22bba15001f/cancellation-token-0.1.0/src/lib.rs:434:1
   |
   = note: doesn't satisfy `Canceled: Into<anyhow::Error>`
   |
   = note: doesn't satisfy `Canceled: anyhow::kind::TraitKind`
   |
   = note: doesn't satisfy `Canceled: std::fmt::Display`
   |
   = note: the following trait bounds were not satisfied:
           `Canceled: Into<anyhow::Error>`
           which is required by `Canceled: anyhow::kind::TraitKind`
           `Canceled: std::fmt::Display`
           which is required by `&Canceled: anyhow::kind::AdhocKind`
           `&Canceled: Into<anyhow::Error>`
           which is required by `&Canceled: anyhow::kind::TraitKind`
   = note: this error originates in the macro `::anyhow::anyhow` (in Nightly builds, run with -Z macro-backtrace for more info)

For more information about this error, try `rustc --explain E0599`.```

something like this from std::sync::mpsc::RecvError:

#[stable(feature = "rust1", since = "1.0.0")]
impl fmt::Display for RecvError {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        "receiving on a closed channel".fmt(f)
    }
}

#[stable(feature = "rust1", since = "1.0.0")]
impl error::Error for RecvError {
    #[allow(deprecated)]
    fn description(&self) -> &str {
        "receiving on a closed channel"
    }
}

I'm currently considering whether Canceled should implement Error.

In the .NET Framework, which inspired this crate, cancellation is represented as OperationCanceledException and is treated the same as an error, but I believe this is due to implementation convenience.

If representing the outcome of a process that might result in cancellation or an error as anyhow::Result<T> is considered best practice, then I think Canceled should implement Error. However, if it's best practice to represent it as MayBeCanceled<anyhow::Result<T>>, then implementing Error for Canceled could lead to mistakes, so it should not be implemented in that case.

I have not yet determined which is the best practice and would like to make a decision based on use cases.

That's up to the designer of the API that uses the cancellation token. She can do both if Canceled implements Error. If you read https://crates.io/crates/anyhow it is meant typical for binary that doesn't want to have explicit error types; for convenience and simple error logging. Whereas for bespoke explicit errors typically for library interfaces is different crate https://crates.io/crates/thiserror .

As a Cancellation token library shouldn't limit how it will be used. Implementing Error is a good practice it doesn't limit any means of usage.

MayBeCanceled<anyhow::Result<T>> doesn't make any sense that defeats the purpose of anyhow however MayBeCanceled<Result<(),MySpecificError>> makes a lot of sense. This option is not limited when Canceled implements Error trait

plus you might add extra metadata to the Canceled error through the Error trait, like the source or reason for cancellation although that might be an overkill

or you can even do it as conditional compilation through feature, like by default off feature anyhow that people who want it can turn it on

#3

That's up to the designer of the API that uses the cancellation token. She can do both if Canceled implements Error. If you read https://crates.io/crates/anyhow it is meant typical for binary that doesn't want to have explicit error types; for convenience and simple error logging. Whereas for bespoke explicit errors typically for library interfaces is different crate https://crates.io/crates/thiserror .

As a Cancellation token library shouldn't limit how it will be used. Implementing Error is a good practice it doesn't limit any means of usage.

MayBeCanceled<anyhow::Result<T>> doesn't make any sense that defeats the purpose of anyhow however MayBeCanceled<Result<(),MySpecificError>> makes a lot of sense. This option is not limited when Canceled implements Error trait

Indeed, it looks like MayBeCanceled<anyhow::Result<T>> doesn't need to be considered. Then it seems that Canceled would be fine to implement Error.

plus you might add extra metadata to the Canceled error through the Error trait, like the source or reason for cancellation although that might be an overkill

I agree with this as well.
If it were to be like .NET, it would be natural to have those features. However, since I cannot think of a scenario where they would be used, I believe it's better to implement them when they are actually needed.

or you can even do it as conditional compilation through feature, like by default off feature anyhow that people who want it can turn it on

Because of the complexity of the specification when using crate feature, I would like to avoid using it unless necessary. In this case, we can implement without it, so I do not think it is necessary to use it.

thanks