rustler icon indicating copy to clipboard operation
rustler copied to clipboard

Result behaviour difference

Open thomas9911 opened this issue 3 years ago • 3 comments

I was testing some error handling in Rustler and I found some odd behaviour, is this expected?

#[rustler::nif]
fn test1() -> Result<u32, ()>{
    Ok(1)
}

#[rustler::nif]
fn test2() -> rustler::NifResult<u32>{
    Ok(2)
}

if I call these functions from elixir:

iex(15)> MyModule.test1()
{:ok, 1}
iex(16)> MyModule.test2()
2

The first one is what I expect that is returned from a Result. But the second one unwraps the :ok case. (on the error both return an error tuple)

Elixir: 1.10.4 OTP: 22.3 Rust: 1.52.1 Rustler: 0.22.0

thomas9911 avatar Sep 02 '21 09:09 thomas9911

Thanks for the report! I can reproduce this on master (662e36b):

diff --git a/rustler_tests/lib/rustler_test.ex b/rustler_tests/lib/rustler_test.ex
index ca25f37..191b521 100644
--- a/rustler_tests/lib/rustler_test.ex
+++ b/rustler_tests/lib/rustler_test.ex
@@ -83,4 +83,6 @@ def raise_term_with_atom_error(), do: err()
   def term_with_tuple_error(), do: err()
 
   def nif_attrs_can_rename(), do: err()
+
+  def test_result(), do: err()
 end
diff --git a/rustler_tests/native/rustler_test/src/lib.rs b/rustler_tests/native/rustler_test/src/lib.rs
index 046363d..025bcf8 100644
--- a/rustler_tests/native/rustler_test/src/lib.rs
+++ b/rustler_tests/native/rustler_test/src/lib.rs
@@ -39,6 +39,7 @@
         test_atom::atom_equals_ok,
         test_atom::binary_to_atom,
         test_atom::binary_to_existing_atom,
+        test_atom::test_result,
         test_binary::make_shorter_subbinary,
         test_binary::parse_integer,
         test_binary::binary_new,
diff --git a/rustler_tests/native/rustler_test/src/test_atom.rs b/rustler_tests/native/rustler_test/src/test_atom.rs
index 5cdd64f..80fb311 100644
--- a/rustler_tests/native/rustler_test/src/test_atom.rs
+++ b/rustler_tests/native/rustler_test/src/test_atom.rs
@@ -9,6 +9,11 @@ pub fn atom_to_string(atom: Term) -> NifResult<String> {
     atom.atom_to_string()
 }
 
+#[rustler::nif]
+pub fn test_result() -> NifResult<u8> {
+    Ok(1)
+}
+
 #[rustler::nif]
 pub fn atom_equals_ok(atom: Atom) -> bool {
     atoms::ok() == atom
diff --git a/rustler_tests/test/atom_test.exs b/rustler_tests/test/atom_test.exs
index 72e1e01..578450f 100644
--- a/rustler_tests/test/atom_test.exs
+++ b/rustler_tests/test/atom_test.exs
@@ -26,4 +26,8 @@ test "atom equals ok" do
     refute RustlerTest.atom_equals_ok(:fish)
     assert catch_error(RustlerTest.atom_equals_ok("ok")) == :badarg
   end
+
+  test "result" do
+    assert {:ok, 1} == RustlerTest.test_result()
+  end
 end
$ mix test test/atom_test.exs:31
  1) test result (RustlerTest.AtomTest)
     test/atom_test.exs:30
     Assertion with == failed
     code:  assert {:ok, 1} == RustlerTest.test_result()
     left:  {:ok, 1}
     right: 1
     stacktrace:
       test/atom_test.exs:31: (test)



Finished in 0.07 seconds (0.07s async, 0.00s sync)
6 tests, 1 failure, 5 excluded

evnu avatar Sep 02 '21 10:09 evnu

IIUC, this happens due to how NifReturnable is implemented. For Result<u8, ()>, the non-unwrapping impl is used, while for NifResult<T>, the unwrapping impl (because the E in Result<T, E> is set to crate::error::Error for the type alias NifResult).

I am not sure when one would use NifResult instead of Result, maybe somebody can clarify this?

evnu avatar Sep 02 '21 10:09 evnu

NifResult does not map exactly to generic Result objects as its Error type can be used to also raise exceptions. It should probably be an independent enum (essentially Return(T), RaiseTerm(U), BadArg...) instead of a typedef.

filmor avatar Sep 02 '21 11:09 filmor

Closing, feel free to reopen if this was not sufficiently answered.

evnu avatar Nov 11 '22 10:11 evnu