refactor: remove PrincipalBufferError (#193) (#194)

Switched from `ExternalError` to `dyn Error` in CanisterBuilder.

Fixes #193.

Author: Daniel-Bloom-dfinity

ic-types/src/principal.rs

@@ -12,38 +12,20 @@ macro_rules! const_panic {
     };
 }
 
-/// An error happened while decoding bytes to make a principal.
-#[derive(Error, Clone, Debug, Eq, PartialEq)]
-pub enum PrincipalBufferError {
-    #[error("Buffer is too long.")]
-    BufferTooLong(),
-}
-
 /// An error happened while encoding, decoding or serializing a principal.
 #[derive(Error, Clone, Debug, Eq, PartialEq)]
 pub enum PrincipalError {
     #[error("Buffer is too long.")]
     BufferTooLong(),
 
     #[error(r#"Invalid textual format: expected "{0}""#)]
-    AbnormalTextualFormat(String),
+    AbnormalTextualFormat(Principal),
 
     #[error("Text must be a base 32 string.")]
     InvalidTextualFormatNotBase32(),
 
     #[error("Text cannot be converted to a Principal; too small.")]
     TextTooSmall(),
-
-    #[error("A custom tool returned an error instead of a Principal: {0}")]
-    ExternalError(String),
-}
-
-impl From<PrincipalBufferError> for PrincipalError {
-    fn from(v: PrincipalBufferError) -> Self {
-        match v {
-            PrincipalBufferError::BufferTooLong() => Self::BufferTooLong(),
-        }
-    }
 }
 
 /// A class of principal. Because this should not be exposed it
@@ -204,13 +186,13 @@ impl Principal {
     }
 
     /// Attempt to decode a slice into a Principal.
-    pub const fn try_from_slice(bytes: &[u8]) -> Result<Self, PrincipalBufferError> {
+    pub const fn try_from_slice(bytes: &[u8]) -> Result<Self, PrincipalError> {
         match bytes {
             [] => Ok(Principal::management_canister()),
             [4] => Ok(Principal::anonymous()),
-            [.., 4] => Err(PrincipalBufferError::BufferTooLong()),
+            [.., 4] => Err(PrincipalError::BufferTooLong()),
             bytes @ [..] => match PrincipalInner::try_from_slice(bytes) {
-                None => Err(PrincipalBufferError::BufferTooLong()),
+                None => Err(PrincipalError::BufferTooLong()),
                 Some(v) => Ok(Principal(v)),
             },
         }
@@ -235,7 +217,7 @@ impl Principal {
                 let expected = format!("{}", result);
 
                 if text.as_ref() != expected {
-                    return Err(PrincipalError::AbnormalTextualFormat(expected));
+                    return Err(PrincipalError::AbnormalTextualFormat(result));
                 }
                 Ok(result)
             }
@@ -302,15 +284,15 @@ impl TryFrom<&str> for Principal {
 
 /// Vector TryFrom. The slice and array version of this trait are defined below.
 impl TryFrom<Vec<u8>> for Principal {
-    type Error = PrincipalBufferError;
+    type Error = PrincipalError;
 
     fn try_from(bytes: Vec<u8>) -> Result<Self, Self::Error> {
         Self::try_from(bytes.as_slice())
     }
 }
 
 impl TryFrom<&Vec<u8>> for Principal {
-    type Error = PrincipalBufferError;
+    type Error = PrincipalError;
 
     fn try_from(bytes: &Vec<u8>) -> Result<Self, Self::Error> {
         Self::try_from(bytes.as_slice())
@@ -319,7 +301,7 @@ impl TryFrom<&Vec<u8>> for Principal {
 
 /// Implement try_from for a generic sized slice.
 impl TryFrom<&[u8]> for Principal {
-    type Error = PrincipalBufferError;
+    type Error = PrincipalError;
 
     fn try_from(bytes: &[u8]) -> Result<Self, Self::Error> {
         Self::try_from_slice(bytes)
@@ -539,6 +521,14 @@ mod tests {
         );
     }
 
+    #[test]
+    fn parse_text_bad_format() {
+        assert_eq!(
+            Principal::from_str("aaaaa-aA").unwrap_err().to_string(),
+            r#"Invalid textual format: expected "aaaaa-aa""#,
+        );
+    }
+
     #[test]
     fn parse_management_canister_to_text_ok() {
         assert_eq!(Principal::from_str("aaaaa-aa").unwrap().as_slice(), &[]);

ic-utils/src/canister.rs

@@ -4,15 +4,15 @@ use candid::parser::value::IDLValue;
 use candid::ser::IDLBuilder;
 use candid::CandidType;
 use ic_agent::{Agent, AgentError};
-use ic_types::{Principal, PrincipalError};
+use ic_types::Principal;
 use std::convert::TryInto;
 use thiserror::Error;
 
 /// An error happened while building a canister.
 #[derive(Debug, Error)]
 pub enum CanisterBuilderError {
     #[error("Getting the Canister ID returned an error: {0}")]
-    PrincipalError(#[from] PrincipalError),
+    PrincipalError(#[from] Box<dyn std::error::Error>),
 
     #[error("Must specify an Agent")]
     MustSpecifyAnAgent(),
@@ -24,22 +24,22 @@ pub enum CanisterBuilderError {
 /// A canister builder, which can be used to create a canister abstraction.
 pub struct CanisterBuilder<'agent, T = ()> {
     agent: Option<&'agent Agent>,
-    canister_id: Option<Result<Principal, PrincipalError>>,
+    canister_id: Option<Result<Principal, CanisterBuilderError>>,
     interface: T,
 }
 
 impl<'agent, T> CanisterBuilder<'agent, T> {
     /// Attach a canister ID to this canister.
     pub fn with_canister_id<E, P>(self, canister_id: P) -> Self
     where
-        E: std::error::Error,
+        E: 'static + std::error::Error,
         P: TryInto<Principal, Error = E>,
     {
         Self {
             canister_id: Some(
                 canister_id
                     .try_into()
-                    .map_err(|e| PrincipalError::ExternalError(format!("{}", e))),
+                    .map_err(|e| CanisterBuilderError::PrincipalError(Box::new(e))),
             ),
             ..self
         }