Unable to handle null return types from Rust

[dansteren]

Describe the bug
When using the JS agent to make requests to Rust canisters methods that return null, the agent throws the error: Wrong number of return values.

To Reproduce

1. Create a new rust canister using dfx: dfx new --type=rust actor_test
2. Enter the directory: cd actor_test
3. Update the canister's lib.rs file to contain one method that returns the unit type (which according to the candid reference is the equivalent of motoko's Null type):

   #[ic_cdk_macros::query]
   fn get_null() -> () {
       return ();
   }

4. Update the candid file to match:

   service : {
       "get_null": () -> (null) query;
   }
   

5. Start up the replica: dfx start
6. Deploy the canister: dfx deploy
7. Open the generated candid UI website in your browser (likely at http://127.0.0.1:8000/?canisterId=r7inp-6aaaa-aaaaa-aaabq-cai&id=rrkah-fqaaa-aaaaa-aaaaq-cai)
8. Open the dev tools to see the console output
9. Click the "QUERY" button in the UI to initiate a call to the get_null canister method
10. Note that the error message "Wrong number of return values" is displayed in the UI and in the console.
11. Make the equivalent call using DFX and note that there aren't any problems: dfx canister call actor_test get_null => (null: null)

Expected behavior
The agent should handle the null return value instead of throwing an error, and both the call and the result (null) should be displayed in the output log in the Candid UI.

Screenshots

Desktop (please complete the following information):

• OS: Ubuntu 21.10
• Browser: Brave
• Version: Version 1.39.111 Chromium: 102.0.5005.61 (Official Build) (64-bit)

Additional context
This is only a problem for Rust and Azle canisters. The agent handles null responses from Motoko canisters without problem. For example, consider the following Motoko canister:

actor ActorTest {
  public query func get_null(): async Null {
    return null;
  };
}

The candid file for both this motoko canister and the provided Rust canister are the same, namely:

service : {
  "get_null": () -> (null) query;
}

Therefore, it should be that the the agent can handle both canisters the same. However, only the null value being returned by the Motoko canister is correctly handled by the JS Agent.

Also worth noting is that although the JS Agent doesn't handle the Rust return values, both canisters return (null: null) when executing dfx canister call agent_test get_null from the command line.

[chenyan-dfinity]

This is expected. Rust type () -> () maps to () -> () in Candid. (_: ()) -> (()) in Rust maps to (null) -> (null) in Candid.

[lastmjs]

Hey, we're trying to understand what you mean here. Rust is warning us that (()) is equivalent to (), and when we run didc bind -t rs on the following Candid:

service: {
    "getNull": () -> (null) query;
    "printNull": (null) -> (null) query;
}

it produces this Rust:

// This is an experimental feature to generate Rust binding from Candid.
// You may want to manually adjust some of the types.
use ic_cdk::export::candid::{self, CandidType, Deserialize};
use ic_cdk::api::call::CallResult;

struct SERVICE(candid::Principal);
impl SERVICE{
  pub async fn getNull(&self) -> CallResult<((),)> {
    ic_cdk::call(self.0, "getNull", ()).await
  }
  pub async fn printNull(&self, arg0: ()) -> CallResult<((),)> {
    ic_cdk::call(self.0, "printNull", (arg0,)).await
  }
}

null in parameters and return types is represented in the generated Rust code as () and not as (()).

Furthermore, when we use dfx canister call on our getNull function the behavior changes based on what the Candid file says, we've tried "getNull": () -> (null) query; and we get (null: null) and when we try "getNull": () -> () query; we get ().

To me I'm wondering if dfx canister call is working correctly, but agent-js is broken in this case.

[lastmjs]

Actually...we just tried (()) with agent-js and it did work...so we're just trying to understand how/why this is. The generated didc bind -t rs code seems incorrect.

[lastmjs]

Perhaps didc bind -t rs is just incorrect when it generates values for null, or perhaps the ic_cdk_macros are wrong or just strange because in Rust (()) and () are equivalent and we get warnings for it, we aren't sure. We've expanded the simple getNull function with cargo expand and there is indeed a difference between returning () and (()):

#[ic_cdk_macros::query]
fn get_null() -> () {}

cargo expands to:

#![feature(prelude_import)]
#[prelude_import]
use std::prelude::rust_2018::*;
#[macro_use]
extern crate std;
#[export_name = "canister_query get_null"]
fn get_null_0_() {
    ic_cdk::setup();
    ic_cdk::spawn(async {
        let () = ic_cdk::api::call::arg_data();
        let result = get_null();
        ic_cdk::api::call::reply(())
    });
}
fn get_null() -> () {}

#[ic_cdk_macros::query]
fn get_null() -> (()) {}

cargo expands to:

#![feature(prelude_import)]
#[prelude_import]
use std::prelude::rust_2018::*;
#[macro_use]
extern crate std;
#[export_name = "canister_query get_null"]
fn get_null_0_() {
    ic_cdk::setup();
    ic_cdk::spawn(async {
        let () = ic_cdk::api::call::arg_data();
        let result = get_null();
        ic_cdk::api::call::reply((result,))
    });
}
fn get_null() -> (()) {}

ic_cdk::api::call::reply's tuple argument has one argument when the return type is (()) and has zero arguments when the return type is ().

[lastmjs]

Perhaps instead of this (()) vs () there should be a candid::types::Null? Similar to the Candid-specific Nat, Int, Reserved etc?

[lastmjs]

Sorry, we keep figuring more things out that you probably already knew.

This candid:

service: {
    "getNull": () -> () query;
}

generates this Rust with didc bind -ts rs:

// This is an experimental feature to generate Rust binding from Candid.
// You may want to manually adjust some of the types.
use ic_cdk::export::candid::{self, CandidType, Deserialize};
use ic_cdk::api::call::CallResult;

struct SERVICE(candid::Principal);
impl SERVICE{
  pub async fn getNull(&self) -> CallResult<()> {
    ic_cdk::call(self.0, "getNull", ()).await
  }
}

And this Candid:

service: {
    "getNull": () -> (null) query;
}

generates this Rust with didc bind -ts rs:

// This is an experimental feature to generate Rust binding from Candid.
// You may want to manually adjust some of the types.
use ic_cdk::export::candid::{self, CandidType, Deserialize};
use ic_cdk::api::call::CallResult;

struct SERVICE(candid::Principal);
impl SERVICE{
  pub async fn getNull(&self) -> CallResult<((),)> {
    ic_cdk::call(self.0, "getNull", ()).await
  }
}

So there is a differentiation that we weren't taking into account. This just seems strange to us, so maybe you can enlighten us. I do wonder if a candid::types::Null wouldn't be a good idea.

[chenyan-dfinity]

You are right. null as a single return value is probably not expressible in Rust at the moment. But you can return multiple values which contain a null value, e.g., () -> (u8, (), Vec<()>, ()) maps to () -> (nat8, null, vec null, null) in Candid.

Some facts:

• Rust type () maps to null in Candid type.
• If the return type in Rust is a n-tuple, it means the function has n return values in Candid.
• If the return type T in Rust is not a tuple, it means a single return type, represented as (candid(T)) in Candid.

Therefore, when we see a return type () in Rust, it has two interpretations: 1) the function has zero return value, thus () in Candid; 2) the function returns a single value of null type, which is ((),). The first interpretation is way more popular than the second. In fact, why would someone return a single null value instead of an empty value? To resolve this, we need to define candid::types::Null. But how useful is a single null return value anyway?

[krpeacock]

@dansteren do you consider this issue resolved?

[lastmjs]

I'm not sure this is resolved, this situation has given us quite a bit of confusion while developing Azle. If the real answer is developing candid::types::Null, then perhaps we should pursue that course.

[dansteren]

@krpeacock we were able to work around this as soon as we understood the syntax you guys require. But I agree with @lastmjs, the syntax was quite confusing and not intuitive. Given that there is already a precedence for other candid types such as Empty and Required, I also think it would be best if we pursued creating a candid::types::Null as well. So, this isn't a blocker per say, but I wouldn't say it's resolved either.