Add impl<E: std::error::Error> From<E> for crate::Error

[yutannihilation]

Close #305

This pull request makes conversion from other error to savvy::Error easily, just like anyhow. This is a breaking change, so this bumps the version.

Pros

To put simply, such .map_err() will no longer be needed.

Before

fn foo() -> savvy::Result<()> {
    let value = mutex
        .lock()
        .map_err(|e| crate::Error::new(&e.to_string()))?;
    ...
}

After

fn foo() -> savvy::Result<()> {
    let value = mutex
        .lock()?;
    ...
}

Cons

On the other hand, savvy::Error loses From<String> and From<&str>. If savvy has these impl, it conflicts with the error conversion. I for got what this error means, but, considering the fact that anyhow::Error doesn't have From<String> as well, this is probably unavoidable.

error[E0119]: conflicting implementations of trait `std::convert::From<std::string::String>` for type `error::Error`
  --> src\error.rs:75:1
   |
55 |   impl From<String> for Error {
   |   --------------------------- first implementation here
...
75 | / impl<E> From<E> for Error
76 | | where
77 | |     E: std::error::Error + 'static,
   | |___________________________________^ conflicting implementation for `error::Error`
   |
   = note: upstream crates may add a new impl of trait `std::error::Error` for type `std::string::String` in future versions

As a workaround, savvy provides savvy_err!() macro, which is a shorthand of savvy::Error::new(format!(...)). This follows the idea of anyhow!() macro.

So, if the current implementation uses .into() conversion from a string, it needs to replaced with savvy_err!().

Before

    Err("Not a known ALTREP".into())
}

After

    Err(savvy_err!("Not a known ALTREP"))
}

Minor technical note

anyhow::Error requires Send and Sync (so, the above code of Mutex doesn't work for anyhow::Error). If I understand correctly, this is because

• anyhow needs to be async-aware (cf. Why does Error require Send (and Sync)? dtolnay/anyhow#81)
• anyhow preserves the original implementation for std::error::Error by accessing vtable directly. So, if the origin is not Send or Sync, anyhow::Error is the same.

On the other hand, savvy creates a string immediately on conversion. Because only a string can be propagated to R session anyway. A string is Send and Sync no matter whether the original error is Send and/or Sync or not. So, savvy's conversion doesn't require Send and Sync.

[yutannihilation]

After experimenting a bit, I found this is not a good idea. This makes it impossible to implement custom From<T> for savvy::Error. Closing.

   error[E0119]: conflicting implementations of trait `From<RPolarsErr>` for type `savvy::Error`
     --> src\error.rs:27:1
      |
   27 | impl From<RPolarsErr> for savvy::Error {
      | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      |
      = note: conflicting implementation in crate `savvy`:
              - impl<E> From<E> for savvy::Error
                where E: StdError, E: 'static;

[yutannihilation]

Hmm, I'm closing this again.

To mimic anyhow, savvy::Error probably needs to implement impl AsRef<dyn StdError> for T instead of impl StdError for T.

https://docs.rs/anyhow/latest/anyhow/struct.Error.html#impl-AsRef%3Cdyn+Error+%2B+Send+%2B+Sync%3E-for-Error

Maybe it might be easier to actually use anyhow....?

[yutannihilation]

Maybe it might be easier to actually use anyhow....?

This isn't possible because savvy::Error::Abort(SEXP) needs to be thrown, and it's not Send or Sync.