Skip to content

Commit

Permalink
chore: deprecate old call API (#542)
Browse files Browse the repository at this point in the history
* deprecated annotations with note

* update ic-ledger-types

* RejectCode and CallPerformErrorCode have NoError variant

* ic-cdk-timers migrate to Call::new

* fix deprecation usage in async e2e

* fix doctests
  • Loading branch information
lwshang authored Dec 20, 2024
1 parent e2bbc39 commit c2ba198
Show file tree
Hide file tree
Showing 7 changed files with 243 additions and 135 deletions.
57 changes: 33 additions & 24 deletions e2e-tests/src/bin/async.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use candid::Principal;
use ic_cdk::call::{Call, CallError, CallResult, SendableCall};
use ic_cdk::{query, update};
use lazy_static::lazy_static;
use std::sync::RwLock;
Expand Down Expand Up @@ -31,9 +32,11 @@ async fn panic_after_async() {
let value = *lock;
// Do not drop the lock before the await point.

let _: (u64,) = ic_cdk::call(ic_cdk::api::canister_self(), "inc", (value,))
let _: u64 = Call::new(ic_cdk::api::canister_self(), "inc")
.with_arg(value)
.call()
.await
.expect("failed to call self");
.unwrap();
ic_cdk::api::trap("Goodbye, cruel world.")
}

Expand All @@ -47,9 +50,10 @@ async fn panic_twice() {
}

async fn async_then_panic() {
let _: (u64,) = ic_cdk::call(ic_cdk::api::canister_self(), "on_notify", ())
let _: u64 = Call::new(ic_cdk::api::canister_self(), "on_notify")
.call()
.await
.expect("Failed to call self");
.unwrap();
panic!();
}

Expand All @@ -65,12 +69,14 @@ fn on_notify() {

#[update]
fn notify(whom: Principal, method: String) {
ic_cdk::notify(whom, method.as_str(), ()).unwrap_or_else(|reject| {
ic_cdk::api::trap(format!(
"failed to notify (callee={}, method={}): {:?}",
whom, method, reject
))
});
Call::new(whom, method.as_str())
.call_oneway()
.unwrap_or_else(|reject| {
ic_cdk::api::trap(format!(
"failed to notify (callee={}, method={}): {:?}",
whom, method, reject
))
});
}

#[query]
Expand All @@ -80,28 +86,31 @@ fn greet(name: String) -> String {

#[query(composite = true)]
async fn greet_self(greeter: Principal) -> String {
let (greeting,) = ic_cdk::api::call::call(greeter, "greet", ("myself",))
Call::new(greeter, "greet")
.with_arg("myself")
.call()
.await
.unwrap();
greeting
.unwrap()
}

#[update]
async fn invalid_reply_payload_does_not_trap() -> String {
// We're decoding an integer instead of a string, decoding must fail.
let result: Result<(u64,), _> = ic_cdk::call(
ic_cdk::api::canister_self(),
"greet",
("World".to_string(),),
)
.await;
let result: CallResult<u64> = Call::new(ic_cdk::api::canister_self(), "greet")
.with_arg("World")
.call()
.await;

match result {
Ok((_n,)) => ic_cdk::api::trap("expected the decoding to fail"),
Err((err_code, _)) => format!(
"handled decoding error gracefully with code {}",
err_code as i32
),
Ok(_) => ic_cdk::api::trap("expected the decoding to fail"),
Err(e) => match e {
CallError::CandidDecodeFailed(candid_err) => {
format!("handled decoding error gracefully with candid error: {candid_err}")
}
other_err => ic_cdk::api::trap(format!(
"expected a CandidDecodeFailed error, got {other_err}"
)),
},
}
}

Expand Down
27 changes: 12 additions & 15 deletions e2e-tests/tests/async.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
use pocket_ic::common::rest::RawEffectivePrincipal;
use pocket_ic::{call_candid, query_candid, CallError, ErrorCode, PocketIc};
use pocket_ic::{call_candid, query_candid, CallError, ErrorCode};

mod test_utilities;
use test_utilities::{cargo_build_canister, pocket_ic};

#[test]
fn panic_after_async_frees_resources() {
let pic: PocketIc = pocket_ic();
let pic = pocket_ic();
let wasm = cargo_build_canister("async");
let canister_id = pic.create_canister();
pic.add_cycles(canister_id, 2_000_000_000_000);
Expand Down Expand Up @@ -43,7 +43,7 @@ fn panic_after_async_frees_resources() {
"invocation_count",
(),
)
.expect("failed to call invocation_count");
.unwrap();

assert_eq!(i, n, "expected the invocation count to be {}, got {}", i, n);
}
Expand All @@ -55,8 +55,8 @@ fn panic_after_async_frees_resources() {
"invalid_reply_payload_does_not_trap",
(),
)
.expect("call failed");
assert_eq!(&message, "handled decoding error gracefully with code 5");
.unwrap();
assert!(message.contains("handled decoding error gracefully"));

let err = call_candid::<_, ()>(
&pic,
Expand All @@ -76,15 +76,15 @@ fn panic_after_async_frees_resources() {
"notifications_received",
(),
)
.expect("failed to call unrelated function afterwards");
.unwrap();
let _: (u64,) = call_candid(
&pic,
canister_id,
RawEffectivePrincipal::None,
"invocation_count",
(),
)
.expect("failed to recover lock");
.unwrap();
}

#[test]
Expand All @@ -98,8 +98,7 @@ fn notify_calls() {
pic.add_cycles(receiver_id, 2_000_000_000_000);
pic.install_canister(receiver_id, wasm, vec![], None);

let (n,): (u64,) = query_candid(&pic, receiver_id, "notifications_received", ())
.expect("failed to query 'notifications_received'");
let (n,): (u64,) = query_candid(&pic, receiver_id, "notifications_received", ()).unwrap();
assert_eq!(n, 0);

let () = call_candid(
Expand All @@ -109,14 +108,12 @@ fn notify_calls() {
"notify",
(receiver_id, "on_notify"),
)
.expect("failed to call 'notify'");
.unwrap();

let (n,): (u64,) = query_candid(&pic, receiver_id, "notifications_received", ())
.expect("failed to query 'notifications_received'");
let (n,): (u64,) = query_candid(&pic, receiver_id, "notifications_received", ()).unwrap();
assert_eq!(n, 1);
}

// Composite queries are not enabled yet.
#[test]
fn test_composite_query() {
let pic = pocket_ic();
Expand All @@ -128,7 +125,7 @@ fn test_composite_query() {
pic.add_cycles(receiver_id, 2_000_000_000_000);
pic.install_canister(receiver_id, wasm, vec![], None);

let (greeting,): (String,) = query_candid(&pic, sender_id, "greet_self", (receiver_id,))
.expect("failed to query 'greet_self'");
let (greeting,): (String,) =
query_candid(&pic, sender_id, "greet_self", (receiver_id,)).unwrap();
assert_eq!(greeting, "Hello, myself");
}
46 changes: 26 additions & 20 deletions ic-cdk-timers/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use std::{
use futures::{stream::FuturesUnordered, StreamExt};
use slotmap::{new_key_type, KeyData, SlotMap};

use ic_cdk::api::call::RejectionCode;
use ic_cdk::call::{Call, CallError, CallPerformErrorCode, RejectCode, SendableCall};

// To ensure that tasks are removable seamlessly, there are two separate concepts here: tasks, for the actual function being called,
// and timers, the scheduled execution of tasks. As this is an implementation detail, this does not affect the exported name TimerId,
Expand Down Expand Up @@ -113,11 +113,12 @@ extern "C" fn global_timer() {
call_futures.push(async move {
(
timer,
ic_cdk::call(
Call::new(
ic_cdk::api::canister_self(),
"<ic-cdk internal> timer_executor",
(task_id.0.as_ffi(),),
)
.with_arg(task_id.0.as_ffi())
.call::<()>()
.await,
)
});
Expand All @@ -131,25 +132,30 @@ extern "C" fn global_timer() {
// run all the collected tasks, and clean up after them if necessary
while let Some((timer, res)) = call_futures.next().await {
let task_id = timer.task;
match res {
Ok(()) => {}
Err((code, msg)) => {
ic_cdk::println!("in canister_global_timer: {code:?}: {msg}");
match code {
RejectionCode::SysTransient => {
// Try to execute the timer again later.
TIMERS.with(|timers| {
timers.borrow_mut().push(timer);
});
continue;
if let Err(e) = res {
ic_cdk::println!("[ic-cdk-timers] canister_global_timer: {e:?}");
let mut retry_later = false;
match e {
CallError::CallRejected(reject_code, _) => {
if reject_code == RejectCode::SysTransient {
retry_later = true;
}
}
CallError::CallPerformFailed(call_perform_error_code) => {
if call_perform_error_code == CallPerformErrorCode::SysTransient {
retry_later = true;
}
RejectionCode::NoError
| RejectionCode::SysFatal
| RejectionCode::DestinationInvalid
| RejectionCode::CanisterReject
| RejectionCode::CanisterError
| RejectionCode::Unknown => {}
}
CallError::CandidEncodeFailed(_) | CallError::CandidDecodeFailed(_) => {
// These errors are not transient, and will not be retried.
}
}
if retry_later {
// Try to execute the timer again later.
TIMERS.with(|timers| {
timers.borrow_mut().push(timer);
});
continue;
}
}
TASKS.with(|tasks| {
Expand Down
44 changes: 44 additions & 0 deletions ic-cdk/src/api/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ use std::task::{Context, Poll, Waker};
///
/// These can be obtained either using `reject_code()` or `reject_result()`.
#[allow(missing_docs)]
#[deprecated(
since = "0.18.0",
note = "Please use `ic_cdk::call::RejectCode` instead."
)]
#[repr(usize)]
#[derive(CandidType, Deserialize, Clone, Copy, Hash, Debug, PartialEq, Eq, PartialOrd, Ord)]
pub enum RejectionCode {
Expand Down Expand Up @@ -55,6 +59,10 @@ impl From<u32> for RejectionCode {
/// The result of a Call.
///
/// Errors on the IC have two components; a Code and a message associated with it.
#[deprecated(
since = "0.18.0",
note = "Please use `ic_cdk::call::CallResult` instead."
)]
pub type CallResult<R> = Result<R, (RejectionCode, String)>;

// Internal state for the Future when sending a call.
Expand Down Expand Up @@ -223,6 +231,10 @@ fn add_payment(payment: u128) {
/// * If the payment is non-zero and the system fails to deliver the notification, the behaviour
/// is unspecified: the funds can be either reimbursed or consumed irrevocably by the IC depending
/// on the underlying implementation of one-way calls.
#[deprecated(
since = "0.18.0",
note = "Please use `ic_cdk::call::Call::new().with_arg(,,).with_cycles(..).call_oneway()` instead."
)]
pub fn notify_with_payment128<T: ArgumentEncoder>(
id: Principal,
method: &str,
Expand All @@ -234,6 +246,10 @@ pub fn notify_with_payment128<T: ArgumentEncoder>(
}

/// Like [notify_with_payment128], but sets the payment to zero.
#[deprecated(
since = "0.18.0",
note = "Please use `ic_cdk::call::Call::new().with_arg(,,).with_cycles(..).call_oneway()` instead."
)]
pub fn notify<T: ArgumentEncoder>(
id: Principal,
method: &str,
Expand All @@ -243,6 +259,10 @@ pub fn notify<T: ArgumentEncoder>(
}

/// Like [notify], but sends the argument as raw bytes, skipping Candid serialization.
#[deprecated(
since = "0.18.0",
note = "Please use `ic_cdk::call::Call::new().with_raw_args(..).with_cycles(..).call_oneway()` instead."
)]
pub fn notify_raw(
id: Principal,
method: &str,
Expand Down Expand Up @@ -299,6 +319,10 @@ pub fn notify_raw(
/// call_raw(callee_canister(), "add_user", b"abcd", 1_000_000u64).await.unwrap()
/// }
/// ```
#[deprecated(
since = "0.18.0",
note = "Please use `ic_cdk::call::Call::new().with_raw_args(..).with_cycles(..).call()` instead."
)]
pub fn call_raw<'a, T: AsRef<[u8]> + Send + Sync + 'a>(
id: Principal,
method: &str,
Expand All @@ -322,6 +346,10 @@ pub fn call_raw<'a, T: AsRef<[u8]> + Send + Sync + 'a>(
/// call_raw128(callee_canister(), "add_user", b"abcd", 1_000_000u128).await.unwrap()
/// }
/// ```
#[deprecated(
since = "0.18.0",
note = "Please use `ic_cdk::call::Call::new().with_raw_args(..).with_cycles(..).call()` instead."
)]
pub fn call_raw128<'a, T: AsRef<[u8]> + Send + Sync + 'a>(
id: Principal,
method: &str,
Expand Down Expand Up @@ -388,6 +416,10 @@ fn decoder_error_to_reject<T>(err: candid::error::Error) -> (RejectionCode, Stri
/// * The type annotation on return type is required. Or the return type can be inferred from the context.
/// * The asynchronous call must be awaited in order for the inter-canister call to be made.
/// * If the reply payload is not a valid encoding of the expected type `T`, the call results in [RejectionCode::CanisterError] error.
#[deprecated(
since = "0.18.0",
note = "Please use `ic_cdk::call::Call::new().with_arg(..).call()` instead."
)]
pub fn call<T: ArgumentEncoder, R: for<'a> ArgumentDecoder<'a>>(
id: Principal,
method: &str,
Expand Down Expand Up @@ -430,6 +462,10 @@ pub fn call<T: ArgumentEncoder, R: for<'a> ArgumentDecoder<'a>>(
/// * The type annotation on return type is required. Or the return type can be inferred from the context.
/// * The asynchronous call must be awaited in order for the inter-canister call to be made.
/// * If the reply payload is not a valid encoding of the expected type `T`, the call results in [RejectionCode::CanisterError] error.
#[deprecated(
since = "0.18.0",
note = "Please use `ic_cdk::call::Call::new().with_arg(..).with_cycles(..).call()` instead."
)]
pub fn call_with_payment<T: ArgumentEncoder, R: for<'a> ArgumentDecoder<'a>>(
id: Principal,
method: &str,
Expand Down Expand Up @@ -473,6 +509,10 @@ pub fn call_with_payment<T: ArgumentEncoder, R: for<'a> ArgumentDecoder<'a>>(
/// * The type annotation on return type is required. Or the return type can be inferred from the context.
/// * The asynchronous call must be awaited in order for the inter-canister call to be made.
/// * If the reply payload is not a valid encoding of the expected type `T`, the call results in [RejectionCode::CanisterError] error.
#[deprecated(
since = "0.18.0",
note = "Please use `ic_cdk::call::Call::new().with_arg(..).with_cycles(..).call()` instead."
)]
pub fn call_with_payment128<T: ArgumentEncoder, R: for<'a> ArgumentDecoder<'a>>(
id: Principal,
method: &str,
Expand Down Expand Up @@ -519,6 +559,10 @@ pub fn call_with_payment128<T: ArgumentEncoder, R: for<'a> ArgumentDecoder<'a>>(
/// user_id
/// }
/// ```
#[deprecated(
since = "0.18.0",
note = "Please use `ic_cdk::call::Call::new().with_arg(..).with_cycles(..).with_decoder_config(..).call()` instead."
)]
pub fn call_with_config<'b, T: ArgumentEncoder, R: for<'a> ArgumentDecoder<'a>>(
id: Principal,
method: &'b str,
Expand Down
Loading

0 comments on commit c2ba198

Please sign in to comment.