From aa8394a83839c9c08088eced868178fb4b364c5e Mon Sep 17 00:00:00 2001 From: Alexis Sellier Date: Mon, 29 May 2023 11:33:29 +0200 Subject: [PATCH] cob: Make errors more consistent across COBs Though these are perhaps not the final error types, we name them the same across COBs and have them work the same way. All COB modules now have a `Error` enum. In a future change, we may want the `FromHistory` trait to support multiple error types, eg. one for `apply` and one for `validate`. --- radicle/src/cob/identity.rs | 2 +- radicle/src/cob/issue.rs | 2 +- radicle/src/cob/patch.rs | 32 +++++++++++++------------------- radicle/src/cob/test.rs | 2 +- radicle/src/cob/thread.rs | 10 +++++----- 5 files changed, 21 insertions(+), 27 deletions(-) diff --git a/radicle/src/cob/identity.rs b/radicle/src/cob/identity.rs index 55df7c2d..6b791d02 100644 --- a/radicle/src/cob/identity.rs +++ b/radicle/src/cob/identity.rs @@ -92,7 +92,7 @@ pub enum ApplyError { Redacted(EntryId), /// Error applying an op to the proposal thread. #[error("thread apply failed: {0}")] - Thread(#[from] thread::OpError), + Thread(#[from] thread::Error), /// Error validating the state. #[error("validation failed: {0}")] Validate(&'static str), diff --git a/radicle/src/cob/issue.rs b/radicle/src/cob/issue.rs index 4ba8b5ae..79f74aee 100644 --- a/radicle/src/cob/issue.rs +++ b/radicle/src/cob/issue.rs @@ -39,7 +39,7 @@ pub enum Error { #[error("description missing")] DescriptionMissing, #[error("thread apply failed: {0}")] - Thread(#[from] thread::OpError), + Thread(#[from] thread::Error), #[error("store: {0}")] Store(#[from] store::Error), } diff --git a/radicle/src/cob/patch.rs b/radicle/src/cob/patch.rs index b8a92783..c5ee1f32 100644 --- a/radicle/src/cob/patch.rs +++ b/radicle/src/cob/patch.rs @@ -50,7 +50,7 @@ pub type RevisionIx = usize; /// Error applying an operation onto a state. #[derive(Debug, Error)] -pub enum ApplyError { +pub enum Error { /// Causal dependency missing. /// /// This error indicates that the operations are not being applied @@ -62,7 +62,7 @@ pub enum ApplyError { Missing(EntryId), /// Error applying an op to the patch thread. #[error("thread apply failed: {0}")] - Thread(#[from] thread::OpError), + Thread(#[from] thread::Error), /// Error loading the identity document committed to by an operation. #[error("identity doc failed to load: {0}")] Doc(#[from] DocError), @@ -78,13 +78,7 @@ pub enum ApplyError { /// Validation error. #[error("validation failed: {0}")] Validate(&'static str), -} - -/// Error updating or creating patches. -#[derive(Error, Debug)] -pub enum Error { - #[error("apply failed: {0}")] - Apply(#[from] ApplyError), + /// Store error. #[error("store: {0}")] Store(#[from] store::Error), } @@ -341,7 +335,7 @@ impl Patch { impl store::FromHistory for Patch { type Action = Action; - type Error = ApplyError; + type Error = Error; fn type_name() -> &'static TypeName { &*TYPENAME @@ -349,10 +343,10 @@ impl store::FromHistory for Patch { fn validate(&self) -> Result<(), Self::Error> { if self.revisions.is_empty() { - return Err(ApplyError::Validate("no revisions found")); + return Err(Error::Validate("no revisions found")); } if self.title().is_empty() { - return Err(ApplyError::Validate("empty title")); + return Err(Error::Validate("empty title")); } Ok(()) } @@ -361,7 +355,7 @@ impl store::FromHistory for Patch { &mut self, ops: impl IntoIterator, repo: &R, - ) -> Result<(), ApplyError> { + ) -> Result<(), Error> { for op in ops { let id = op.id; let author = Author::new(op.author); @@ -392,7 +386,7 @@ impl store::FromHistory for Patch { if let Some(Redactable::Present(revision)) = self.revisions.get_mut(&revision) { revision.description.set(description, op.clock); } else { - return Err(ApplyError::Missing(revision)); + return Err(Error::Missing(revision)); } } Action::Revision { @@ -422,7 +416,7 @@ impl store::FromHistory for Patch { if let Some(revision) = self.revisions.get_mut(&revision) { revision.merge(Redactable::Redacted); } else { - return Err(ApplyError::Missing(revision)); + return Err(Error::Missing(revision)); } } Action::Review { @@ -443,7 +437,7 @@ impl store::FromHistory for Patch { ), ); } else { - return Err(ApplyError::Missing(revision)); + return Err(Error::Missing(revision)); } } Action::Merge { revision, commit } => { @@ -453,7 +447,7 @@ impl store::FromHistory for Patch { match self.target() { MergeTarget::Delegates => { if !doc.is_delegate(&op.author) { - return Err(ApplyError::InvalidMerge(op.id)); + return Err(Error::InvalidMerge(op.id)); } let proj = doc.project()?; let branch = git::refs::branch(proj.default_branch()); @@ -519,7 +513,7 @@ impl store::FromHistory for Patch { } } } else { - return Err(ApplyError::Missing(revision)); + return Err(Error::Missing(revision)); } } Action::Thread { revision, action } => { @@ -538,7 +532,7 @@ impl store::FromHistory for Patch { repo, )?; } else { - return Err(ApplyError::Missing(revision)); + return Err(Error::Missing(revision)); } } } diff --git a/radicle/src/cob/test.rs b/radicle/src/cob/test.rs index fddd7d95..214417f0 100644 --- a/radicle/src/cob/test.rs +++ b/radicle/src/cob/test.rs @@ -192,7 +192,7 @@ impl Actor { base: git::Oid, oid: git::Oid, repo: &R, - ) -> Result { + ) -> Result { Patch::from_ops( [ self.op(patch::Action::Revision { diff --git a/radicle/src/cob/thread.rs b/radicle/src/cob/thread.rs index 33539ca0..b7083453 100644 --- a/radicle/src/cob/thread.rs +++ b/radicle/src/cob/thread.rs @@ -21,7 +21,7 @@ pub static TYPENAME: Lazy = /// Error applying an operation onto a state. #[derive(Error, Debug)] -pub enum OpError { +pub enum Error { /// Causal dependency missing. /// /// This error indicates that the operations are not being applied @@ -261,7 +261,7 @@ impl Thread { impl cob::store::FromHistory for Thread { type Action = Action; - type Error = OpError; + type Error = Error; fn type_name() -> &'static radicle_cob::TypeName { &*TYPENAME @@ -269,7 +269,7 @@ impl cob::store::FromHistory for Thread { fn validate(&self) -> Result<(), Self::Error> { if self.comments.is_empty() { - return Err(OpError::Validate("no comments found")); + return Err(Error::Validate("no comments found")); } Ok(()) } @@ -278,7 +278,7 @@ impl cob::store::FromHistory for Thread { &mut self, ops: impl IntoIterator>, _repo: &R, - ) -> Result<(), OpError> { + ) -> Result<(), Error> { for op in ops.into_iter() { let id = op.id; let author = op.author; @@ -304,7 +304,7 @@ impl cob::store::FromHistory for Thread { if let Some(Redactable::Present(comment)) = self.comments.get_mut(&id) { comment.edit(op.clock, body, timestamp); } else { - return Err(OpError::Missing(id)); + return Err(Error::Missing(id)); } } Action::Redact { id } => {