Switch thiserror with anyhow based errors in application code

This should simplify most code.

crates/gitbutler-core/src/fs.rs

@@ -5,7 +5,7 @@ use std::{
     path::{Path, PathBuf},
 };
 
-use anyhow::Result;
+use anyhow::{Context, Result};
 use bstr::BString;
 use gix::{
     dir::walk::EmissionMode,
@@ -105,9 +105,7 @@ fn persist_tempfile(
 ///
 /// If the file does not exist, it will be created.
 // TODO(ST): make this anyhow.
-pub(crate) fn read_toml_file_or_default<T: DeserializeOwned + Default>(
-    path: &Path,
-) -> Result<T, crate::reader::Error> {
+pub(crate) fn read_toml_file_or_default<T: DeserializeOwned + Default>(path: &Path) -> Result<T> {
     let mut file = match File::open(path) {
         Ok(f) => f,
         Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(T::default()),
@@ -115,9 +113,7 @@ pub(crate) fn read_toml_file_or_default<T: DeserializeOwned + Default>(
     };
     let mut contents = String::new();
     file.read_to_string(&mut contents)?;
-    let value: T = toml::from_str(&contents).map_err(|err| crate::reader::Error::ParseError {
-        path: path.to_owned(),
-        source: err,
-    })?;
+    let value: T =
+        toml::from_str(&contents).with_context(|| format!("Failed to parse {}", path.display()))?;
     Ok(value)
 }

crates/gitbutler-core/src/git/error.rs

@@ -1,9 +1,6 @@
 use std::str::Utf8Error;
 
-use crate::{
-    error::{Context, ErrorWithContext},
-    keys,
-};
+use crate::error::{Context, ErrorWithContext};
 
 #[derive(Debug, thiserror::Error)]
 pub enum Error {
@@ -11,8 +8,6 @@ pub enum Error {
     NotFound(git2::Error),
     #[error("authentication failed")]
     Auth(git2::Error),
-    #[error("sign error: {0}")]
-    Signing(keys::SignError),
     #[error("remote url error: {0}")]
     Url(super::url::ParseError),
     #[error("io error: {0}")]
@@ -60,12 +55,6 @@ impl ErrorWithContext for Error {
     }
 }
 
-impl From<keys::SignError> for Error {
-    fn from(err: keys::SignError) -> Self {
-        Error::Signing(err)
-    }
-}
-
 impl From<super::url::ParseError> for Error {
     fn from(err: super::url::ParseError) -> Self {
         Error::Url(err)

crates/gitbutler-core/src/keys/key.rs

@@ -7,12 +7,6 @@ use ssh_key::{HashAlg, LineEnding, SshSig};
 #[derive(Debug, Clone, Eq)]
 pub struct PrivateKey(ssh_key::PrivateKey);
 
-#[derive(Debug, thiserror::Error)]
-pub enum SignError {
-    #[error(transparent)]
-    Ssh(#[from] ssh_key::Error),
-}
-
 impl PrivateKey {
     pub fn generate() -> Self {
         Self::default()
@@ -22,7 +16,7 @@ impl PrivateKey {
         PublicKey::from(self)
     }
 
-    pub fn sign(&self, bytes: &[u8]) -> Result<String, SignError> {
+    pub fn sign(&self, bytes: &[u8]) -> anyhow::Result<String> {
         let sig = SshSig::sign(&self.0, "git", HashAlg::Sha512, bytes)?;
         sig.to_pem(LineEnding::default()).map_err(Into::into)
     }

crates/gitbutler-core/src/keys/mod.rs

@@ -3,4 +3,4 @@ mod key;
 pub mod storage;
 
 pub use controller::*;
-pub use key::{PrivateKey, PublicKey, SignError};
+pub use key::{PrivateKey, PublicKey};

crates/gitbutler-core/src/keys/storage.rs

@@ -1,5 +1,6 @@
 use super::PrivateKey;
 use crate::storage;
+use anyhow::Result;
 use std::path::PathBuf;
 
 // TODO(ST): get rid of this type, it's more trouble than it's worth.
@@ -8,14 +9,6 @@ pub struct Storage {
     inner: storage::Storage,
 }
 
-#[derive(Debug, thiserror::Error)]
-pub enum Error {
-    #[error(transparent)]
-    Storage(#[from] std::io::Error),
-    #[error("SSH key error: {0}")]
-    SSHKey(#[from] ssh_key::Error),
-}
-
 impl Storage {
     pub fn new(storage: storage::Storage) -> Storage {
         Storage { inner: storage }
@@ -25,13 +18,13 @@ impl Storage {
         Storage::new(storage::Storage::new(path))
     }
 
-    pub fn get(&self) -> Result<Option<PrivateKey>, Error> {
+    pub fn get(&self) -> Result<Option<PrivateKey>> {
         let key = self.inner.read("keys/ed25519")?;
         key.map(|s| s.parse().map_err(Into::into)).transpose()
     }
 
     // TODO(ST): see if Key should rather deal with bytes instead for this kind of serialization.
-    pub fn create(&self, key: &PrivateKey) -> Result<(), Error> {
+    pub fn create(&self, key: &PrivateKey) -> Result<()> {
         self.inner.write("keys/ed25519", &key.to_string())?;
         self.inner
             .write("keys/ed25519.pub", &key.public_key().to_string())?;

crates/gitbutler-core/src/ops/state.rs

@@ -88,7 +88,7 @@ impl OplogHandle {
     ///
     /// If the file does not exist, it will be created.
     fn read_file(&self) -> Result<Oplog> {
-        Ok(read_toml_file_or_default(&self.file_path)?)
+        read_toml_file_or_default(&self.file_path)
     }
 
     fn write_file(&self, mut oplog: Oplog) -> Result<()> {

crates/gitbutler-core/src/projects/controller.rs

@@ -149,7 +149,7 @@ impl Controller {
         #[cfg(windows)]
         let project = &project_owned;
 
-        Ok(self.projects_storage.update(project)?)
+        self.projects_storage.update(project)
     }
 
     pub fn get(&self, id: ProjectId) -> Result<Project> {
@@ -182,11 +182,9 @@ impl Controller {
     }
 
     pub async fn delete(&self, id: ProjectId) -> Result<(), Error> {
-        let project = match self.projects_storage.get(id) {
-            Ok(project) => Ok(project),
-            Err(super::storage::Error::NotFound) => return Ok(()),
-            Err(error) => Err(Error::from_err(error)),
-        }?;
+        let Some(project) = self.projects_storage.try_get(id)? else {
+            return Ok(());
+        };
 
         if let Some(watchers) = &self.watchers {
             watchers.stop(id).await;

crates/gitbutler-core/src/projects/storage.rs

@@ -1,3 +1,4 @@
+use anyhow::{Context, Result};
 use serde::{Deserialize, Serialize};
 use std::path::PathBuf;
 
@@ -29,16 +30,6 @@ pub struct UpdateRequest {
     pub snapshot_lines_threshold: Option<usize>,
 }
 
-#[derive(Debug, thiserror::Error)]
-pub enum Error {
-    #[error(transparent)]
-    Storage(#[from] std::io::Error),
-    #[error(transparent)]
-    Json(#[from] serde_json::Error),
-    #[error("project not found")]
-    NotFound,
-}
-
 impl Storage {
     pub fn new(storage: storage::Storage) -> Self {
         Self { inner: storage }
@@ -48,11 +39,11 @@ impl Storage {
         Self::new(storage::Storage::new(path))
     }
 
-    pub fn list(&self) -> Result<Vec<project::Project>, Error> {
+    pub fn list(&self) -> Result<Vec<project::Project>> {
         match self.inner.read(PROJECTS_FILE)? {
             Some(projects) => {
                 let all_projects: Vec<project::Project> = serde_json::from_str(&projects)?;
-                let mut all_projects: Vec<project::Project> = all_projects
+                let mut all_projects: Vec<_> = all_projects
                     .into_iter()
                     .map(|mut p| {
                         // backwards compatibility for description field
@@ -66,27 +57,28 @@ impl Storage {
                     .collect();
 
                 all_projects.sort_by(|a, b| a.title.cmp(&b.title));
-
                 Ok(all_projects)
             }
             None => Ok(vec![]),
         }
     }
 
-    pub fn get(&self, id: ProjectId) -> Result<project::Project, Error> {
-        let projects = self.list()?;
-        match projects.into_iter().find(|p| p.id == id) {
-            Some(project) => Ok(project),
-            None => Err(Error::NotFound),
-        }
+    pub fn get(&self, id: ProjectId) -> Result<project::Project> {
+        self.try_get(id)?
+            .with_context(|| format!("project {id} not found"))
     }
 
-    pub fn update(&self, update_request: &UpdateRequest) -> Result<project::Project, Error> {
+    pub fn try_get(&self, id: ProjectId) -> Result<Option<project::Project>> {
+        let projects = self.list()?;
+        Ok(projects.into_iter().find(|p| p.id == id))
+    }
+
+    pub fn update(&self, update_request: &UpdateRequest) -> Result<project::Project> {
         let mut projects = self.list()?;
         let project = projects
             .iter_mut()
             .find(|p| p.id == update_request.id)
-            .ok_or(Error::NotFound)?;
+            .with_context(|| "project {id} not found for update")?;
 
         if let Some(title) = &update_request.title {
             project.title.clone_from(title);
@@ -140,7 +132,7 @@ impl Storage {
             .clone())
     }
 
-    pub fn purge(&self, id: ProjectId) -> Result<(), Error> {
+    pub fn purge(&self, id: ProjectId) -> Result<()> {
         let mut projects = self.list()?;
         if let Some(index) = projects.iter().position(|p| p.id == id) {
             projects.remove(index);
@@ -150,7 +142,7 @@ impl Storage {
         Ok(())
     }
 
-    pub fn add(&self, project: &project::Project) -> Result<(), Error> {
+    pub fn add(&self, project: &project::Project) -> Result<()> {
         let mut projects = self.list()?;
         projects.push(project.clone());
         let projects = serde_json::to_string_pretty(&projects)?;

crates/gitbutler-core/src/reader.rs

@@ -1,52 +1,8 @@
-use std::{
-    fs, io, num,
-    path::{Path, PathBuf},
-    str,
-    sync::Arc,
-};
+use std::{fs, io, path::Path, str};
 
-use anyhow::Result;
+use anyhow::{bail, Result};
 use serde::{ser::SerializeStruct, Serialize};
 
-#[derive(Debug, Clone, thiserror::Error)]
-pub enum Error {
-    #[error("file not found")]
-    NotFound,
-    #[error("io error: {0}")]
-    Io(Arc<io::Error>),
-    #[error(transparent)]
-    From(FromError),
-    #[error("failed to parse {}", path.display())]
-    ParseError {
-        path: PathBuf,
-        source: toml::de::Error,
-    },
-}
-
-impl From<io::Error> for Error {
-    fn from(error: io::Error) -> Self {
-        Error::Io(Arc::new(error))
-    }
-}
-
-impl From<FromError> for Error {
-    fn from(error: FromError) -> Self {
-        Error::From(error)
-    }
-}
-
-#[derive(Debug, Clone, thiserror::Error)]
-pub enum FromError {
-    #[error(transparent)]
-    ParseInt(#[from] num::ParseIntError),
-    #[error(transparent)]
-    ParseBool(#[from] str::ParseBoolError),
-    #[error("file is binary")]
-    Binary,
-    #[error("file too large")]
-    Large,
-}
-
 #[derive(Debug, Clone, PartialEq)]
 pub enum Content {
     UTF8(String),
@@ -118,19 +74,19 @@ impl From<&[u8]> for Content {
 }
 
 impl TryFrom<&Content> for usize {
-    type Error = FromError;
+    type Error = anyhow::Error;
 
     fn try_from(content: &Content) -> Result<Self, Self::Error> {
         match content {
-            Content::UTF8(text) => text.parse().map_err(FromError::ParseInt),
-            Content::Binary => Err(FromError::Binary),
-            Content::Large => Err(FromError::Large),
+            Content::UTF8(text) => text.parse().map_err(Into::into),
+            Content::Binary => bail!("file is binary"),
+            Content::Large => bail!("file too large"),
         }
     }
 }
 
 impl TryFrom<Content> for usize {
-    type Error = FromError;
+    type Error = anyhow::Error;
 
     fn try_from(content: Content) -> Result<Self, Self::Error> {
         Self::try_from(&content)
@@ -138,19 +94,19 @@ impl TryFrom<Content> for usize {
 }
 
 impl TryFrom<&Content> for String {
-    type Error = FromError;
+    type Error = anyhow::Error;
 
     fn try_from(content: &Content) -> Result<Self, Self::Error> {
         match content {
             Content::UTF8(text) => Ok(text.clone()),
-            Content::Binary => Err(FromError::Binary),
-            Content::Large => Err(FromError::Large),
+            Content::Binary => bail!("file is binary"),
+            Content::Large => bail!("file too large"),
         }
     }
 }
 
 impl TryFrom<Content> for String {
-    type Error = FromError;
+    type Error = anyhow::Error;
 
     fn try_from(content: Content) -> Result<Self, Self::Error> {
         Self::try_from(&content)
@@ -158,7 +114,7 @@ impl TryFrom<Content> for String {
 }
 
 impl TryFrom<Content> for i64 {
-    type Error = FromError;
+    type Error = anyhow::Error;
 
     fn try_from(content: Content) -> Result<Self, Self::Error> {
         Self::try_from(&content)
@@ -166,16 +122,16 @@ impl TryFrom<Content> for i64 {
 }
 
 impl TryFrom<&Content> for i64 {
-    type Error = FromError;
+    type Error = anyhow::Error;
 
     fn try_from(content: &Content) -> Result<Self, Self::Error> {
         let text: String = content.try_into()?;
-        text.parse().map_err(FromError::ParseInt)
+        text.parse().map_err(Into::into)
     }
 }
 
 impl TryFrom<Content> for u64 {
-    type Error = FromError;
+    type Error = anyhow::Error;
 
     fn try_from(content: Content) -> Result<Self, Self::Error> {
         Self::try_from(&content)
@@ -183,16 +139,16 @@ impl TryFrom<Content> for u64 {
 }
 
 impl TryFrom<&Content> for u64 {
-    type Error = FromError;
+    type Error = anyhow::Error;
 
     fn try_from(content: &Content) -> Result<Self, Self::Error> {
         let text: String = content.try_into()?;
-        text.parse().map_err(FromError::ParseInt)
+        text.parse().map_err(Into::into)
     }
 }
 
 impl TryFrom<Content> for u128 {
-    type Error = FromError;
+    type Error = anyhow::Error;
 
     fn try_from(content: Content) -> Result<Self, Self::Error> {
         Self::try_from(&content)
@@ -200,16 +156,16 @@ impl TryFrom<Content> for u128 {
 }
 
 impl TryFrom<&Content> for u128 {
-    type Error = FromError;
+    type Error = anyhow::Error;
 
     fn try_from(content: &Content) -> Result<Self, Self::Error> {
         let text: String = content.try_into()?;
-        text.parse().map_err(FromError::ParseInt)
+        text.parse().map_err(Into::into)
     }
 }
 
 impl TryFrom<Content> for bool {
-    type Error = FromError;
+    type Error = anyhow::Error;
 
     fn try_from(content: Content) -> Result<Self, Self::Error> {
         Self::try_from(&content)
@@ -217,10 +173,10 @@ impl TryFrom<Content> for bool {
 }
 
 impl TryFrom<&Content> for bool {
-    type Error = FromError;
+    type Error = anyhow::Error;
 
     fn try_from(content: &Content) -> Result<Self, Self::Error> {
         let text: String = content.try_into()?;
-        text.parse().map_err(FromError::ParseBool)
+        text.parse().map_err(Into::into)
     }
 }

crates/gitbutler-core/src/users/storage.rs

@@ -10,14 +10,6 @@ pub struct Storage {
     inner: storage::Storage,
 }
 
-#[derive(Debug, thiserror::Error)]
-pub enum Error {
-    #[error(transparent)]
-    Storage(#[from] std::io::Error),
-    #[error(transparent)]
-    Json(#[from] serde_json::Error),
-}
-
 impl Storage {
     pub fn new(storage: storage::Storage) -> Storage {
         Storage { inner: storage }
@@ -27,21 +19,19 @@ impl Storage {
         Storage::new(storage::Storage::new(path))
     }
 
-    pub fn get(&self) -> Result<Option<user::User>, Error> {
+    pub fn get(&self) -> Result<Option<user::User>> {
         match self.inner.read(USER_FILE)? {
             Some(data) => Ok(Some(serde_json::from_str(&data)?)),
             None => Ok(None),
         }
     }
 
-    pub fn set(&self, user: &user::User) -> Result<(), Error> {
+    pub fn set(&self, user: &user::User) -> Result<()> {
         let data = serde_json::to_string(user)?;
-        self.inner.write(USER_FILE, &data)?;
-        Ok(())
+        Ok(self.inner.write(USER_FILE, &data)?)
     }
 
-    pub fn delete(&self) -> Result<(), Error> {
-        self.inner.delete(USER_FILE)?;
-        Ok(())
+    pub fn delete(&self) -> Result<()> {
+        Ok(self.inner.delete(USER_FILE)?)
     }
 }

crates/gitbutler-core/src/virtual_branches/state.rs

@@ -4,7 +4,7 @@ use std::{
 };
 
 use crate::fs::read_toml_file_or_default;
-use anyhow::anyhow;
+use anyhow::{anyhow, Result};
 use serde::{Deserialize, Serialize};
 
 use super::{target::Target, Branch};
@@ -38,7 +38,7 @@ impl VirtualBranchesHandle {
     /// Persists the default target for the given repository.
     ///
     /// Errors if the file cannot be read or written.
-    pub fn set_default_target(&self, target: Target) -> anyhow::Result<()> {
+    pub fn set_default_target(&self, target: Target) -> Result<()> {
         let mut virtual_branches = self.read_file()?;
         virtual_branches.default_target = Some(target);
         self.write_file(&virtual_branches)?;
@@ -48,7 +48,7 @@ impl VirtualBranchesHandle {
     /// Gets the default target for the given repository.
     ///
     /// Errors if the file cannot be read or written.
-    pub fn get_default_target(&self) -> anyhow::Result<Target> {
+    pub fn get_default_target(&self) -> Result<Target> {
         let virtual_branches = self.read_file();
         virtual_branches?
             .default_target
@@ -58,7 +58,7 @@ impl VirtualBranchesHandle {
     /// Sets the target for the given virtual branch.
     ///
     /// Errors if the file cannot be read or written.
-    pub fn set_branch_target(&self, id: BranchId, target: Target) -> anyhow::Result<()> {
+    pub fn set_branch_target(&self, id: BranchId, target: Target) -> Result<()> {
         let mut virtual_branches = self.read_file()?;
         virtual_branches.branch_targets.insert(id, target);
         self.write_file(&virtual_branches)?;
@@ -68,7 +68,7 @@ impl VirtualBranchesHandle {
     /// Sets the state of the given virtual branch.
     ///
     /// Errors if the file cannot be read or written.
-    pub fn set_branch(&self, branch: Branch) -> anyhow::Result<()> {
+    pub fn set_branch(&self, branch: Branch) -> Result<()> {
         let mut virtual_branches = self.read_file()?;
         virtual_branches.branches.insert(branch.id, branch);
         self.write_file(&virtual_branches)?;
@@ -78,7 +78,7 @@ impl VirtualBranchesHandle {
     /// Removes the given virtual branch.
     ///
     /// Errors if the file cannot be read or written.
-    pub fn remove_branch(&self, id: BranchId) -> anyhow::Result<()> {
+    pub fn remove_branch(&self, id: BranchId) -> Result<()> {
         let mut virtual_branches = self.read_file()?;
         virtual_branches.branches.remove(&id);
         self.write_file(&virtual_branches)?;
@@ -88,14 +88,14 @@ impl VirtualBranchesHandle {
     /// Gets the state of the given virtual branch.
     ///
     /// Errors if the file cannot be read or written.
-    pub fn get_branch(&self, id: BranchId) -> anyhow::Result<Branch> {
+    pub fn get_branch(&self, id: BranchId) -> Result<Branch> {
         self.try_branch(id)?
             .ok_or_else(|| anyhow!("branch with ID {id} not found"))
     }
 
     /// Gets the state of the given virtual branch returning `Some(branch)` or `None`
     /// if that branch doesn't exist.
-    pub fn try_branch(&self, id: BranchId) -> anyhow::Result<Option<Branch>> {
+    pub fn try_branch(&self, id: BranchId) -> Result<Option<Branch>> {
         let virtual_branches = self.read_file()?;
         Ok(virtual_branches.branches.get(&id).cloned())
     }
@@ -103,7 +103,7 @@ impl VirtualBranchesHandle {
     /// Lists all virtual branches.
     ///
     /// Errors if the file cannot be read or written.
-    pub fn list_branches(&self) -> anyhow::Result<Vec<Branch>> {
+    pub fn list_branches(&self) -> Result<Vec<Branch>> {
         let virtual_branches = self.read_file()?;
         let branches: Vec<Branch> = virtual_branches.branches.values().cloned().collect();
         Ok(branches)
@@ -119,15 +119,15 @@ impl VirtualBranchesHandle {
     /// Reads and parses the state file.
     ///
     /// If the file does not exist, it will be created.
-    fn read_file(&self) -> Result<VirtualBranches, crate::reader::Error> {
+    fn read_file(&self) -> Result<VirtualBranches> {
         read_toml_file_or_default(&self.file_path)
     }
 
-    fn write_file(&self, virtual_branches: &VirtualBranches) -> anyhow::Result<()> {
+    fn write_file(&self, virtual_branches: &VirtualBranches) -> Result<()> {
         write(self.file_path.as_path(), virtual_branches)
     }
 }
 
-fn write<P: AsRef<Path>>(file_path: P, virtual_branches: &VirtualBranches) -> anyhow::Result<()> {
+fn write<P: AsRef<Path>>(file_path: P, virtual_branches: &VirtualBranches) -> Result<()> {
     crate::fs::write(file_path, toml::to_string(&virtual_branches)?)
 }