dfinity / candid

Candid Library for the Internet Computer
Apache License 2.0
282 stars 79 forks source link

internal error: entered unreachable code #242

Closed paulyoung closed 3 years ago

paulyoung commented 3 years ago

As mentioned in https://github.com/dfinity/agent-rs/issues/201#issuecomment-855175908, I have a canister written in Rust like this:

use ic_cdk_macros::{query};
use ic_utils_http_request::{HttpResponse, HttpRequest};

#[query]
fn http_request(request: HttpRequest) -> HttpResponse {
    HttpResponse {
        status_code: 200,
        headers: vec![],
        body: request.body.to_vec(),
        streaming_strategy: None,
    }
}

However, when I try to make a GET request via icx-proxy I get a 500 response and see this in the output from dfx start:

[Canister rrkah-fqaaa-aaaaa-aaaaq-cai] Panicked at 'internal error: entered unreachable code', /Users/py/.cargo/registry/src/github.com-1ecc6299db9ec823/candid-0.6.21/src/parser/value.rs:326:9

I started icx-proxy with -v -v so I see the GET request being logged.

I tried to curl the proxy instead and got Replica Error (5): "IC0502: Canister rrkah-fqaaa-aaaaa-aaaaq-cai trapped: unreachable"

I did this in Motoko first as part of testing https://github.com/dfinity/agent-rs/pull/195 and didn't run into this.

Could someone help me understand if I'm doing something wrong here? Thanks.

paulyoung commented 3 years ago

From looking at https://docs.rs/crate/candid/0.6.21/source/src/parser/value.rs I suspect this might originate from: https://github.com/dfinity/candid/blob/03d6d82fdd7e6c9012d2cea19f2b6b1a9c5ba0c3/rust/candid/src/parser/value.rs#L328-L330

paulyoung commented 3 years ago

I see the latest version of candid is 0.7.0. Is there some mismatch because I'm using dfx version 0.7.0-beta.8 but ic-cdk version 0.3.0 exports candid version 0.6.21?

chenyan-dfinity commented 3 years ago

Looks fine on the candid side. The error is due to calling IDLValue::ty(), which is not allowed. Because you cannot infer candid types from IDLValue.

Maybe related to https://github.com/dfinity/agent-rs/issues/132, but I was doing a similar thing in Rust and did not see this error: https://github.com/dfinity/candid/blob/master/tools/ui/src/didjs/lib.rs#L50

paulyoung commented 3 years ago

@chenyan-dfinity is it possible I need to add candid_method and export_service?

I tried that before but I couldn't find any documentation. I gather they're related to generating a .did file but I don't understand how they're intended to be used.

I still have a .did file around from the hello world tutorial. Could that be causing problems?

paulyoung commented 3 years ago

I just removed the streaming_strategy field from the definition of HttpResponse that I copied from ic-utils and it works fine now.

Based on what you said, it seems like the issue is because it's defined like this, and uses IDLValue:


#[derive(CandidType, Deserialize)]
pub struct CallbackStrategy {
    pub callback: IDLValue,
    pub token: IDLValue,
}

#[derive(CandidType, Deserialize)]
pub enum StreamingStrategy {
    Callback(CallbackStrategy),
}

#[derive(CandidType, Deserialize)]
pub struct HttpResponse {
    pub status_code: u16,
    pub headers: Vec<HeaderField>,
    #[serde(with = "serde_bytes")]
    pub body: Vec<u8>,
    pub streaming_strategy: Option<StreamingStrategy>,
}

#[derive(CandidType, Deserialize)]
pub struct StreamingCallbackHttpResponse {
    #[serde(with = "serde_bytes")]
    pub body: Vec<u8>,
    pub token: Option<IDLValue>,
}
chenyan-dfinity commented 3 years ago

Right, pub token: Option<IDLValue> was the problem, and CDK cannot handle it properly. We need to change the token type to Option<candid::Func> in agent-rs.

add candid_method and export_service

Yeah, that is only used for auto-generating did files, and not related to this error. We are still trying to figure out the best way to integrate this into the CDK. For now, the best document is probably the test: https://github.com/dfinity/candid/blob/master/rust/candid/tests/types.rs#L98

paulyoung commented 3 years ago

Thanks @chenyan-dfinity!

paulyoung commented 3 years ago

pub token: Option<IDLValue> was the problem

token and callback I think

paulyoung commented 3 years ago

I think StreamingCallbackHttpResponse isn't relevant here, and including it may have confused things.