dfinity / candid

Candid Library for the Internet Computer
Apache License 2.0
277 stars 81 forks source link

export_service! macro should correctly name records wrapped in `ManualReply`s #375

Open dansteren opened 2 years ago

dansteren commented 2 years ago

Describe the bug The candid export service doesn't correctly handle canister methods with a return type of ManualReply. Instead of exporting the record type in the canister method signature, it exports a generic ManualReply type. Subsequent ManualReplys will be given incrementing names of format ManualReply_n where n increases for the amount of records.

To Reproduce Steps to reproduce the behavior:

  1. Create a Rust canister with the following contents:

    use ic_cdk::api::call::{self, ManualReply};
    
    #[derive(candid::CandidType)]
    struct User {
        id: String,
    }
    
    #[ic_cdk_macros::query]
    #[candid::candid_method(query)]
    async fn method() -> ManualReply<User> {
        let user = User {
            id: "a".to_string(),
        };
        call::reply((user,));
        ManualReply::empty()
    }
    
    candid::export_service!();
    #[ic_cdk_macros::query(name = "__get_candid_interface_tmp_hack")]
    fn export_candid() -> String {
        __export_service()
    }
    
    #[cfg(test)]
    mod tests {
        use super::*;
        #[test]
        fn write_candid_to_disk() {
            std::fs::write("test.did", export_candid()).unwrap();
        }
    }

    Full example can be found at https://github.com/dansteren/candid_export_bug

  2. Run cargo test

  3. Inspect the generated candid file at canisters/tests/test.did

  4. Notice that it contains a type ManualReply which doesn't have the same name as the User struct in canisters/test/src/lib.rs

    type ManualReply = record { id : text };
    service : { method : () -> (ManualReply) query }

Expected behavior The candid file should name the user type User not ManualReply. I.e.

type User = record { id : text };
service : { method : () -> (User) query }

Screenshots N/A

Platform

Additional context The problem occurs regardless of whether the function is a Query or an Update. The problem does not occur if another function also returns that type (See https://github.com/dansteren/candid_export_bug/tree/include_type_in_other_function_sig).

PS: Your Bug issue template isn't working so I was unable to apply the Bug label.

chenyan-dfinity commented 2 years ago

Have you tried this: https://github.com/dfinity/cdk-rs/blob/main/src/ic-cdk/src/api/call.rs#L636

paulyoung commented 2 years ago

I think @chenyan-dfinity was referring to #[query(manual_reply = true)] and #[update(manual_reply = true)]

I use these and my exported interface contains multiple ManualReply types.

However, it's not a problem for me because I'm not writing the exported service to disk; I'm comparing it with a hand-written file that's already on disk using service_compatible.

dansteren commented 1 year ago

@chenyan-dfinity I think I just accidentally left that attribute out somehow when making this issue, but yes, I am also using #[ic_cdk_macros::query(manual_reply = true)] (See https://github.com/dansteren/candid_export_bug/blob/main/canisters/test/src/lib.rs#L8).

This does not fix the problem. The candid file still lists the type as ManualReply instead of User.

dansteren commented 1 year ago

This seems to still be an issue in 0.9.0-beta.2. I just bumped all the dependencies in my reproducible to the latest available beta versions and am still experiencing this issue.