diff --git a/Cargo.lock b/Cargo.lock index a7a8bc96c9..b3230081fe 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2695,7 +2695,7 @@ dependencies = [ [[package]] name = "ic-agent" version = "0.40.0" -source = "git+https://github.com/dfinity/agent-rs?rev=b53d770cfd07df07b1024cfd9cc25f7ff80d1b76#b53d770cfd07df07b1024cfd9cc25f7ff80d1b76" +source = "git+https://github.com/dfinity/agent-rs?rev=686c249adb80aac58b466e654827d8b9dcfe00f4#686c249adb80aac58b466e654827d8b9dcfe00f4" dependencies = [ "arc-swap", "async-channel", @@ -2704,7 +2704,6 @@ dependencies = [ "async-watch", "backoff", "cached 0.52.0", - "candid", "der 0.7.9", "ecdsa 0.16.9", "ed25519-consensus", @@ -2716,10 +2715,13 @@ dependencies = [ "ic-certification 3.0.2", "ic-transport-types 0.40.0", "ic-verify-bls-signature", + "ic_principal", + "itertools 0.14.0", "k256 0.13.4", "leb128", "p256", "pem 3.0.4", + "pin-project", "pkcs8 0.10.2", "rand", "rangemap", @@ -3154,7 +3156,7 @@ dependencies = [ [[package]] name = "ic-identity-hsm" version = "0.40.0" -source = "git+https://github.com/dfinity/agent-rs?rev=b53d770cfd07df07b1024cfd9cc25f7ff80d1b76#b53d770cfd07df07b1024cfd9cc25f7ff80d1b76" +source = "git+https://github.com/dfinity/agent-rs?rev=686c249adb80aac58b466e654827d8b9dcfe00f4#686c249adb80aac58b466e654827d8b9dcfe00f4" dependencies = [ "hex", "ic-agent", @@ -3293,7 +3295,7 @@ dependencies = [ [[package]] name = "ic-transport-types" version = "0.40.0" -source = "git+https://github.com/dfinity/agent-rs?rev=b53d770cfd07df07b1024cfd9cc25f7ff80d1b76#b53d770cfd07df07b1024cfd9cc25f7ff80d1b76" +source = "git+https://github.com/dfinity/agent-rs?rev=686c249adb80aac58b466e654827d8b9dcfe00f4#686c249adb80aac58b466e654827d8b9dcfe00f4" dependencies = [ "candid", "hex", @@ -3363,7 +3365,7 @@ dependencies = [ [[package]] name = "ic-utils" version = "0.40.0" -source = "git+https://github.com/dfinity/agent-rs?rev=b53d770cfd07df07b1024cfd9cc25f7ff80d1b76#b53d770cfd07df07b1024cfd9cc25f7ff80d1b76" +source = "git+https://github.com/dfinity/agent-rs?rev=686c249adb80aac58b466e654827d8b9dcfe00f4#686c249adb80aac58b466e654827d8b9dcfe00f4" dependencies = [ "async-trait", "candid", @@ -3807,6 +3809,15 @@ dependencies = [ "either", ] +[[package]] +name = "itertools" +version = "0.14.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2b192c782037fadd9cfa75548310488aabdbf3d2da73885b31bd0abd03351285" +dependencies = [ + "either", +] + [[package]] name = "itoa" version = "1.0.14" @@ -4784,6 +4795,26 @@ version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5be167a7af36ee22fe3115051bc51f6e6c7054c9348e28deb4f49bd6f705a315" +[[package]] +name = "pin-project" +version = "1.1.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "677f1add503faace112b9f1373e43e9e054bfdd22ff1a63c1bc485eaec6a6a8a" +dependencies = [ + "pin-project-internal", +] + +[[package]] +name = "pin-project-internal" +version = "1.1.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6e918e4ff8c4549eb882f14b3a4bc8c8bc93de829416eacf579f1207a8fbf861" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.98", +] + [[package]] name = "pin-project-lite" version = "0.2.16" diff --git a/Cargo.toml b/Cargo.toml index ea4888a19f..8cef19435c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,11 +25,11 @@ needless_lifetimes = "allow" candid = "0.10.11" candid_parser = "0.1.4" dfx-core = { path = "src/dfx-core", version = "0.1.0" } -ic-agent = { version = "0.40", git = "https://github.com/dfinity/agent-rs", rev = "b53d770cfd07df07b1024cfd9cc25f7ff80d1b76" } +ic-agent = { version = "0.40", git = "https://github.com/dfinity/agent-rs", rev = "686c249adb80aac58b466e654827d8b9dcfe00f4" } ic-asset = { path = "src/canisters/frontend/ic-asset", version = "0.22.0" } ic-cdk = "0.13.1" -ic-identity-hsm = { version = "0.40", git = "https://github.com/dfinity/agent-rs", rev = "b53d770cfd07df07b1024cfd9cc25f7ff80d1b76" } -ic-utils = { version = "0.40", git = "https://github.com/dfinity/agent-rs", rev = "b53d770cfd07df07b1024cfd9cc25f7ff80d1b76" } +ic-identity-hsm = { version = "0.40", git = "https://github.com/dfinity/agent-rs", rev = "686c249adb80aac58b466e654827d8b9dcfe00f4" } +ic-utils = { version = "0.40", git = "https://github.com/dfinity/agent-rs", rev = "686c249adb80aac58b466e654827d8b9dcfe00f4" } aes-gcm = { version = "0.10.3", features = ["std"] } anyhow = "1.0.56" diff --git a/src/canisters/frontend/ic-asset/src/batch_upload/retryable.rs b/src/canisters/frontend/ic-asset/src/batch_upload/retryable.rs index 235902cf93..66f215bb8d 100644 --- a/src/canisters/frontend/ic-asset/src/batch_upload/retryable.rs +++ b/src/canisters/frontend/ic-asset/src/batch_upload/retryable.rs @@ -1,17 +1,23 @@ +use ic_agent::agent::OperationStatus; use ic_agent::export::reqwest::StatusCode; -use ic_agent::AgentError; +use ic_utils::error::{BaseError, CanisterError}; -pub(crate) fn retryable(agent_error: &AgentError) -> bool { - match agent_error { - AgentError::TimeoutWaitingForResponse() => true, - AgentError::TransportError(_) => true, - AgentError::HttpError(http_error) => { - http_error.status == StatusCode::INTERNAL_SERVER_ERROR - || http_error.status == StatusCode::BAD_GATEWAY - || http_error.status == StatusCode::SERVICE_UNAVAILABLE - || http_error.status == StatusCode::GATEWAY_TIMEOUT - || http_error.status == StatusCode::TOO_MANY_REQUESTS - } - _ => false, +pub(crate) fn retryable(canister_error: &BaseError) -> bool { + let Some(agent_error) = canister_error.as_agent() else { + return false; + }; + if agent_error + .operation_info() + .is_some_and(|op| op.status == OperationStatus::NotSent) + { + true + } else if let Some(http_error) = agent_error.as_http_error() { + http_error.status == StatusCode::INTERNAL_SERVER_ERROR + || http_error.status == StatusCode::BAD_GATEWAY + || http_error.status == StatusCode::SERVICE_UNAVAILABLE + || http_error.status == StatusCode::GATEWAY_TIMEOUT + || http_error.status == StatusCode::TOO_MANY_REQUESTS + } else { + false } } diff --git a/src/canisters/frontend/ic-asset/src/canister_api/methods/asset_properties.rs b/src/canisters/frontend/ic-asset/src/canister_api/methods/asset_properties.rs index aebad4ccdf..aa7208249e 100644 --- a/src/canisters/frontend/ic-asset/src/canister_api/methods/asset_properties.rs +++ b/src/canisters/frontend/ic-asset/src/canister_api/methods/asset_properties.rs @@ -11,8 +11,8 @@ use crate::{ use backoff::backoff::Backoff; use backoff::ExponentialBackoffBuilder; use futures_intrusive::sync::SharedSemaphore; -use ic_agent::{agent::RejectResponse, AgentError}; use ic_utils::call::SyncCall; +use ic_utils::error::{BaseError, CanisterError}; use ic_utils::Canister; use std::{collections::HashMap, time::Duration}; @@ -51,12 +51,12 @@ pub(crate) async fn get_assets_properties( } break Ok(asset_properties); } - Err(agent_err) if !retryable(&agent_err) => { - break Err(agent_err); + Err(canister_err) if !retryable(&canister_err) => { + break Err(canister_err); } - Err(agent_err) => match retry_policy.next_backoff() { + Err(canister_err) => match retry_policy.next_backoff() { Some(duration) => tokio::time::sleep(duration).await, - None => break Err(agent_err), + None => break Err(canister_err), }, }; } @@ -73,12 +73,14 @@ pub(crate) async fn get_assets_properties( } // older canisters don't have get_assets_properties method // therefore we can break the loop - Err(AgentError::UncertifiedReject { - reject: RejectResponse { reject_message, .. }, - .. - }) if reject_message - .contains(&format!("has no query method '{GET_ASSET_PROPERTIES}'")) - || reject_message.contains("query method does not exist") => + Err(err) + if err.as_agent().is_some_and(|err| { + err.as_reject().is_some_and(|err| { + err.reject_message + .contains(&format!("has no query method '{GET_ASSET_PROPERTIES}'")) + || err.reject_message.contains("query method does not exist") + }) + }) => { break; } @@ -94,7 +96,7 @@ pub(crate) async fn get_assets_properties( pub(crate) async fn get_asset_properties( canister: &Canister<'_>, asset_id: &str, -) -> Result { +) -> Result { let (asset_properties,): (AssetProperties,) = canister .query(GET_ASSET_PROPERTIES) .with_arg(GetAssetPropertiesArgument(asset_id.to_string())) diff --git a/src/canisters/frontend/ic-asset/src/canister_api/methods/batch.rs b/src/canisters/frontend/ic-asset/src/canister_api/methods/batch.rs index 15b30379fd..0cd038b3b1 100644 --- a/src/canisters/frontend/ic-asset/src/canister_api/methods/batch.rs +++ b/src/canisters/frontend/ic-asset/src/canister_api/methods/batch.rs @@ -8,12 +8,12 @@ use crate::canister_api::types::batch_upload::common::{ use backoff::backoff::Backoff; use backoff::ExponentialBackoffBuilder; use candid::{CandidType, Nat}; -use ic_agent::AgentError; +use ic_utils::error::BaseError; use ic_utils::Canister; use serde_bytes::ByteBuf; use std::time::Duration; -pub(crate) async fn create_batch(canister: &Canister<'_>) -> Result { +pub(crate) async fn create_batch(canister: &Canister<'_>) -> Result { let mut retry_policy = ExponentialBackoffBuilder::new() .with_initial_interval(Duration::from_secs(1)) .with_max_interval(Duration::from_secs(16)) @@ -31,12 +31,12 @@ pub(crate) async fn create_batch(canister: &Canister<'_>) -> Result break Ok(batch_id), - Err(agent_err) if !retryable(&agent_err) => { - break Err(agent_err); + Err(canister_err) if !retryable(&canister_err) => { + break Err(canister_err); } - Err(agent_err) => match retry_policy.next_backoff() { + Err(canister_err) => match retry_policy.next_backoff() { Some(duration) => tokio::time::sleep(duration).await, - None => break Err(agent_err), + None => break Err(canister_err), }, }; }?; @@ -47,7 +47,7 @@ pub(crate) async fn submit_commit_batch( canister: &Canister<'_>, method_name: &str, arg: T, // CommitBatchArguments_{v0,v1,etc} -) -> Result<(), AgentError> { +) -> Result<(), BaseError> { let mut retry_policy = ExponentialBackoffBuilder::new() .with_initial_interval(Duration::from_secs(1)) .with_max_interval(Duration::from_secs(16)) @@ -58,12 +58,12 @@ pub(crate) async fn submit_commit_batch( loop { match canister.update(method_name).with_arg(&arg).build().await { Ok(()) => return Ok(()), - Err(agent_err) if !retryable(&agent_err) => { - return Err(agent_err); + Err(canister_err) if !retryable(&canister_err) => { + return Err(canister_err); } - Err(agent_err) => match retry_policy.next_backoff() { + Err(canister_err) => match retry_policy.next_backoff() { Some(duration) => tokio::time::sleep(duration).await, - None => return Err(agent_err), + None => return Err(canister_err), }, } } @@ -72,21 +72,21 @@ pub(crate) async fn submit_commit_batch( pub(crate) async fn commit_batch( canister: &Canister<'_>, arg: T, // CommitBatchArguments_{v0,v1,etc} -) -> Result<(), AgentError> { +) -> Result<(), BaseError> { submit_commit_batch(canister, COMMIT_BATCH, arg).await } pub(crate) async fn propose_commit_batch( canister: &Canister<'_>, arg: T, // CommitBatchArguments_{v0,v1,etc} -) -> Result<(), AgentError> { +) -> Result<(), BaseError> { submit_commit_batch(canister, PROPOSE_COMMIT_BATCH, arg).await } pub(crate) async fn compute_evidence( canister: &Canister<'_>, arg: &ComputeEvidenceArguments, -) -> Result, AgentError> { +) -> Result, BaseError> { let mut retry_policy = ExponentialBackoffBuilder::new() .with_initial_interval(Duration::from_secs(1)) .with_max_interval(Duration::from_secs(16)) @@ -103,12 +103,12 @@ pub(crate) async fn compute_evidence( .await { Ok(x) => return Ok(x.0), - Err(agent_err) if !retryable(&agent_err) => { - return Err(agent_err); + Err(canister_err) if !retryable(&canister_err) => { + return Err(canister_err); } - Err(agent_err) => match retry_policy.next_backoff() { + Err(canister_err) => match retry_policy.next_backoff() { Some(duration) => tokio::time::sleep(duration).await, - None => return Err(agent_err), + None => return Err(canister_err), }, } } diff --git a/src/canisters/frontend/ic-asset/src/canister_api/methods/chunk.rs b/src/canisters/frontend/ic-asset/src/canister_api/methods/chunk.rs index 190f799f15..6e36c0d7f4 100644 --- a/src/canisters/frontend/ic-asset/src/canister_api/methods/chunk.rs +++ b/src/canisters/frontend/ic-asset/src/canister_api/methods/chunk.rs @@ -55,7 +55,7 @@ pub(crate) async fn create_chunk( .and_then(|bytes| Ok((Decode!(&bytes, CreateChunkResponse)?.chunk_id,))) } }, - Err(agent_err) => Err(agent_err), + Err(canister_err) => Err(canister_err), }; match wait_result { @@ -65,12 +65,12 @@ pub(crate) async fn create_chunk( } return Ok(chunk_id); } - Err(agent_err) if !retryable(&agent_err) => { - return Err(CreateChunkError::CreateChunk(agent_err)); + Err(canister_err) if !retryable(&canister_err) => { + return Err(CreateChunkError::CreateChunk(canister_err)); } - Err(agent_err) => match retry_policy.next_backoff() { + Err(canister_err) => match retry_policy.next_backoff() { Some(duration) => tokio::time::sleep(duration).await, - None => return Err(CreateChunkError::CreateChunk(agent_err)), + None => return Err(CreateChunkError::CreateChunk(canister_err)), }, } } @@ -117,7 +117,7 @@ pub(crate) async fn create_chunks( .and_then(|bytes| Ok((Decode!(&bytes, CreateChunksResponse)?.chunk_ids,))) } }, - Err(agent_err) => Err(agent_err), + Err(canister_err) => Err(canister_err), }; match wait_result { @@ -127,12 +127,12 @@ pub(crate) async fn create_chunks( } return Ok(chunk_ids); } - Err(agent_err) if !retryable(&agent_err) => { - return Err(CreateChunkError::CreateChunks(agent_err)); + Err(canister_err) if !retryable(&canister_err) => { + return Err(CreateChunkError::CreateChunks(canister_err)); } - Err(agent_err) => match retry_policy.next_backoff() { + Err(canister_err) => match retry_policy.next_backoff() { Some(duration) => tokio::time::sleep(duration).await, - None => return Err(CreateChunkError::CreateChunks(agent_err)), + None => return Err(CreateChunkError::CreateChunks(canister_err)), }, } } diff --git a/src/canisters/frontend/ic-asset/src/canister_api/methods/list.rs b/src/canisters/frontend/ic-asset/src/canister_api/methods/list.rs index 321e30b34c..f1aef1ad4b 100644 --- a/src/canisters/frontend/ic-asset/src/canister_api/methods/list.rs +++ b/src/canisters/frontend/ic-asset/src/canister_api/methods/list.rs @@ -1,13 +1,13 @@ use crate::canister_api::methods::method_names::LIST; use crate::canister_api::types::{asset::AssetDetails, list::ListAssetsRequest}; -use ic_agent::AgentError; use ic_utils::call::SyncCall; +use ic_utils::error::BaseError; use ic_utils::Canister; use std::collections::HashMap; pub(crate) async fn list_assets( canister: &Canister<'_>, -) -> Result, AgentError> { +) -> Result, BaseError> { let (entries,): (Vec,) = canister .query(LIST) .with_arg(ListAssetsRequest {}) diff --git a/src/canisters/frontend/ic-asset/src/error/compute_evidence.rs b/src/canisters/frontend/ic-asset/src/error/compute_evidence.rs index d6137119f7..5b790cdd89 100644 --- a/src/canisters/frontend/ic-asset/src/error/compute_evidence.rs +++ b/src/canisters/frontend/ic-asset/src/error/compute_evidence.rs @@ -2,7 +2,7 @@ use crate::error::create_project_asset::CreateProjectAssetError; use crate::error::gather_asset_descriptors::GatherAssetDescriptorsError; use crate::error::get_asset_properties::GetAssetPropertiesError; use crate::error::hash_content::HashContentError; -use ic_agent::AgentError; +use ic_utils::error::BaseError; use thiserror::Error; use super::AssembleCommitBatchArgumentError; @@ -32,5 +32,5 @@ pub enum ComputeEvidenceError { /// Failed to list assets in the asset canister. #[error("Failed to list assets")] - ListAssets(#[source] AgentError), + ListAssets(#[source] BaseError), } diff --git a/src/canisters/frontend/ic-asset/src/error/create_chunk.rs b/src/canisters/frontend/ic-asset/src/error/create_chunk.rs index adba0d4b91..fad8702c6e 100644 --- a/src/canisters/frontend/ic-asset/src/error/create_chunk.rs +++ b/src/canisters/frontend/ic-asset/src/error/create_chunk.rs @@ -1,4 +1,4 @@ -use ic_agent::AgentError; +use ic_utils::error::BaseError; use thiserror::Error; /// Errors related to creating a chunk. @@ -6,11 +6,11 @@ use thiserror::Error; pub enum CreateChunkError { /// Failed in call to create_chunk, or in waiting for response. #[error("Failed to create chunk")] - CreateChunk(#[source] AgentError), + CreateChunk(#[source] BaseError), /// Failed in call to create_chunks, or in waiting for response. #[error("Failed to create chunks")] - CreateChunks(#[source] AgentError), + CreateChunks(#[source] BaseError), /// Failed to decode the create chunk response. #[error("Failed to decode create chunk response")] diff --git a/src/canisters/frontend/ic-asset/src/error/get_asset_properties.rs b/src/canisters/frontend/ic-asset/src/error/get_asset_properties.rs index 395fe43cdc..8ffc2141b9 100644 --- a/src/canisters/frontend/ic-asset/src/error/get_asset_properties.rs +++ b/src/canisters/frontend/ic-asset/src/error/get_asset_properties.rs @@ -1,4 +1,4 @@ -use ic_agent::AgentError; +use ic_utils::error::BaseError; use thiserror::Error; /// Errors related to getting asset properties. @@ -6,5 +6,5 @@ use thiserror::Error; pub enum GetAssetPropertiesError { /// The call to get_asset_properties failed. #[error("Failed to get asset properties for {0}")] - GetAssetPropertiesFailed(String, #[source] AgentError), + GetAssetPropertiesFailed(String, #[source] BaseError), } diff --git a/src/canisters/frontend/ic-asset/src/error/prepare_sync_for_proposal.rs b/src/canisters/frontend/ic-asset/src/error/prepare_sync_for_proposal.rs index a22a5dd871..e2877e90bd 100644 --- a/src/canisters/frontend/ic-asset/src/error/prepare_sync_for_proposal.rs +++ b/src/canisters/frontend/ic-asset/src/error/prepare_sync_for_proposal.rs @@ -1,5 +1,5 @@ use crate::error::upload_content::UploadContentError; -use ic_agent::AgentError; +use ic_utils::error::BaseError; use thiserror::Error; /// Errors related to preparing synchronization operations for a proposal. @@ -7,11 +7,11 @@ use thiserror::Error; pub enum PrepareSyncForProposalError { /// Failed while requesting that the asset canister compute evidence. #[error("Failed to compute evidence")] - ComputeEvidence(#[source] AgentError), + ComputeEvidence(#[source] BaseError), /// Failed while calling propose_commit_batch. #[error("Failed to propose batch to commit")] - ProposeCommitBatch(#[source] AgentError), + ProposeCommitBatch(#[source] BaseError), /// Failed while uploading content for synchronization. #[error(transparent)] diff --git a/src/canisters/frontend/ic-asset/src/error/sync.rs b/src/canisters/frontend/ic-asset/src/error/sync.rs index b93f0d251c..40b83427af 100644 --- a/src/canisters/frontend/ic-asset/src/error/sync.rs +++ b/src/canisters/frontend/ic-asset/src/error/sync.rs @@ -1,6 +1,6 @@ use crate::error::compatibility::CompatibilityError; use crate::error::upload_content::UploadContentError; -use ic_agent::AgentError; +use ic_utils::error::BaseError; use thiserror::Error; /// Errors related to the sync process. @@ -8,7 +8,7 @@ use thiserror::Error; pub enum SyncError { /// Failed when calling commit_batch #[error("Failed to commit batch")] - CommitBatchFailed(#[source] AgentError), + CommitBatchFailed(#[source] BaseError), /// Failed when trying to work with an older asset canister. #[error(transparent)] diff --git a/src/canisters/frontend/ic-asset/src/error/upload.rs b/src/canisters/frontend/ic-asset/src/error/upload.rs index d9277c483c..15caccbf1b 100644 --- a/src/canisters/frontend/ic-asset/src/error/upload.rs +++ b/src/canisters/frontend/ic-asset/src/error/upload.rs @@ -1,7 +1,7 @@ use super::AssembleCommitBatchArgumentError; use crate::error::compatibility::CompatibilityError; use crate::error::create_project_asset::CreateProjectAssetError; -use ic_agent::AgentError; +use ic_utils::error::BaseError; use thiserror::Error; /// Errors encountered during the upload process. @@ -9,7 +9,7 @@ use thiserror::Error; pub enum UploadError { /// Failed when calling commit_batch. #[error("Commit batch failed")] - CommitBatchFailed(#[source] AgentError), + CommitBatchFailed(#[source] BaseError), /// Failure when trying to work with an older asset canister. #[error(transparent)] @@ -17,7 +17,7 @@ pub enum UploadError { /// Failed when calling create_batch. #[error("Create batch failed")] - CreateBatchFailed(#[source] AgentError), + CreateBatchFailed(#[source] BaseError), /// Failed when assembling commit_batch argument. #[error("Failed to assemble commit_batch argument")] @@ -29,5 +29,5 @@ pub enum UploadError { /// Failed when calling the list method. #[error("List assets failed")] - ListAssetsFailed(#[source] AgentError), + ListAssetsFailed(#[source] BaseError), } diff --git a/src/canisters/frontend/ic-asset/src/error/upload_content.rs b/src/canisters/frontend/ic-asset/src/error/upload_content.rs index 65bf203c23..040717ef1b 100644 --- a/src/canisters/frontend/ic-asset/src/error/upload_content.rs +++ b/src/canisters/frontend/ic-asset/src/error/upload_content.rs @@ -1,7 +1,7 @@ use crate::error::create_project_asset::CreateProjectAssetError; use crate::error::gather_asset_descriptors::GatherAssetDescriptorsError; use crate::error::get_asset_properties::GetAssetPropertiesError; -use ic_agent::AgentError; +use ic_utils::error::BaseError; use thiserror::Error; use super::AssembleCommitBatchArgumentError; @@ -15,7 +15,7 @@ pub enum UploadContentError { /// Failed when calling create_batch. #[error("Failed to create batch")] - CreateBatchFailed(#[source] AgentError), + CreateBatchFailed(#[source] BaseError), /// Failed when creating project assets. #[error("Failed to create project asset")] @@ -31,5 +31,5 @@ pub enum UploadContentError { /// Failed when calling the list method. #[error("Failed to list assets")] - ListAssetsFailed(#[source] AgentError), + ListAssetsFailed(#[source] BaseError), } diff --git a/src/canisters/frontend/ic-asset/src/sync.rs b/src/canisters/frontend/ic-asset/src/sync.rs index cc180d291a..b5632fbf3d 100644 --- a/src/canisters/frontend/ic-asset/src/sync.rs +++ b/src/canisters/frontend/ic-asset/src/sync.rs @@ -33,7 +33,7 @@ use crate::error::UploadContentError; use crate::error::UploadContentError::{CreateBatchFailed, ListAssetsFailed}; use crate::progress::{AssetSyncProgressRenderer, AssetSyncState}; use candid::Nat; -use ic_agent::AgentError; +use ic_utils::error::BaseError; use ic_utils::Canister; use itertools::Itertools; use serde_bytes::ByteBuf; @@ -190,7 +190,7 @@ async fn commit_in_stages( commit_batch_args: CommitBatchArguments, logger: &Logger, progress: Option<&dyn AssetSyncProgressRenderer>, -) -> Result<(), AgentError> { +) -> Result<(), BaseError> { if let Some(progress) = progress { progress.set_total_batch_operations(commit_batch_args.operations.len()); } diff --git a/src/dfx-core/src/canister/mod.rs b/src/dfx-core/src/canister/mod.rs index 4244666fd8..4963356e93 100644 --- a/src/dfx-core/src/canister/mod.rs +++ b/src/dfx-core/src/canister/mod.rs @@ -94,13 +94,13 @@ YOU WILL LOSE ALL DATA IN THE CANISTER. }; wallet .call( - *mgr.canister_id_(), + *mgr.canister_id(), "install_code", Argument::from_candid((install_args,)), 0, ) .await - .map_err(CanisterInstallError::InstallWasmError) + .map_err(CanisterInstallError::InstallWasmThroughWalletError) } } } diff --git a/src/dfx-core/src/error/canister.rs b/src/dfx-core/src/error/canister.rs index 3b2999beaf..db571a0a8a 100644 --- a/src/dfx-core/src/error/canister.rs +++ b/src/dfx-core/src/error/canister.rs @@ -1,9 +1,10 @@ +use ic_utils::{error::BaseError, interfaces::wallet::WalletError}; use thiserror::Error; #[derive(Error, Debug)] pub enum CanisterBuilderError { #[error("Failed to construct wallet canister caller")] - WalletCanisterCaller(#[source] ic_agent::AgentError), + WalletCanisterCaller(#[source] WalletError), #[error("Failed to build call sender")] CallSenderBuildError(#[source] ic_agent::AgentError), @@ -18,5 +19,8 @@ pub enum CanisterInstallError { CanisterBuilderError(#[from] CanisterBuilderError), #[error("Failed during wasm installation call")] - InstallWasmError(#[source] ic_agent::AgentError), + InstallWasmError(#[source] BaseError), + + #[error("Failed during wasm installation call")] + InstallWasmThroughWalletError(#[source] WalletError), } diff --git a/src/dfx-core/src/interface/builder.rs b/src/dfx-core/src/interface/builder.rs index e7390e2c93..9d26a93cfc 100644 --- a/src/dfx-core/src/interface/builder.rs +++ b/src/dfx-core/src/interface/builder.rs @@ -225,7 +225,7 @@ mod tests { use crate::DfxInterface; use candid::Principal; use futures::Future; - use ic_agent::{AgentError::TransportError, Identity}; + use ic_agent::{agent_error::ErrorKind, Identity}; use serde_json::json; use std::path::Path; use std::sync::Arc; @@ -433,7 +433,7 @@ mod tests { assert!(!d.network_descriptor.is_ic); assert!(d.network_descriptor.local_server_descriptor.is_some()); } - Err(FetchRootKey(AgentError(TransportError(_)))) => { + Err(FetchRootKey(AgentError(err))) if err.kind() == ErrorKind::Transport => { // local replica isn't running, so this is expected, // but we can't check anything else } @@ -516,7 +516,7 @@ mod tests { .with_force_fetch_root_key_insecure_non_mainnet_only() .build() .await, - Err(FetchRootKey(AgentError(TransportError(_)))) + Err(FetchRootKey(AgentError(err))) if err.kind() == ErrorKind::Transport )); }) .await; diff --git a/src/dfx/src/commands/canister/deposit_cycles.rs b/src/dfx/src/commands/canister/deposit_cycles.rs index 2425ccf51b..c5eb0de4a9 100644 --- a/src/dfx/src/commands/canister/deposit_cycles.rs +++ b/src/dfx/src/commands/canister/deposit_cycles.rs @@ -109,7 +109,7 @@ pub async fn exec( .await { Ok(wallet) => { - proxy_sender = CallSender::Wallet(*wallet.canister_id_()); + proxy_sender = CallSender::Wallet(*wallet.canister_id()); call_sender = &proxy_sender; } Err(err) => { diff --git a/src/dfx/src/commands/canister/metadata.rs b/src/dfx/src/commands/canister/metadata.rs index 2c4ea1337a..c808e1eed0 100644 --- a/src/dfx/src/commands/canister/metadata.rs +++ b/src/dfx/src/commands/canister/metadata.rs @@ -34,6 +34,12 @@ pub async fn exec(env: &dyn Environment, opts: CanisterMetadataOpts) -> DfxResul "Failed to read `{}` metadata of canister {}.", opts.metadata_name, canister_id ) + })? + .with_context(|| { + format!( + "Metadata `{}` of canister {} does not exist.", + opts.metadata_name, canister_id + ) })?; stdout().write_all(&metadata)?; diff --git a/src/dfx/src/commands/canister/request_status.rs b/src/dfx/src/commands/canister/request_status.rs index 22d9f3bd3f..1a7139ac39 100644 --- a/src/dfx/src/commands/canister/request_status.rs +++ b/src/dfx/src/commands/canister/request_status.rs @@ -3,13 +3,13 @@ use crate::lib::error::{DfxError, DfxResult}; use crate::lib::root_key::fetch_root_key_if_needed; use crate::util::clap::parsers; use crate::util::print_idl_blob; -use anyhow::Context; +use anyhow::{bail, Context}; use backoff::backoff::Backoff; use backoff::ExponentialBackoff; use candid::Principal; use clap::Parser; use ic_agent::agent::RequestStatusResponse; -use ic_agent::{AgentError, RequestId}; +use ic_agent::RequestId; use std::str::FromStr; /// Requests the status of a call from a canister. @@ -57,10 +57,7 @@ pub async fn exec(env: &dyn Environment, opts: RequestStatusOpts) -> DfxResult { match response { RequestStatusResponse::Replied(reply) => return Ok(reply.arg), RequestStatusResponse::Rejected(response) => { - return Err(DfxError::new(AgentError::CertifiedReject { - reject: response, - operation: None, - })) + bail!("The replica returned a rejection error: reject code {:?}, reject message {}, error code {:?}", response.reject_code, response.reject_message, response.error_code); } RequestStatusResponse::Unknown => (), RequestStatusResponse::Received | RequestStatusResponse::Processing => { @@ -75,15 +72,13 @@ pub async fn exec(env: &dyn Environment, opts: RequestStatusOpts) -> DfxResult { } } RequestStatusResponse::Done => { - return Err(DfxError::new(AgentError::RequestStatusDoneNoReply( - String::from(request_id), - ))) + bail!("Call was marked as done but dfx never saw the reply."); } }; let interval = retry_policy .next_backoff() - .ok_or_else(|| DfxError::new(AgentError::TimeoutWaitingForResponse()))?; + .context("Timed out waiting for a response")?; tokio::time::sleep(interval).await; } } diff --git a/src/dfx/src/commands/wallet/redeem_faucet_coupon.rs b/src/dfx/src/commands/wallet/redeem_faucet_coupon.rs index 9d342f7553..d28de7d72d 100644 --- a/src/dfx/src/commands/wallet/redeem_faucet_coupon.rs +++ b/src/dfx/src/commands/wallet/redeem_faucet_coupon.rs @@ -50,7 +50,7 @@ pub async fn exec(env: &dyn Environment, opts: RedeemFaucetCouponOpts) -> DfxRes match wallet { // identity has a wallet already - faucet should top up the wallet Ok(wallet_canister) => { - let wallet_principal = wallet_canister.canister_id_(); + let wallet_principal = wallet_canister.canister_id(); let response = agent .update(&faucet_principal, "redeem_to_wallet") .with_arg( diff --git a/src/dfx/src/lib/deps/pull/mod.rs b/src/dfx/src/lib/deps/pull/mod.rs index 0db305b485..f521a7d42a 100644 --- a/src/dfx/src/lib/deps/pull/mod.rs +++ b/src/dfx/src/lib/deps/pull/mod.rs @@ -13,7 +13,7 @@ use candid::Principal; use dfx_core::config::model::dfinity::Pullable; use dfx_core::fs::composite::{ensure_dir_exists, ensure_parent_dir_exists}; use fn_error_context::context; -use ic_agent::{Agent, AgentError}; +use ic_agent::Agent; use ic_wasm::metadata::get_metadata; use sha2::{Digest, Sha256}; use slog::{error, info, trace, warn, Logger}; @@ -205,10 +205,9 @@ async fn fetch_metadata( .read_state_canister_metadata(*canister_id, metadata) .await { - Ok(data) => Ok(Some(data)), - Err(agent_error) => match agent_error { - // replica returns such error - AgentError::HttpError(ref e) => { + Ok(data) => Ok(data), + Err(agent_error) => { + if let Some(e) = agent_error.as_http_error() { let status = e.status; let content = String::from_utf8(e.content.clone())?; if status == 404 @@ -218,13 +217,10 @@ async fn fetch_metadata( } else { bail!(agent_error); } - } - // ic-ref returns such error when the canister doesn't define the metadata - AgentError::LookupPathAbsent(_) => Ok(None), - _ => { + } else { bail!(agent_error) } - }, + } } } diff --git a/src/dfx/src/lib/diagnosis.rs b/src/dfx/src/lib/diagnosis.rs index 2a66b9adeb..9c82e7c0ee 100644 --- a/src/dfx/src/lib/diagnosis.rs +++ b/src/dfx/src/lib/diagnosis.rs @@ -3,10 +3,11 @@ use crate::lib::error_code; use anyhow::Error as AnyhowError; use dfx_core::error::root_key::FetchRootKeyError; use dfx_core::network::provider::get_network_context; -use ic_agent::agent::{RejectCode, RejectResponse}; +use ic_agent::agent::RejectCode; use ic_agent::AgentError; use ic_asset::error::{GatherAssetDescriptorsError, SyncError, UploadContentError}; use regex::Regex; +use std::error::Error; use std::path::Path; use thiserror::Error as ThisError; @@ -52,7 +53,7 @@ pub fn diagnose(err: &AnyhowError) -> DiagnosedError { } if not_a_controller(agent_err) { return diagnose_http_403(); - } else if *agent_err == AgentError::CertificateNotAuthorized() { + } else if certificate_not_authorized(agent_err) { return subnet_not_authorized(); } if cycles_ledger_not_found(err) { @@ -92,7 +93,10 @@ fn local_replica_not_running(err: &AnyhowError) -> bool { err.downcast_ref::() } }; - if let Some(AgentError::TransportError(transport_error)) = maybe_agent_error { + if let Some(transport_error) = maybe_agent_error.and_then(|err| { + err.source() + .and_then(|source| source.downcast_ref::()) + }) { transport_error.is_connect() && transport_error .url() @@ -110,41 +114,25 @@ fn local_replica_not_running(err: &AnyhowError) -> bool { fn not_a_controller(err: &AgentError) -> bool { // Newer replicas include the error code in the reject response. - matches!( - err, - AgentError::UncertifiedReject { - reject: RejectResponse { - reject_code: RejectCode::CanisterError, - error_code: Some(error_code), - .. - }, - .. - } if error_code == error_code::CANISTER_INVALID_CONTROLLER - ) + err.as_reject().is_some_and(|reject| { + reject.reject_code == RejectCode::CanisterError + && reject + .error_code + .as_ref() + .is_some_and(|code| code == error_code::CANISTER_INVALID_CONTROLLER) + }) } fn wallet_method_not_found(err: &AgentError) -> bool { - match err { - AgentError::CertifiedReject { - reject: - RejectResponse { - reject_code: RejectCode::CanisterError, - reject_message, - .. - }, - .. - } if reject_message.contains("Canister has no update method 'wallet_") => true, - AgentError::UncertifiedReject { - reject: - RejectResponse { - reject_code: RejectCode::CanisterError, - reject_message, - .. - }, - .. - } if reject_message.contains("Canister has no query method 'wallet_") => true, - _ => false, - } + err.as_reject().is_some_and(|reject| { + (reject + .reject_message + .contains("Canister has no update method 'wallet_") + || reject + .reject_message + .contains("Canister has no query method 'wallet_")) + && reject.reject_code == RejectCode::CanisterError + }) } fn diagnose_http_403() -> DiagnosedError { @@ -170,6 +158,10 @@ fn diagnose_local_replica_not_running() -> DiagnosedError { DiagnosedError::new(explanation, action_suggestion) } +fn certificate_not_authorized(error: &AgentError) -> bool { + format!("{error}").contains("Certificate is not authorized") +} + fn subnet_not_authorized() -> DiagnosedError { let explanation = "Subnet is not authorized to respond for the requested canister id."; let action_suggestion = "If you are connecting to a node directly instead of a boundary node, try using --provisional-create-canister-effective-canister-id with a canister id in the subnet's canister range. First non-root subnet: 5v3p4-iyaaa-aaaaa-qaaaa-cai, second non-root subnet: jrlun-jiaaa-aaaab-aaaaa-cai"; diff --git a/src/dfx/src/lib/identity/wallet.rs b/src/dfx/src/lib/identity/wallet.rs index 71d07b5dcc..05a3383aa1 100644 --- a/src/dfx/src/lib/identity/wallet.rs +++ b/src/dfx/src/lib/identity/wallet.rs @@ -12,7 +12,8 @@ use dfx_core::error::wallet_config::WalletConfigError; use dfx_core::identity::wallet::{get_wallet_config_path, wallet_canister_id}; use dfx_core::identity::{Identity, WalletGlobalConfig, WalletNetworkMap}; use ic_agent::agent::{RejectCode, RejectResponse}; -use ic_agent::AgentError; +use ic_agent::agent_error::ErrorKind; +use ic_utils::error::CanisterError; use ic_utils::interfaces::management_canister::builders::InstallMode; use ic_utils::interfaces::{ManagementCanister, WalletCanister}; use slog::info; @@ -105,15 +106,19 @@ pub async fn create_wallet( .with_mode(InstallMode::Install) .await { - Err(AgentError::CertifiedReject { - reject: + Err(err) + if err.as_agent().is_some_and(|err| { + err.kind() == ErrorKind::Reject + && err.operation_info().is_some_and(|op| { + matches!(op.response().unwrap().unwrap_err(), RejectResponse { reject_code: RejectCode::CanisterError, reject_message, .. - }, - .. - }) if reject_message.contains("not empty") => { + } if reject_message.contains("not empty")) + }) + }) => + { bail!( r#"The wallet canister "{canister_id}" already exists for user "{name}" on "{}" network."#, network.name diff --git a/src/dfx/src/lib/installers/assets/mod.rs b/src/dfx/src/lib/installers/assets/mod.rs index db76562b64..c65da07490 100644 --- a/src/dfx/src/lib/installers/assets/mod.rs +++ b/src/dfx/src/lib/installers/assets/mod.rs @@ -41,7 +41,7 @@ pub async fn post_install_store_assets( .with_context(|| { format!( "Failed asset sync with canister {}.", - canister.canister_id_() + canister.canister_id() ) }) } @@ -73,7 +73,7 @@ pub async fn prepare_assets_for_proposal( .with_context(|| { format!( "Failed asset sync with canister {}.", - canister.canister_id_() + canister.canister_id() ) })?; diff --git a/src/dfx/src/lib/migrate.rs b/src/dfx/src/lib/migrate.rs index e940e74e15..910b3bbf19 100644 --- a/src/dfx/src/lib/migrate.rs +++ b/src/dfx/src/lib/migrate.rs @@ -31,7 +31,10 @@ pub async fn migrate(env: &dyn Environment, network: &NetworkDescriptor, fix: bo let wallet = if let Ok(wallet) = WalletCanister::create(agent, wallet).await { wallet } else { - let controllers = agent.read_state_canister_controllers(wallet).await?; + let controllers = agent + .read_state_canister_controllers(wallet) + .await? + .context("Canister does not exist")?; bail!("This identity isn't a controller of the wallet. You need to be one of these principals to upgrade the wallet: {}", controllers.into_iter().join(", ")) }; did_migrate |= migrate_wallet(env, agent, &wallet, fix).await?; @@ -65,7 +68,7 @@ async fn migrate_wallet( install_wallet( env, agent, - *wallet.canister_id_(), + *wallet.canister_id(), InstallMode::Upgrade(None), ) .await? @@ -86,9 +89,11 @@ async fn migrate_canister( ident: &Identity, fix: bool, ) -> DfxResult { - let mut controllers = agent.read_state_canister_controllers(canister_id).await?; - if controllers.contains(wallet.canister_id_()) - && !controllers.contains(&ident.sender().unwrap()) + let mut controllers = agent + .read_state_canister_controllers(canister_id) + .await? + .context("Canister does not exist")?; + if controllers.contains(wallet.canister_id()) && !controllers.contains(&ident.sender().unwrap()) { if fix { println!( diff --git a/src/dfx/src/lib/operations/canister/create_canister.rs b/src/dfx/src/lib/operations/canister/create_canister.rs index 8998fcfa9d..4e485e00eb 100644 --- a/src/dfx/src/lib/operations/canister/create_canister.rs +++ b/src/dfx/src/lib/operations/canister/create_canister.rs @@ -16,10 +16,11 @@ use dfx_core::canister::build_wallet_canister; use dfx_core::identity::CallSender; use dfx_core::network::provider::get_network_context; use fn_error_context::context; -use ic_agent::agent::{RejectCode, RejectResponse}; -use ic_agent::agent_error::HttpErrorPayload; -use ic_agent::{Agent, AgentError}; + +use ic_agent::Agent; +use ic_utils::error::CanisterError; use ic_utils::interfaces::management_canister::builders::CanisterSettings; +use ic_utils::interfaces::wallet::WalletError; use ic_utils::interfaces::ManagementCanister; use ic_utils::Argument; use icrc_ledger_types::icrc1::account::Subaccount; @@ -136,7 +137,7 @@ The command line value will be used.", ) .await { - Ok(wallet) => CallSender::Wallet(*wallet.canister_id_()), + Ok(wallet) => CallSender::Wallet(*wallet.canister_id()), Err(err) => { if matches!( err, @@ -229,30 +230,28 @@ async fn create_with_management_canister( using `dfx ledger create-canister`, but doing so will not associate the created canister with any of the canisters in your project."; match res { Ok((o,)) => Ok(o), - Err(AgentError::HttpError(HttpErrorPayload { - status, content, .. - })) if (400..500).contains(&status) => { - let message = String::from_utf8_lossy(&content); - if message.contains( - "does not belong to an existing subnet and it is not a mainnet canister ID.", - ) { - Err(anyhow!("{message}")) - } else { - Err(anyhow!(NEEDS_WALLET)) + Err(e) => { + if let Some(agent_err) = e.as_agent() { + if let Some(http) = agent_err.as_http_error() { + if (400..500).contains(&http.status) { + let message = String::from_utf8_lossy(&http.content); + if message.contains("does not belong to an existing subnet and it is not a mainnet canister ID.") { + bail!("{message}"); + } else { + bail!(NEEDS_WALLET); + } + } + } else if let Some(reject) = agent_err.as_reject() { + if reject + .reject_message + .contains("is not allowed to call ic00 method") + { + bail!(NEEDS_WALLET); + } + } } + Err(e).context("Canister creation call failed.") } - Err(AgentError::UncertifiedReject { - reject: - RejectResponse { - reject_code: RejectCode::CanisterReject, - reject_message, - .. - }, - .. - }) if reject_message.contains("is not allowed to call ic00 method") => { - Err(anyhow!(NEEDS_WALLET)) - } - Err(e) => Err(e).context("Canister creation call failed."), } } @@ -283,10 +282,7 @@ async fn create_with_wallet( } }; - let call_result: Result< - (Result,), - ic_agent::AgentError, - > = wallet + let call_result: Result<(Result,), _> = wallet .call128( MAINNET_CYCLE_MINTER_CANISTER_ID, CMC_CREATE_CANISTER_METHOD, @@ -300,22 +296,27 @@ async fn create_with_wallet( match call_result { Ok((Ok(canister_id),)) => Ok(canister_id), Ok((Err(err),)) => Err(anyhow!(err)), - Err(AgentError::WalletUpgradeRequired(s)) => Err(anyhow!( + Err(WalletError::WalletUpgradeRequired(s)) => Err(anyhow!( "{}\nTo upgrade, run dfx wallet upgrade.", - AgentError::WalletUpgradeRequired(s) + WalletError::WalletUpgradeRequired(s) )), Err(other) => Err(anyhow!(other)), } } else { if settings.reserved_cycles_limit.is_some() { bail!( - "Cannot create a canister using a wallet if the reserved_cycles_limit is set. Please create with --no-wallet or use dfx canister update-settings instead.") + "Cannot create a canister using a wallet if the reserved_cycles_limit is set. Please create with --no-wallet or use dfx canister update-settings instead." + ) } if settings.wasm_memory_limit.is_some() { - bail!("Cannot create a canister using a wallet if the wasm_memory_limit is set. Please create with --no-wallet or use dfx canister update-settings instead.") + bail!( + "Cannot create a canister using a wallet if the wasm_memory_limit is set. Please create with --no-wallet or use dfx canister update-settings instead." + ) } if settings.log_visibility.is_some() { - bail!("Cannot create a canister using a wallet if log_visibility is set. Please create with --no-wallet or use dfx canister update-settings instead.") + bail!( + "Cannot create a canister using a wallet if log_visibility is set. Please create with --no-wallet or use dfx canister update-settings instead." + ) } match wallet .wallet_create_canister( @@ -328,9 +329,9 @@ async fn create_with_wallet( .await { Ok(result) => Ok(result.canister_id), - Err(AgentError::WalletUpgradeRequired(s)) => Err(anyhow!( + Err(WalletError::WalletUpgradeRequired(s)) => Err(anyhow!( "{}\nTo upgrade, run dfx wallet upgrade.", - AgentError::WalletUpgradeRequired(s) + WalletError::WalletUpgradeRequired(s) )), Err(other) => Err(anyhow!(other)), } diff --git a/src/dfx/src/lib/operations/cycles_ledger.rs b/src/dfx/src/lib/operations/cycles_ledger.rs index d06e15a056..00c062a9a6 100644 --- a/src/dfx/src/lib/operations/cycles_ledger.rs +++ b/src/dfx/src/lib/operations/cycles_ledger.rs @@ -16,7 +16,7 @@ use crate::lib::operations::{ ICRC1_BALANCE_OF_METHOD, ICRC1_TRANSFER_METHOD, ICRC2_APPROVE_METHOD, ICRC2_TRANSFER_FROM_METHOD, }; -use crate::lib::retryable::retryable; +use crate::lib::retryable::{canister_retryable, retryable}; use crate::lib::telemetry::{CyclesHost, Telemetry}; use crate::util::clap::subnet_selection_opt::SubnetSelectionType; use anyhow::{anyhow, bail, Context}; @@ -64,10 +64,10 @@ pub async fn balance( .await; match result { Ok((balance,)) => Ok(balance), - Err(agent_err) if retryable(&agent_err) => { - Err(backoff::Error::transient(anyhow!(agent_err))) + Err(canister_err) if canister_retryable(&canister_err) => { + Err(backoff::Error::transient(anyhow!(canister_err))) } - Err(agent_err) => Err(backoff::Error::permanent(anyhow!(agent_err))), + Err(canister_err) => Err(backoff::Error::permanent(anyhow!(canister_err))), } }) .await @@ -122,10 +122,10 @@ pub async fn transfer( Ok(duplicate_of) } Ok(Err(transfer_err)) => Err(backoff::Error::permanent(anyhow!(transfer_err))), - Err(agent_err) if retryable(&agent_err) => { - Err(backoff::Error::transient(anyhow!(agent_err))) + Err(canister_err) if canister_retryable(&canister_err) => { + Err(backoff::Error::transient(anyhow!(canister_err))) } - Err(agent_err) => Err(backoff::Error::permanent(anyhow!(agent_err))), + Err(canister_err) => Err(backoff::Error::permanent(anyhow!(canister_err))), } }) .await?; @@ -179,10 +179,10 @@ pub async fn transfer_from( Ok(Err(transfer_from_err)) => { Err(backoff::Error::permanent(anyhow!(transfer_from_err))) } - Err(agent_err) if retryable(&agent_err) => { - Err(backoff::Error::transient(anyhow!(agent_err))) + Err(canister_err) if canister_retryable(&canister_err) => { + Err(backoff::Error::transient(anyhow!(canister_err))) } - Err(agent_err) => Err(backoff::Error::permanent(anyhow!(agent_err))), + Err(canister_err) => Err(backoff::Error::permanent(anyhow!(canister_err))), } }) .await?; @@ -237,10 +237,10 @@ pub async fn approve( Ok(duplicate_of) } Ok(Err(approve_err)) => Err(backoff::Error::permanent(anyhow!(approve_err))), - Err(agent_err) if retryable(&agent_err) => { - Err(backoff::Error::transient(anyhow!(agent_err))) + Err(canister_err) if canister_retryable(&canister_err) => { + Err(backoff::Error::transient(anyhow!(canister_err))) } - Err(agent_err) => Err(backoff::Error::permanent(anyhow!(agent_err))), + Err(canister_err) => Err(backoff::Error::permanent(anyhow!(canister_err))), } }) .await?; @@ -295,10 +295,10 @@ pub async fn withdraw( "send error: {:?}", send_err ))), - Err(agent_err) if retryable(&agent_err) => { - Err(backoff::Error::transient(anyhow!(agent_err))) + Err(canister_err) if canister_retryable(&canister_err) => { + Err(backoff::Error::transient(anyhow!(canister_err))) } - Err(agent_err) => Err(backoff::Error::permanent(anyhow!(agent_err))), + Err(canister_err) => Err(backoff::Error::permanent(anyhow!(canister_err))), } }) .await?; diff --git a/src/dfx/src/lib/operations/ledger.rs b/src/dfx/src/lib/operations/ledger.rs index 844befc505..37debbf76b 100644 --- a/src/dfx/src/lib/operations/ledger.rs +++ b/src/dfx/src/lib/operations/ledger.rs @@ -17,14 +17,14 @@ use anyhow::{anyhow, bail, ensure, Context}; use backoff::backoff::Backoff; use backoff::future::retry; use backoff::ExponentialBackoff; -use candid::{Decode, Encode, Nat, Principal}; +use candid::{Encode, Nat, Principal}; use fn_error_context::context; -use ic_agent::agent::{RejectCode, RejectResponse}; -use ic_agent::agent_error::HttpErrorPayload; +use ic_agent::agent::RejectCode; use ic_agent::{ hash_tree::{HashTree, LookupResult}, - lookup_value, Agent, AgentError, + lookup_value, Agent, }; +use ic_utils::error::{BaseError, CanisterError}; use ic_utils::{call::SyncCall, Canister}; use icrc_ledger_types::icrc1; use icrc_ledger_types::icrc1::transfer::BlockIndex; @@ -149,51 +149,49 @@ pub async fn transfer( ); let mut retry_policy = ExponentialBackoff::default(); + let canister = Canister::builder() + .with_agent(agent) + .with_canister_id(*canister_id) + .build()?; let block_height: BlockHeight = loop { - match agent - .update(canister_id, TRANSFER_METHOD) - .with_arg( - Encode!(&TransferArgs { - memo, - amount, - fee, - from_subaccount, - to, - created_at_time: Some(TimeStamp { timestamp_nanos }), - }) - .context("Failed to encode arguments.")?, - ) + match canister + .update(TRANSFER_METHOD) + .with_arg(&TransferArgs { + memo, + amount, + fee, + from_subaccount, + to, + created_at_time: Some(TimeStamp { timestamp_nanos }), + }) + .build::<(TransferResult,)>() .await { - Ok(data) => { - let result = Decode!(&data, TransferResult) - .context("Failed to decode transfer response.")?; - match result { - Ok(block_height) => break block_height, - Err(TransferError::TxDuplicate { duplicate_of }) => { - info!(logger, "{}", TransferError::TxDuplicate { duplicate_of }); - break duplicate_of; - } - Err(TransferError::InsufficientFunds { balance }) => { - return Err(anyhow!(TransferError::InsufficientFunds { balance })) - .with_context(|| { - diagnose_insufficient_funds_error(agent, from_subaccount) - }); - } - Err(transfer_err) => bail!(transfer_err), + Ok((result,)) => match result { + Ok(block_height) => break block_height, + Err(TransferError::TxDuplicate { duplicate_of }) => { + info!(logger, "{}", TransferError::TxDuplicate { duplicate_of }); + break duplicate_of; } + Err(TransferError::InsufficientFunds { balance }) => { + return Err(anyhow!(TransferError::InsufficientFunds { balance })) + .with_context(|| { + diagnose_insufficient_funds_error(agent, from_subaccount) + }); + } + Err(transfer_err) => bail!(transfer_err), + }, + Err(canister_err) if !retryable(&canister_err) => { + bail!(canister_err); } - Err(agent_err) if !retryable(&agent_err) => { - bail!(agent_err); - } - Err(agent_err) => match retry_policy.next_backoff() { + Err(canister_err) => match retry_policy.next_backoff() { Some(duration) => { - eprintln!("Waiting to retry after error: {:?}", &agent_err); + eprintln!("Waiting to retry after error: {:?}", &canister_err); tokio::time::sleep(duration).await; println!("Sending duplicate transaction"); } - None => bail!(agent_err), + None => bail!(canister_err), }, } }; @@ -250,10 +248,10 @@ pub async fn icrc1_transfer( Ok(duplicate_of) } Ok(Err(transfer_err)) => Err(backoff::Error::permanent(anyhow!(transfer_err))), - Err(agent_err) if retryable(&agent_err) => { - Err(backoff::Error::transient(anyhow!(agent_err))) + Err(canister_err) if retryable(&canister_err) => { + Err(backoff::Error::transient(anyhow!(canister_err))) } - Err(agent_err) => Err(backoff::Error::permanent(anyhow!(agent_err))), + Err(canister_err) => Err(backoff::Error::permanent(anyhow!(canister_err))), } }) .await?; @@ -438,26 +436,13 @@ Your principal for ICP wallets and decentralized exchanges: {} DiagnosedError::new(explanation, suggestion) } -fn retryable(agent_error: &AgentError) -> bool { - match agent_error { - AgentError::CertifiedReject { - reject: - RejectResponse { - reject_code: RejectCode::CanisterError, - reject_message, - .. - }, - .. - } if reject_message.contains("is out of cycles") => false, - AgentError::HttpError(HttpErrorPayload { - status, - content_type: _, - content: _, - }) if *status == 403 => { - // sometimes out of cycles looks like this - // assume any 403(unauthorized) is not retryable - false - } - _ => true, - } +fn retryable(canister_error: &BaseError) -> bool { + !canister_error.as_agent().is_some_and(|agent_error| { + agent_error.as_reject().is_some_and(|reject| { + reject.reject_code == RejectCode::CanisterError + && reject.reject_message.contains("out of cycles") + }) || agent_error + .as_http_error() + .is_some_and(|http| http.status == 403) + }) } diff --git a/src/dfx/src/lib/retryable.rs b/src/dfx/src/lib/retryable.rs index edc76f241f..f41be7b80e 100644 --- a/src/dfx/src/lib/retryable.rs +++ b/src/dfx/src/lib/retryable.rs @@ -1,8 +1,12 @@ -use ic_agent::AgentError; +use ic_agent::{agent::OperationStatus, AgentError}; +use ic_utils::error::{BaseError, CanisterError}; pub fn retryable(agent_error: &AgentError) -> bool { - matches!( - agent_error, - AgentError::TimeoutWaitingForResponse() | AgentError::TransportError(_) - ) + agent_error + .operation_info() + .is_some_and(|op| op.status == OperationStatus::NotSent) +} + +pub fn canister_retryable(canister_error: &BaseError) -> bool { + canister_error.as_agent().is_some_and(retryable) } diff --git a/src/dfx/src/lib/state_tree/canister_info.rs b/src/dfx/src/lib/state_tree/canister_info.rs index 5a4a7a84ea..f281879f62 100644 --- a/src/dfx/src/lib/state_tree/canister_info.rs +++ b/src/dfx/src/lib/state_tree/canister_info.rs @@ -1,23 +1,16 @@ use crate::lib::error::DfxResult; -use anyhow::{anyhow, bail, Context}; +use anyhow::Context; use candid::Principal; -use ic_agent::{Agent, AgentError}; +use ic_agent::Agent; pub async fn read_state_tree_canister_controllers( agent: &Agent, canister_id: Principal, ) -> DfxResult>> { - let controllers = match agent.read_state_canister_controllers(canister_id).await { - Err(AgentError::LookupPathAbsent(_)) => { - return Ok(None); - } - Err(AgentError::InvalidCborData(_)) => { - return Err(anyhow!("Invalid cbor data in controllers canister info.")); - } - r => r.with_context(|| format!("Failed to read controllers of canister {canister_id}."))?, - }; - - Ok(Some(controllers)) + agent + .read_state_canister_controllers(canister_id) + .await + .with_context(|| format!("Failed to read controllers of canister {canister_id}.")) } /// None can indicate either of these, but we can't tell from here: @@ -27,11 +20,5 @@ pub async fn read_state_tree_canister_module_hash( agent: &Agent, canister_id: Principal, ) -> DfxResult>> { - let module_hash = match agent.read_state_canister_module_hash(canister_id).await { - Ok(blob) => Some(blob), - Err(AgentError::LookupPathAbsent(_)) => None, - Err(x) => bail!(x), - }; - - Ok(module_hash) + Ok(agent.read_state_canister_module_hash(canister_id).await?) } diff --git a/src/dfx/src/lib/subnet.rs b/src/dfx/src/lib/subnet.rs index 684b8c2f73..9635c893b0 100644 --- a/src/dfx/src/lib/subnet.rs +++ b/src/dfx/src/lib/subnet.rs @@ -3,11 +3,11 @@ use anyhow::anyhow; use backoff::future::retry; use backoff::ExponentialBackoff; use candid::{CandidType, Deserialize, Principal}; -use ic_agent::{Agent, AgentError}; +use ic_agent::Agent; use ic_utils::call::SyncCall; use ic_utils::Canister; -use super::retryable::retryable; +use super::retryable::canister_retryable; pub const MAINNET_REGISTRY_CANISTER_ID: Principal = Principal::from_slice(&[0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x01]); @@ -37,14 +37,13 @@ pub async fn get_subnet_for_canister( let arg = GetSubnetForCanisterRequest { principal: Some(canister_id), }; - let result: Result, AgentError> = - registry_canister - .query("get_subnet_for_canister") - .with_arg(arg) - .build() - .call() - .await - .map(|(result,)| result); + let result: Result, _> = registry_canister + .query("get_subnet_for_canister") + .with_arg(arg) + .build() + .call() + .await + .map(|(result,)| result); match result { Ok(Ok(GetSubnetForCanisterResponse { subnet_id: Some(subnet_id), @@ -56,10 +55,10 @@ pub async fn get_subnet_for_canister( "unable to determine subnet: {}", text ))), - Err(agent_err) if retryable(&agent_err) => { - Err(backoff::Error::transient(anyhow!(agent_err))) + Err(canister_err) if canister_retryable(&canister_err) => { + Err(backoff::Error::transient(anyhow!(canister_err))) } - Err(agent_err) => Err(backoff::Error::permanent(anyhow!(agent_err))), + Err(canister_err) => Err(backoff::Error::permanent(anyhow!(canister_err))), } }) .await diff --git a/src/dfx/src/lib/telemetry.rs b/src/dfx/src/lib/telemetry.rs index d60ac376f8..ba9f210236 100644 --- a/src/dfx/src/lib/telemetry.rs +++ b/src/dfx/src/lib/telemetry.rs @@ -19,8 +19,7 @@ use dfx_core::fs; use dfx_core::identity::IdentityType; use fd_lock::{RwLock as FdRwLock, RwLockWriteGuard}; use fn_error_context::context; -use ic_agent::agent::RejectResponse; -use ic_agent::agent_error::Operation; +use ic_agent::agent::{Operation, RejectResponse}; use ic_agent::AgentError; use reqwest::StatusCode; use semver::Version; @@ -164,12 +163,10 @@ impl Telemetry { with_telemetry(|telemetry| { for source in error.chain() { if let Some(agent_err) = source.downcast_ref::() { - if let AgentError::CertifiedReject { reject, operation } - | AgentError::UncertifiedReject { reject, operation } = agent_err - { + if let Some(reject) = agent_err.as_reject() { telemetry.last_reject = Some(reject.clone()); - if let Some(operation) = operation { - telemetry.last_operation = Some(operation.clone()); + if let Some(operation) = agent_err.operation_info() { + telemetry.last_operation = Some(operation.operation.clone()); } } break; @@ -241,7 +238,7 @@ impl Telemetry { try_with_telemetry(|telemetry| { let reject = telemetry.last_reject.as_ref(); let call_site = telemetry.last_operation.as_ref().map(|o| match o { - Operation::Call { method, canister } => { + Operation::Update { method, canister } | Operation::Query { method, canister } => { if telemetry.allowlisted_canisters.contains(canister) { method } else { diff --git a/src/dfx/src/util/mod.rs b/src/dfx/src/util/mod.rs index 0afbab1ef1..1ccdc27d45 100644 --- a/src/dfx/src/util/mod.rs +++ b/src/dfx/src/util/mod.rs @@ -107,7 +107,7 @@ pub async fn read_module_metadata( &agent .read_state_canister_metadata(canister_id, metadata) .await - .ok()?, + .ok()??, ) .into(), )