Elixir errors using Rust's `?`

[KoviRobi]

Hi,

First of all, Rustler is awesome, creating the NIF seems much simplified compared to C!

I was wondering about what would be the best practice for errors, in particular it would be nice to have errors using ?. I have put together a repo for it at https://github.com/KoviRobi/rustler_raise_error and I was wondering if there is an easier way to do it?

It's possible my whole use-case/assumptions are wrong, of course, and this isn't the right way to go about handling errors. What I wanted is an easy way to do errors that are semi user-understandable (not necessarily in normal English but a tech person should be able to figure it out). Here is what I got:

1. Using the {:ok, _}, {:error, _} paradigm

   iex(1)> NIF.getenv("FOO")
   {:error, "Variable not set: environment variable not found"}

   Note that this paradigm doesn't need the wrapper. I've just used one so that I can also reuse it for getenv!. But thinking about this, the whole thing wouldn't be so bad if we could reuse the function definition for the non-! version (currently it's impossible because the definition is hidden behind a structure).

2. Using the exceptions paradigm -- this also shows what advantage it might have over {:ok, _} = ... which is printing the arguments the function was called with.

   iex(2)> NIF.getenv!("FOO")
   ** (ErlangError) Erlang error: "Variable not set: environment variable not found"
      (rustler_raise_error 0.1.0) NIF.getenv!("FOO")
   iex(2)> {:ok, value} = NIF.getenv("FOO")
   ** (MatchError) no match of right hand side value: {:error, "Variable not set: environment variable not found"}

This also brought up the question, is there a way to have functions in Rustler that have ! in them? I tried doing

#[nif(name = "getenv!")]

but that didn't seem to work, it failed with

error: custom attribute panicked
  --> src/lib.rs:64:1
   |
64 | #[nif(name = "getenv!")]
   | ^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = help: message: `"getenv!"` is not a valid identifier

which seems like a bug? I've had a go at solving it in https://github.com/rusterlium/rustler/pull/505

[KoviRobi]

As an added bonus, I've also played around with phantom types to add context to the error, see the branch error-context-with-phantom-types or the difference between that branch and main. I haven't merged it as it's a small (but I think cool) distraction.

[KoviRobi]

Initially I was trying to get rid of the two wrappers, but I have had the realisation that if we have the pattern

fn _getenv(key: String) -> Result<String, MyError> {
    // ...
}

#[nif]
pub fn getenv(key: String) -> Result<String, MyError> {
    _getenv(key)
}

#[nif(name = "getenv!")]
pub fn getenv_bang(key: String) -> rustler::NifResult<String> {
    Ok(_getenv(key)?)
}

then we don't need to use the ?'s auto from behaviour, we could just have the wrappers call a given method to convert from whatever error type to the rustler::error::Error type. At which point we could have the nif macro generate the wrappers, e.g. the following could do something like the above?

#[nif(error = error_mapper, bang = raise_mapper)]
fn getenv(key: String) -> Result<String, Box<dyn Error>> {
    // ...
}

expanding to

#[nif]
pub fn test(raise: bool) -> rustler::NifResult<rustler::Atom> {
    _test(raise).map_err(error_mapper)
}

#[nif(name = "test!")]
pub fn test_bang(raise: bool) -> rustler::NifResult<rustler::Atom> {
    _test(raise).map_err(raise_mapper)
}

where error_mapper and raise_mapper would be defined by the user, but maybe this default implementation could be useful

fn error_mapper(error: Box<dyn Error>) -> rustler::Error {
    rustler::Error::Term(Box::new(format!("Error: {}", error)))
}
fn raise_mapper(error: Box<dyn Error>) -> rustler::Error {
    rustler::Error::RaiseTerm(Box::new(format!("Error: {}", error)))
}

[filmor]

To be honest, I don't see how this last part adds value. You can just as well add this mapping to the Elixir module, and the error mapping can be done using a straightforward mapping in the nif implementation. Raising or returning strings as errors is also not a good style, IMO.

[KoviRobi]

True, it could do that, and it's easy in Elixir to just do a compile-time for I guess. I agree about string errors not being great, but I just want to log unexpected cases -- I guess the normal way in Rust to tell the user something bad happened that the program can't recover from would be a panic. The problem is that panics don't get raised with enough info (with new-line clearing because erlang seems to set the TTY into CRLF newlines and rust just does LF?):

read '<unnamed>' panicked at 'called `Result::unwrap()` on an `Err` value: "Raising"', src/lib.rs:55:18
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
** (ErlangError) Erlang error: :nif_panicked
    (rustler_raise_error 0.1.0) NIF.test_panic(true)

where the erlang error is just :nif_panicked which isn't enough info (the panic is not captured), whereas DEVICE_NOT_FOUND is much better at telling the user that they just forgot to plug the device in.

But also the error mapper doesn't have to return strings of course :)

I guess just having an error_mapper not a raise_mapper could be useful though, because it just saves a lot of wrappers/manually having to make a function that does the map_error? For my use case I have 14 functions, some have long signatures like

fn _i2c_write_read<'a>(
    env: Env<'a>,
    i2c_wrapper: ResourceArc<DroppableDevice>,
    address: u8,
    write: Binary<'a>,
    read_len: usize,
) -> Result<Binary<'a>, Error<I2cWriteRead>> {

so duplicating this twice is annoying. The problem is, I cannot implement impl From<libftd2xx::FtStatus> for rustler::Error because neither are in my crate. If I could, then I wouldn't have to have the map_error

P.S.: I might be missing something obvious of course -- I'm new to Rust so feel free to tell me I'm just barking up the wrong tree

[KoviRobi]

Or if we could have an encode method on a custom, crate-local error type that could return RaiseTerm or RaiseAtom not just Term or Atom