From de00ceef6e046b0454a1a74e77a78a055585e678 Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Fri, 17 Jan 2025 14:39:41 -0500 Subject: [PATCH 1/5] add back Unrecognized(u32) variant in RejectCode --- ic-cdk/src/call.rs | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/ic-cdk/src/call.rs b/ic-cdk/src/call.rs index bd924568..11340ae6 100644 --- a/ic-cdk/src/call.rs +++ b/ic-cdk/src/call.rs @@ -14,6 +14,7 @@ use std::task::{Context, Poll, Waker}; /// Reject code explains why the inter-canister call is rejected. /// /// See [Reject codes](https://internetcomputer.org/docs/current/references/ic-interface-spec/#reject-codes) for more details. +#[repr(u32)] #[derive(CandidType, Deserialize, Clone, Copy, Hash, Debug, PartialEq, Eq, PartialOrd, Ord)] pub enum RejectCode { /// Fatal system error, retry unlikely to be useful. @@ -28,34 +29,41 @@ pub enum RejectCode { CanisterError = 5, /// Response unknown; system stopped waiting for it (e.g., timed out, or system under high load). SysUnknown = 6, + + /// Unrecognized reject code. + /// + /// Note that this variant is not part of the IC interface spec, and is used to represent + /// reject codes that are not recognized by the library. + Unrecognized(u32), } /// Error type for [`RejectCode`] conversion. /// -/// A reject code is invalid if it is not one of the known reject codes. +/// The only case where this error can occur is when trying to convert a 0 to a [`RejectCode`]. #[derive(Clone, Copy, Debug)] -pub struct InvalidRejectCode(pub u32); +pub struct ZeroIsInvalidRejectCode; -impl std::fmt::Display for InvalidRejectCode { +impl std::fmt::Display for ZeroIsInvalidRejectCode { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "invalid reject code: {}", self.0) + write!(f, "zero is invalid reject code") } } -impl Error for InvalidRejectCode {} +impl Error for ZeroIsInvalidRejectCode {} impl TryFrom for RejectCode { - type Error = InvalidRejectCode; + type Error = ZeroIsInvalidRejectCode; fn try_from(code: u32) -> Result { match code { + 0 => Err(ZeroIsInvalidRejectCode), 1 => Ok(RejectCode::SysFatal), 2 => Ok(RejectCode::SysTransient), 3 => Ok(RejectCode::DestinationInvalid), 4 => Ok(RejectCode::CanisterReject), 5 => Ok(RejectCode::CanisterError), 6 => Ok(RejectCode::SysUnknown), - n => Err(InvalidRejectCode(n)), + _ => Ok(RejectCode::Unrecognized(code)), } } } @@ -69,6 +77,7 @@ impl From for u32 { RejectCode::CanisterReject => 4, RejectCode::CanisterError => 5, RejectCode::SysUnknown => 6, + RejectCode::Unrecognized(code) => code, } } } @@ -537,6 +546,7 @@ impl> Future for CallFuture { // call_perform returns 0 means the call was successfully enqueued. } _ => { + // SAFETY: The conversion is safe because the code is not 0. let reject_code = RejectCode::try_from(code).unwrap(); let result = Err(CallRejected { reject_code, @@ -570,6 +580,7 @@ unsafe extern "C" fn callback>(state_ptr: *const RwLock Ok(msg_arg_data()), code => { + // SAFETY: The conversion is safe because the code is not 0. let reject_code = RejectCode::try_from(code).unwrap(); Err(CallRejected { reject_code, @@ -688,6 +699,7 @@ fn call_oneway_internal>( match code { 0 => Ok(()), _ => { + // SAFETY: The conversion is safe because the code is not 0. let reject_code = RejectCode::try_from(code).unwrap(); Err(CallRejected { reject_code, From 03559937c9742932ce17257e4b4112641406e1b7 Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Fri, 17 Jan 2025 14:53:21 -0500 Subject: [PATCH 2/5] Clarify candid encoding fail and DRY --- ic-cdk/src/call.rs | 39 ++++++++++++++++++++++----------------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/ic-cdk/src/call.rs b/ic-cdk/src/call.rs index 11340ae6..ab9bf213 100644 --- a/ic-cdk/src/call.rs +++ b/ic-cdk/src/call.rs @@ -32,8 +32,14 @@ pub enum RejectCode { /// Unrecognized reject code. /// - /// Note that this variant is not part of the IC interface spec, and is used to represent + /// # Note + /// + /// This variant is not part of the IC interface spec, and is used to represent /// reject codes that are not recognized by the library. + /// + /// This variant is needed just in case the IC introduces new reject codes in the future. + /// If that happens, a Canister using existing library versions will still be able to convert + /// the new reject codes to this variant without panicking. Unrecognized(u32), } @@ -391,10 +397,7 @@ impl SendableCall for Call<'_> { impl<'a, T: ArgumentEncoder + Send + Sync> SendableCall for CallWithArgs<'a, T> { async fn call_raw(self) -> SystemResult> { - // Candid Encoding can only fail if heap memory is exhausted. - // That is not a recoverable error, so we panic. - let args_raw = - encode_args(self.args).unwrap_or_else(|e| panic!("Failed to encode args: {}", e)); + let args_raw = encode_args(self.args).unwrap_or_else(panic_when_encode_fails); call_raw_internal( self.call.canister_id, self.call.method, @@ -406,10 +409,7 @@ impl<'a, T: ArgumentEncoder + Send + Sync> SendableCall for CallWithArgs<'a, T> } fn call_oneway(self) -> SystemResult<()> { - // Candid Encoding can only fail if heap memory is exhausted. - // That is not a recoverable error, so we panic. - let args_raw = - encode_args(self.args).unwrap_or_else(|e| panic!("Failed to encode args: {}", e)); + let args_raw = encode_args(self.args).unwrap_or_else(panic_when_encode_fails); call_oneway_internal( self.call.canister_id, self.call.method, @@ -422,10 +422,7 @@ impl<'a, T: ArgumentEncoder + Send + Sync> SendableCall for CallWithArgs<'a, T> impl<'a, T: CandidType + Send + Sync> SendableCall for CallWithArg<'a, T> { async fn call_raw(self) -> SystemResult> { - // Candid Encoding can only fail if heap memory is exhausted. - // That is not a recoverable error, so we panic. - let args_raw = - encode_one(self.arg).unwrap_or_else(|e| panic!("Failed to encode arg: {}", e)); + let args_raw = encode_one(self.arg).unwrap_or_else(panic_when_encode_fails); call_raw_internal( self.call.canister_id, self.call.method, @@ -437,10 +434,7 @@ impl<'a, T: CandidType + Send + Sync> SendableCall for CallWithArg<'a, T> { } fn call_oneway(self) -> SystemResult<()> { - // Candid Encoding can only fail if heap memory is exhausted. - // That is not a recoverable error, so we panic. - let args_raw = - encode_one(self.arg).unwrap_or_else(|e| panic!("Failed to encode arg: {}", e)); + let args_raw = encode_one(self.arg).unwrap_or_else(panic_when_encode_fails); call_oneway_internal( self.call.canister_id, self.call.method, @@ -728,3 +722,14 @@ fn call_cycles_add(cycles: u128) { fn decoder_error_to_call_error(err: candid::error::Error) -> CallError { CallError::CandidDecodeFailed(format!("{}: {}", std::any::type_name::(), err)) } + +/// When args encoding fails, we panic with an informative message. +/// +/// Currently, Candid encoding only fails when heap memory is exhausted, +/// in which case execution would trap before reaching the unwrap. +/// +/// However, since future implementations might introduce other failure cases, +/// we provide an informative panic message for better debuggability. +fn panic_when_encode_fails(err: candid::error::Error) -> Vec { + panic!("failed to encode args: {}", err) +} From 129b60d4a5dcade9c167f26057896c47a481e284 Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Fri, 17 Jan 2025 14:56:02 -0500 Subject: [PATCH 3/5] update reject_message when sync failure --- ic-cdk/src/call.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/ic-cdk/src/call.rs b/ic-cdk/src/call.rs index ab9bf213..29c15f27 100644 --- a/ic-cdk/src/call.rs +++ b/ic-cdk/src/call.rs @@ -11,6 +11,8 @@ use std::sync::atomic::Ordering; use std::sync::{Arc, RwLock, Weak}; use std::task::{Context, Poll, Waker}; +const CALL_PERFORM_REJECT_MESSAGE: &str = "call_perform failed"; + /// Reject code explains why the inter-canister call is rejected. /// /// See [Reject codes](https://internetcomputer.org/docs/current/references/ic-interface-spec/#reject-codes) for more details. @@ -137,7 +139,7 @@ impl CallRejected { /// this message is set with [`msg_reject`](crate::api::msg_reject). /// /// When the call was rejected synchronously (`ic0.call_preform` returns non-zero code), - /// this message is set to a fixed string ("failed to enqueue the call"). + /// this message is set to a fixed string ("call_perform failed"). pub fn reject_message(&self) -> &str { &self.reject_message } @@ -544,7 +546,7 @@ impl> Future for CallFuture { let reject_code = RejectCode::try_from(code).unwrap(); let result = Err(CallRejected { reject_code, - reject_message: "failed to enqueue the call".to_string(), + reject_message: CALL_PERFORM_REJECT_MESSAGE.to_string(), sync: true, }); state.result = Some(result.clone()); @@ -697,7 +699,7 @@ fn call_oneway_internal>( let reject_code = RejectCode::try_from(code).unwrap(); Err(CallRejected { reject_code, - reject_message: "failed to enqueue the call".to_string(), + reject_message: CALL_PERFORM_REJECT_MESSAGE.to_string(), sync: true, }) } From 127fe4b94d2cd66836f0ff67e86c7bafebef06a5 Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Fri, 17 Jan 2025 14:57:25 -0500 Subject: [PATCH 4/5] mention msg_reject_msg for reject_message --- ic-cdk/src/call.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ic-cdk/src/call.rs b/ic-cdk/src/call.rs index 29c15f27..92342403 100644 --- a/ic-cdk/src/call.rs +++ b/ic-cdk/src/call.rs @@ -136,7 +136,7 @@ impl CallRejected { /// Returns the reject message. /// /// When the call was rejected asynchronously (IC rejects the call after it was enqueued), - /// this message is set with [`msg_reject`](crate::api::msg_reject). + /// this message is get from [`msg_reject_msg`](crate::api::msg_reject_msg). /// /// When the call was rejected synchronously (`ic0.call_preform` returns non-zero code), /// this message is set to a fixed string ("call_perform failed"). From 006e69f525c26c299a394d80dec8e00678913166 Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Fri, 17 Jan 2025 15:04:38 -0500 Subject: [PATCH 5/5] fix explicit target is redundant --- ic-cdk/src/call.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ic-cdk/src/call.rs b/ic-cdk/src/call.rs index 92342403..f6f53587 100644 --- a/ic-cdk/src/call.rs +++ b/ic-cdk/src/call.rs @@ -136,7 +136,7 @@ impl CallRejected { /// Returns the reject message. /// /// When the call was rejected asynchronously (IC rejects the call after it was enqueued), - /// this message is get from [`msg_reject_msg`](crate::api::msg_reject_msg). + /// this message is get from [`msg_reject_msg`]. /// /// When the call was rejected synchronously (`ic0.call_preform` returns non-zero code), /// this message is set to a fixed string ("call_perform failed").