trouble mapping TryFrom errors

[cataggar]

I'd like to be able to have the error be the source and have a formatted message.

impl TryFrom<u16> for StatusCode {
    type Error = Error;
    fn try_from(value: u16) -> crate::Result<Self> {
        Ok(StatusCode(
            http_types::StatusCode::try_from(value).with_context(ErrorKind::DataConversion, || {
                format!("StatusCode::try_from failed for '{value}'")
            }),
        ))
    }
}

results in this error:

error[E0599]: the method `with_context` exists for enum `std::result::Result<http_types::StatusCode, http_types::Error>`, but its trait bounds were not satisfied
   --> sdk\core\src\status_code.rs:50:53
    |
50  |             http_types::StatusCode::try_from(value).with_context(ErrorKind::DataConversion, || {
    |                                                     ^^^^^^^^^^^^ method cannot be called on `std::result::Result<http_types::StatusCode, http_types::Error>` due to unsatisfied trait bounds
    |
   ::: C:\Users\cataggar\.rustup\toolchains\stable-x86_64-pc-windows-msvc\lib/rustlib/src/rust\library\core\src\result.rs:504:1
    |
504 | pub enum Result<T, E> {
    | --------------------- doesn't satisfy `_: error::ResultExt<http_types::StatusCode>`
    |
   ::: C:\Users\cataggar\.cargo\registry\src\github.com-1ecc6299db9ec823\http-types-2.12.0\src\error.rs:16:1
    |
16  | pub struct Error {
    | ---------------- doesn't satisfy `http_types::Error: StdError`
    |
note: trait bound `http_types::Error: StdError` was not satisfied
   --> sdk\core\src\error\mod.rs:285:8
    |
283 | impl<T, E> ResultExt<T> for std::result::Result<T, E>
    |            ------------     -------------------------
284 | where
285 |     E: std::error::Error + Send + Sync + 'static,
    |        ^^^^^^^^^^^^^^^^^ unsatisfied trait bound introduced here

---

This works, but full does not accept a formatted message. Should a Error::with_full be added?

impl TryFrom<u16> for StatusCode {
    type Error = Error;
    fn try_from(value: u16) -> crate::Result<Self> {
        Ok(StatusCode(
            http_types::StatusCode::try_from(value).map_err(|error| {
                Error::full(
                    ErrorKind::DataConversion,
                    error,
                    "StatusCode::try_from failed",
                )
            })?,
        ))
    }
}

[rylev]

I'm surprised that http_types does not implement std::error::Error (cc @yoshuawuyts).

Error::full should accept a formatted message. It takes a Cow<'static, str> which can either be a static string slice or an owned String.

We do this in other places in the code base:

 Error::full(
     ErrorKind::DataConversion,
     e,
     format!(
        "failed to parse date '{}' with format string {:?}",
        date, fmt
      ),
 )

[yoshuawuyts]

I'm surprised that http_types does not implement std::error::Error (cc @yoshuawuyts).

Yeah, that's a big annoyance - and is something we should fix on the http-types side tbh. What's going on is that http_types::Error implements anyhow-like semantics, which is a fancy version of Box<dyn Error + Send + Sync + 'static>. But that in itself doesn't implement Error (for type-system reasons that escape me), so neither can that.

To be honest, we should probably expose more clearly enumerated errors from http-types, and make the catch-all error types either a separate construct - or bubble it up to the framework level instead.

If we're going to be wrapping http_types::Error errors with our own, we can probably provide a conversion to get from one into the other. And I can then work upstream to migrate the http-types errors to a version which doesn't suffer from these issues for the next major release. How does that sound?

edit: In fact http-rs/http-types#395 proposes exactly that, which imo we should do.