gitbutlerapp/gitbutler
1
1
Fork 0
mirror of https://github.com/gitbutlerapp/gitbutler.git synced 2024-11-28 22:03:30 +03:00
Code Issues Packages Projects Releases Wiki Activity

simplify core::errors and remove all the cruft

Browse Source
This commit is contained in:
Sebastian Thiel 2024-05-31 19:34:04 +02:00
parent 0ef944ed25
commit 567a077582
No known key found for this signature in database
GPG Key ID: 9CB5EE7895E8268B
14 changed files with 114 additions and 364 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

142
crates/gitbutler-core/src/error.rs
View File

@ -56,59 +56,10 @@
//!
//! #### With `thiserror`
//!
//! `thiserror` doesn't have a mechanism for generic context, which is why it has to be attached to
//! each type that is generated by thiserror.
//! `thiserror` doesn't have a mechanism for generic context, and if it's needed the error must be converted to `anyhow::Error`.
//!
//! By default, `thiserror` instances have no context.
//!
//!```rust
//! # use gitbutler_core::error::{Code, Context, ErrorWithContext};
//! #[derive(thiserror::Error, Debug)]
//! #[error("user message")]
//! struct Error;
//!
//! // But context can be added like this:
//! impl ErrorWithContext for Error {fn context(&self) -> Option<Context> {
//! // attach a code to make it show up in higher layers.
//! Some(Context::from(Code::Unknown))
//! }
//! }
//! ```
//!
//! Note that it's up to the implementation of [`ErrorWithContext`] to collect all context of errors in variants.
//!
//!```rust
//! # use gitbutler_core::error::{AnyhowContextExt, Code, Context, ErrorWithContext};
//!
//! #[derive(thiserror::Error, Debug)]
//! #[error("user message")]
//! struct TinyError;
//!
//! // But context can be added like this:
//! impl ErrorWithContext for TinyError {
//! fn context(&self) -> Option<Context> {
//! Some(Context::new_static(Code::Unknown, "tiny message"))
//! }
//! }
//! #[derive(thiserror::Error, Debug)]
//! enum Error {
//! #[error(transparent)]
//! Tiny(#[from] TinyError),
//! #[error(transparent)]
//! Other(#[from] anyhow::Error)
//! };
//!
//! // But context can be added like this:
//! impl ErrorWithContext for Error {
//! fn context(&self) -> Option<Context> {
//! match self {
//! Error::Tiny(err) => err.context(),
//! Error::Other(err) => err.custom_context()
//! }
//! }
//! }
//! ```
//!
//! ### Assuring Context
//!
//! Currently, the consumers of errors with context are quite primitive and thus rely on `anyhow`
@ -120,7 +71,7 @@
//! here. It is made to only automatically convert from types that have context information.
//! Those who have not will need to be converted by hand using [`Error::from_err()`].
use std::borrow::Cow;
use std::fmt::{Debug, Display, Formatter};
use std::fmt::Debug;
/// A unique code that consumers of the API may rely on to identify errors.
///
@ -235,92 +186,3 @@ impl AnyhowContextExt for anyhow::Error {
})
}
}
/// A trait that if implemented on `thiserror` instance, allows to extract context we provide
/// in its variants.
///
/// Note that this is a workaround for the inability to control or implement the `provide()` method
/// on the `std::error::Error` implementation of `thiserror`.
pub trait ErrorWithContext: std::error::Error {
/// Obtain the [`Context`], if present.
fn context(&self) -> Option<Context>;
}
/// Convert `err` into an `anyhow` error, but also add provided `Code` or `Context` as anyhow context.
/// This uses the new `provide()` API to attach arbitrary information to error implementations.
pub fn into_anyhow(err: impl ErrorWithContext + Send + Sync + 'static) -> anyhow::Error {
let context = err.context();
let err = anyhow::Error::from(err);
if let Some(context) = context {
err.context(context)
} else {
err
}
}
/// A wrapper around an `anyhow` error which automatically extracts the context that might be attached
/// to `thiserror` instances.
///
/// Whenever `thiserror` is involved, this error type should be used if the alternative would be to write
/// a `thiserror` which just forwards its context (like `app::Error` previously).
#[derive(Debug)]
pub struct Error(anyhow::Error);
impl From<anyhow::Error> for Error {
fn from(value: anyhow::Error) -> Self {
Self(value)
}
}
impl From<Error> for anyhow::Error {
fn from(value: Error) -> Self {
value.0
}
}
impl<E> From<E> for Error
where
E: ErrorWithContext + Send + Sync + 'static,
{
fn from(value: E) -> Self {
Self(into_anyhow(value))
}
}
impl Display for Error {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
Display::fmt(&self.0, f)
}
}
impl Error {
/// A manual, none-overlapping implementation of `From` (or else there are conflicts).
pub fn from_err(err: impl std::error::Error + Send + Sync + 'static) -> Self {
Self(err.into())
}
/// Associated more context to the contained anyhow error
pub fn context<C>(self, context: C) -> Self
where
C: Display + Send + Sync + 'static,
{
let err = self.0;
Self(err.context(context))
}
/// Returns `true` if `E` is contained in our error chain.
pub fn is<E>(&self) -> bool
where
E: Display + Debug + Send + Sync + 'static,
{
self.0.is::<E>()
}
/// Downcast our instance to the given type `E`, or `None` if it's not contained in our context or error chain.
pub fn downcast_ref<E>(&self) -> Option<&E>
where
E: Display + Debug + Send + Sync + 'static,
{
self.0.downcast_ref::<E>()
}
}

1
crates/gitbutler-core/src/fs.rs
View File

@ -104,7 +104,6 @@ fn persist_tempfile(
/// Reads and parses the state file.
///
/// 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> {
let mut file = match File::open(path) {
Ok(f) => f,

8
crates/gitbutler-core/src/git/error.rs
View File

@ -1,7 +1,5 @@
use std::str::Utf8Error;
use crate::error::{Context, ErrorWithContext};
#[derive(Debug, thiserror::Error)]
pub enum Error {
#[error("not found: {0}")]
@ -49,12 +47,6 @@ impl From<git2::Error> for Error {
}
}
impl ErrorWithContext for Error {
fn context(&self) -> Option<Context> {
None
}
}
impl From<super::url::ParseError> for Error {
fn from(err: super::url::ParseError) -> Self {
Error::Url(err)

14
crates/gitbutler-core/src/ops/snapshot.rs
View File

@ -1,4 +1,4 @@
use crate::error::Error;
use anyhow::Result;
use std::vec;
use crate::projects::Project;
@ -15,7 +15,7 @@ impl Project {
pub(crate) fn snapshot_branch_applied(
&self,
snapshot_tree: git::Oid,
result: Result<&String, &Error>,
result: Result<&String, &anyhow::Error>,
) -> anyhow::Result<()> {
let result = result.map(|o| Some(o.clone()));
let details = SnapshotDetails::new(OperationKind::ApplyBranch)
@ -26,7 +26,7 @@ impl Project {
pub(crate) fn snapshot_branch_unapplied(
&self,
snapshot_tree: git::Oid,
result: Result<&Option<Branch>, &Error>,
result: Result<&Option<Branch>, &anyhow::Error>,
) -> anyhow::Result<()> {
let result = result.map(|o| o.clone().map(|b| b.name));
let details = SnapshotDetails::new(OperationKind::UnapplyBranch)
@ -37,7 +37,7 @@ impl Project {
pub(crate) fn snapshot_commit_undo(
&self,
snapshot_tree: git::Oid,
result: Result<&(), &Error>,
result: Result<&(), &anyhow::Error>,
commit_sha: git::Oid,
) -> anyhow::Result<()> {
let result = result.map(|_| Some(commit_sha.to_string()));
@ -49,7 +49,7 @@ impl Project {
pub(crate) fn snapshot_commit_creation(
&self,
snapshot_tree: git::Oid,
error: Option<&Error>,
error: Option<&anyhow::Error>,
commit_message: String,
sha: Option<git::Oid>,
) -> anyhow::Result<()> {
@ -162,7 +162,7 @@ impl Project {
}
}
fn result_trailer(result: Result<Option<String>, &Error>, key: String) -> Vec<Trailer> {
fn result_trailer(result: Result<Option<String>, &anyhow::Error>, key: String) -> Vec<Trailer> {
match result {
Ok(v) => {
if let Some(v) = v {
@ -181,7 +181,7 @@ fn result_trailer(result: Result<Option<String>, &Error>, key: String) -> Vec<Tr
}
}
fn error_trailer(error: Option<&Error>) -> Vec<Trailer> {
fn error_trailer(error: Option<&anyhow::Error>) -> Vec<Trailer> {
error
.map(|e| {
vec![Trailer {

4
crates/gitbutler-core/src/projects/controller.rs
View File

@ -7,8 +7,8 @@ use anyhow::{bail, Context, Result};
use async_trait::async_trait;
use super::{storage, storage::UpdateRequest, Project, ProjectId};
use crate::projects::AuthKey;
use crate::{error, project_repository};
use crate::{error::Error, projects::AuthKey};
#[async_trait]
pub trait Watchers {
@ -181,7 +181,7 @@ impl Controller {
self.projects_storage.list().map_err(Into::into)
}
pub async fn delete(&self, id: ProjectId) -> Result<(), Error> {
pub async fn delete(&self, id: ProjectId) -> Result<()> {
let Some(project) = self.projects_storage.try_get(id)? else {
return Ok(());
};

16
crates/gitbutler-core/src/remotes/controller.rs
View File

@ -1,5 +1,6 @@
use anyhow::Result;
use crate::{
error::Error,
project_repository,
projects::{self, ProjectId},
};
@ -14,22 +15,17 @@ impl Controller {
Self { projects }
}
pub async fn remotes(&self, project_id: ProjectId) -> Result<Vec<String>, Error> {
pub async fn remotes(&self, project_id: ProjectId) -> Result<Vec<String>> {
let project = self.projects.get(project_id)?;
let project_repository = project_repository::Repository::open(&project)?;
Ok(project_repository.remotes()?)
project_repository.remotes()
}
pub async fn add_remote(
&self,
project_id: ProjectId,
name: &str,
url: &str,
) -> Result<(), Error> {
pub async fn add_remote(&self, project_id: ProjectId, name: &str, url: &str) -> Result<()> {
let project = self.projects.get(project_id)?;
let project_repository = project_repository::Repository::open(&project)?;
Ok(project_repository.add_remote(name, url)?)
project_repository.add_remote(name, url)
}
}

190
crates/gitbutler-core/src/virtual_branches/controller.rs
View File

@ -1,7 +1,5 @@
use crate::{
error::Error,
ops::entry::{OperationKind, SnapshotDetails},
};
use crate::ops::entry::{OperationKind, SnapshotDetails};
use anyhow::Result;
use std::{collections::HashMap, path::Path, sync::Arc};
use anyhow::Context;
@ -57,7 +55,7 @@ impl Controller {
message: &str,
ownership: Option<&BranchOwnershipClaims>,
run_hooks: bool,
) -> Result<git::Oid, Error> {
) -> Result<git::Oid> {
self.inner(project_id)
.await
.create_commit(project_id, branch_id, message, ownership, run_hooks)
@ -68,7 +66,7 @@ impl Controller {
&self,
project_id: ProjectId,
branch_name: &git::RemoteRefname,
) -> Result<bool, Error> {
) -> Result<bool> {
self.inner(project_id)
.await
.can_apply_remote_branch(project_id, branch_name)
@ -78,7 +76,7 @@ impl Controller {
&self,
project_id: ProjectId,
branch_id: BranchId,
) -> Result<bool, Error> {
) -> Result<bool> {
self.inner(project_id)
.await
.can_apply_virtual_branch(project_id, branch_id)
@ -87,7 +85,7 @@ impl Controller {
pub async fn list_virtual_branches(
&self,
project_id: ProjectId,
) -> Result<(Vec<super::VirtualBranch>, Vec<git::diff::FileDiff>), Error> {
) -> Result<(Vec<super::VirtualBranch>, Vec<git::diff::FileDiff>)> {
self.inner(project_id)
.await
.list_virtual_branches(project_id)
@ -98,7 +96,7 @@ impl Controller {
&self,
project_id: ProjectId,
create: &super::branch::BranchCreateRequest,
) -> Result<BranchId, Error> {
) -> Result<BranchId> {
self.inner(project_id)
.await
.create_virtual_branch(project_id, create)
@ -109,14 +107,14 @@ impl Controller {
&self,
project_id: ProjectId,
branch: &git::Refname,
) -> Result<BranchId, Error> {
) -> Result<BranchId> {
self.inner(project_id)
.await
.create_virtual_branch_from_branch(project_id, branch)
.await
}
pub async fn get_base_branch_data(&self, project_id: ProjectId) -> Result<BaseBranch, Error> {
pub async fn get_base_branch_data(&self, project_id: ProjectId) -> Result<BaseBranch> {
self.inner(project_id)
.await
.get_base_branch_data(project_id)
@ -126,7 +124,7 @@ impl Controller {
&self,
project_id: ProjectId,
commit_oid: git::Oid,
) -> Result<Vec<RemoteBranchFile>, Error> {
) -> Result<Vec<RemoteBranchFile>> {
self.inner(project_id)
.await
.list_remote_commit_files(project_id, commit_oid)
@ -136,7 +134,7 @@ impl Controller {
&self,
project_id: ProjectId,
target_branch: &git::RemoteRefname,
) -> Result<BaseBranch, Error> {
) -> Result<BaseBranch> {
self.inner(project_id)
.await
.set_base_branch(project_id, target_branch)
@ -146,7 +144,7 @@ impl Controller {
&self,
project_id: ProjectId,
push_remote: &str,
) -> Result<(), Error> {
) -> Result<()> {
self.inner(project_id)
.await
.set_target_push_remote(project_id, push_remote)
@ -156,14 +154,14 @@ impl Controller {
&self,
project_id: ProjectId,
branch_id: BranchId,
) -> Result<(), Error> {
) -> Result<()> {
self.inner(project_id)
.await
.integrate_upstream_commits(project_id, branch_id)
.await
}
pub async fn update_base_branch(&self, project_id: ProjectId) -> Result<Vec<Branch>, Error> {
pub async fn update_base_branch(&self, project_id: ProjectId) -> Result<Vec<Branch>> {
self.inner(project_id)
.await
.update_base_branch(project_id)
@ -174,7 +172,7 @@ impl Controller {
&self,
project_id: ProjectId,
branch_update: super::branch::BranchUpdateRequest,
) -> Result<(), Error> {
) -> Result<()> {
self.inner(project_id)
.await
.update_virtual_branch(project_id, branch_update)
@ -184,7 +182,7 @@ impl Controller {
&self,
project_id: ProjectId,
branch_id: BranchId,
) -> Result<(), Error> {
) -> Result<()> {
self.inner(project_id)
.await
.delete_virtual_branch(project_id, branch_id)
@ -195,7 +193,7 @@ impl Controller {
&self,
project_id: ProjectId,
branch_id: BranchId,
) -> Result<(), Error> {
) -> Result<()> {
self.inner(project_id)
.await
.apply_virtual_branch(project_id, branch_id)
@ -206,18 +204,14 @@ impl Controller {
&self,
project_id: ProjectId,
ownership: &BranchOwnershipClaims,
) -> Result<(), Error> {
) -> Result<()> {
self.inner(project_id)
.await
.unapply_ownership(project_id, ownership)
.await
}
pub async fn reset_files(
&self,
project_id: ProjectId,
files: &Vec<String>,
) -> Result<(), Error> {
pub async fn reset_files(&self, project_id: ProjectId, files: &Vec<String>) -> Result<()> {
self.inner(project_id)
.await
.reset_files(project_id, files)
@ -230,7 +224,7 @@ impl Controller {
branch_id: BranchId,
commit_oid: git::Oid,
ownership: &BranchOwnershipClaims,
) -> Result<git::Oid, Error> {
) -> Result<git::Oid> {
self.inner(project_id)
.await
.amend(project_id, branch_id, commit_oid, ownership)
@ -244,7 +238,7 @@ impl Controller {
from_commit_oid: git::Oid,
to_commit_oid: git::Oid,
ownership: &BranchOwnershipClaims,
) -> Result<git::Oid, Error> {
) -> Result<git::Oid> {
self.inner(project_id)
.await
.move_commit_file(
@ -262,7 +256,7 @@ impl Controller {
project_id: ProjectId,
branch_id: BranchId,
commit_oid: git::Oid,
) -> Result<(), Error> {
) -> Result<()> {
self.inner(project_id)
.await
.undo_commit(project_id, branch_id, commit_oid)
@ -275,7 +269,7 @@ impl Controller {
branch_id: BranchId,
commit_oid: git::Oid,
offset: i32,
) -> Result<(), Error> {
) -> Result<()> {
self.inner(project_id)
.await
.insert_blank_commit(project_id, branch_id, commit_oid, offset)
@ -288,7 +282,7 @@ impl Controller {
branch_id: BranchId,
commit_oid: git::Oid,
offset: i32,
) -> Result<(), Error> {
) -> Result<()> {
self.inner(project_id)
.await
.reorder_commit(project_id, branch_id, commit_oid, offset)
@ -300,7 +294,7 @@ impl Controller {
project_id: ProjectId,
branch_id: BranchId,
target_commit_oid: git::Oid,
) -> Result<(), Error> {
) -> Result<()> {
self.inner(project_id)
.await
.reset_virtual_branch(project_id, branch_id, target_commit_oid)
@ -311,7 +305,7 @@ impl Controller {
&self,
project_id: ProjectId,
branch_id: BranchId,
) -> Result<(), Error> {
) -> Result<()> {
self.inner(project_id)
.await
.unapply_virtual_branch(project_id, branch_id)
@ -324,7 +318,7 @@ impl Controller {
branch_id: BranchId,
with_force: bool,
askpass: Option<Option<BranchId>>,
) -> Result<(), Error> {
) -> Result<()> {
self.inner(project_id)
.await
.push_virtual_branch(project_id, branch_id, with_force, askpass)
@ -336,7 +330,7 @@ impl Controller {
project_id: ProjectId,
branch_id: BranchId,
commit_oid: git::Oid,
) -> Result<Option<git::Oid>, Error> {
) -> Result<Option<git::Oid>> {
self.inner(project_id)
.await
.cherry_pick(project_id, branch_id, commit_oid)
@ -346,7 +340,7 @@ impl Controller {
pub async fn list_remote_branches(
&self,
project_id: ProjectId,
) -> Result<Vec<super::RemoteBranch>, Error> {
) -> Result<Vec<super::RemoteBranch>> {
self.inner(project_id)
.await
.list_remote_branches(project_id)
@ -356,7 +350,7 @@ impl Controller {
&self,
project_id: ProjectId,
refname: &git::Refname,
) -> Result<super::RemoteBranchData, Error> {
) -> Result<super::RemoteBranchData> {
self.inner(project_id)
.await
.get_remote_branch_data(project_id, refname)
@ -367,7 +361,7 @@ impl Controller {
project_id: ProjectId,
branch_id: BranchId,
commit_oid: git::Oid,
) -> Result<(), Error> {
) -> Result<()> {
self.inner(project_id)
.await
.squash(project_id, branch_id, commit_oid)
@ -380,7 +374,7 @@ impl Controller {
branch_id: BranchId,
commit_oid: git::Oid,
message: &str,
) -> Result<(), Error> {
) -> Result<()> {
self.inner(project_id)
.await
.update_commit_message(project_id, branch_id, commit_oid, message)
@ -391,7 +385,7 @@ impl Controller {
&self,
project_id: ProjectId,
askpass: Option<String>,
) -> Result<BaseBranch, Error> {
) -> Result<BaseBranch> {
self.inner(project_id)
.await
.fetch_from_remotes(project_id, askpass)
@ -403,7 +397,7 @@ impl Controller {
project_id: ProjectId,
target_branch_id: BranchId,
commit_oid: git::Oid,
) -> Result<(), Error> {
) -> Result<()> {
self.inner(project_id)
.await
.move_commit(project_id, target_branch_id, commit_oid)
@ -441,7 +435,7 @@ impl ControllerInner {
message: &str,
ownership: Option<&BranchOwnershipClaims>,
run_hooks: bool,
) -> Result<git::Oid, Error> {
) -> Result<git::Oid> {
let _permit = self.semaphore.acquire().await;
self.with_verify_branch(project_id, |project_repository, user| {
@ -471,7 +465,7 @@ impl ControllerInner {
&self,
project_id: ProjectId,
branch_name: &git::RemoteRefname,
) -> Result<bool, Error> {
) -> Result<bool> {
let project = self.projects.get(project_id)?;
let project_repository = project_repository::Repository::open(&project)?;
super::is_remote_branch_mergeable(&project_repository, branch_name).map_err(Into::into)
@ -481,7 +475,7 @@ impl ControllerInner {
&self,
project_id: ProjectId,
branch_id: BranchId,
) -> Result<bool, Error> {
) -> Result<bool> {
let project = self.projects.get(project_id)?;
let project_repository = project_repository::Repository::open(&project)?;
super::is_virtual_branch_mergeable(&project_repository, branch_id).map_err(Into::into)
@ -490,7 +484,7 @@ impl ControllerInner {
pub async fn list_virtual_branches(
&self,
project_id: ProjectId,
) -> Result<(Vec<super::VirtualBranch>, Vec<git::diff::FileDiff>), Error> {
) -> Result<(Vec<super::VirtualBranch>, Vec<git::diff::FileDiff>)> {
let _permit = self.semaphore.acquire().await;
self.with_verify_branch(project_id, |project_repository, _| {
@ -502,7 +496,7 @@ impl ControllerInner {
&self,
project_id: ProjectId,
create: &super::branch::BranchCreateRequest,
) -> Result<BranchId, Error> {
) -> Result<BranchId> {
let _permit = self.semaphore.acquire().await;
self.with_verify_branch(project_id, |project_repository, _| {
@ -515,7 +509,7 @@ impl ControllerInner {
&self,
project_id: ProjectId,
branch: &git::Refname,
) -> Result<BranchId, Error> {
) -> Result<BranchId> {
let _permit = self.semaphore.acquire().await;
self.with_verify_branch(project_id, |project_repository, user| {
@ -524,17 +518,17 @@ impl ControllerInner {
})
}
pub fn get_base_branch_data(&self, project_id: ProjectId) -> Result<BaseBranch, Error> {
pub fn get_base_branch_data(&self, project_id: ProjectId) -> Result<BaseBranch> {
let project = self.projects.get(project_id)?;
let project_repository = project_repository::Repository::open(&project)?;
Ok(super::get_base_branch_data(&project_repository)?)
super::get_base_branch_data(&project_repository)
}
pub fn list_remote_commit_files(
&self,
project_id: ProjectId,
commit_oid: git::Oid,
) -> Result<Vec<RemoteBranchFile>, Error> {
) -> Result<Vec<RemoteBranchFile>> {
let project = self.projects.get(project_id)?;
let project_repository = project_repository::Repository::open(&project)?;
super::list_remote_commit_files(&project_repository.git_repository, commit_oid)
@ -545,33 +539,26 @@ impl ControllerInner {
&self,
project_id: ProjectId,
target_branch: &git::RemoteRefname,
) -> Result<BaseBranch, Error> {
) -> Result<BaseBranch> {
let project = self.projects.get(project_id)?;
let project_repository = project_repository::Repository::open(&project)?;
let _ = project_repository
.project()
.create_snapshot(SnapshotDetails::new(OperationKind::SetBaseBranch));
Ok(super::set_base_branch(&project_repository, target_branch)?)
super::set_base_branch(&project_repository, target_branch)
}
pub fn set_target_push_remote(
&self,
project_id: ProjectId,
push_remote: &str,
) -> Result<(), Error> {
pub fn set_target_push_remote(&self, project_id: ProjectId, push_remote: &str) -> Result<()> {
let project = self.projects.get(project_id)?;
let project_repository = project_repository::Repository::open(&project)?;
Ok(super::set_target_push_remote(
&project_repository,
push_remote,
)?)
super::set_target_push_remote(&project_repository, push_remote)
}
pub async fn integrate_upstream_commits(
&self,
project_id: ProjectId,
branch_id: BranchId,
) -> Result<(), Error> {
) -> Result<()> {
let _permit = self.semaphore.acquire().await;
self.with_verify_branch(project_id, |project_repository, user| {
@ -583,7 +570,7 @@ impl ControllerInner {
})
}
pub async fn update_base_branch(&self, project_id: ProjectId) -> Result<Vec<Branch>, Error> {
pub async fn update_base_branch(&self, project_id: ProjectId) -> Result<Vec<Branch>> {
let _permit = self.semaphore.acquire().await;
self.with_verify_branch(project_id, |project_repository, user| {
@ -598,7 +585,7 @@ impl ControllerInner {
&self,
project_id: ProjectId,
branch_update: super::branch::BranchUpdateRequest,
) -> Result<(), Error> {
) -> Result<()> {
let _permit = self.semaphore.acquire().await;
self.with_verify_branch(project_id, |project_repository, _| {
@ -611,7 +598,7 @@ impl ControllerInner {
&self,
project_id: ProjectId,
branch_id: BranchId,
) -> Result<(), Error> {
) -> Result<()> {
let _permit = self.semaphore.acquire().await;
self.with_verify_branch(project_id, |project_repository, _| {
@ -623,7 +610,7 @@ impl ControllerInner {
&self,
project_id: ProjectId,
branch_id: BranchId,
) -> Result<(), Error> {
) -> Result<()> {
let _permit = self.semaphore.acquire().await;
self.with_verify_branch(project_id, |project_repository, user| {
@ -644,7 +631,7 @@ impl ControllerInner {
&self,
project_id: ProjectId,
ownership: &BranchOwnershipClaims,
) -> Result<(), Error> {
) -> Result<()> {
let _permit = self.semaphore.acquire().await;
self.with_verify_branch(project_id, |project_repository, _| {
@ -655,11 +642,7 @@ impl ControllerInner {
})
}
pub async fn reset_files(
&self,
project_id: ProjectId,
ownership: &Vec<String>,
) -> Result<(), Error> {
pub async fn reset_files(&self, project_id: ProjectId, ownership: &Vec<String>) -> Result<()> {
let _permit = self.semaphore.acquire().await;
self.with_verify_branch(project_id, |project_repository, _| {
@ -676,19 +659,14 @@ impl ControllerInner {
branch_id: BranchId,
commit_oid: git::Oid,
ownership: &BranchOwnershipClaims,
) -> Result<git::Oid, Error> {
) -> Result<git::Oid> {
let _permit = self.semaphore.acquire().await;
self.with_verify_branch(project_id, |project_repository, _| {
let _ = project_repository
.project()
.create_snapshot(SnapshotDetails::new(OperationKind::AmendCommit));
Ok(super::amend(
project_repository,
branch_id,
commit_oid,
ownership,
)?)
super::amend(project_repository, branch_id, commit_oid, ownership)
})
}
@ -699,7 +677,7 @@ impl ControllerInner {
from_commit_oid: git::Oid,
to_commit_oid: git::Oid,
ownership: &BranchOwnershipClaims,
) -> Result<git::Oid, Error> {
) -> Result<git::Oid> {
let _permit = self.semaphore.acquire().await;
self.with_verify_branch(project_id, |project_repository, _| {
@ -722,12 +700,12 @@ impl ControllerInner {
project_id: ProjectId,
branch_id: BranchId,
commit_oid: git::Oid,
) -> Result<(), Error> {
) -> Result<()> {
let _permit = self.semaphore.acquire().await;
self.with_verify_branch(project_id, |project_repository, _| {
let snapshot_tree = project_repository.project().prepare_snapshot();
let result: Result<(), Error> =
let result: Result<()> =
super::undo_commit(project_repository, branch_id, commit_oid).map_err(Into::into);
let _ = snapshot_tree.and_then(|snapshot_tree| {
project_repository.project().snapshot_commit_undo(
@ -746,7 +724,7 @@ impl ControllerInner {
branch_id: BranchId,
commit_oid: git::Oid,
offset: i32,
) -> Result<(), Error> {
) -> Result<()> {
let _permit = self.semaphore.acquire().await;
self.with_verify_branch(project_id, |project_repository, user| {
@ -764,7 +742,7 @@ impl ControllerInner {
branch_id: BranchId,
commit_oid: git::Oid,
offset: i32,
) -> Result<(), Error> {
) -> Result<()> {
let _permit = self.semaphore.acquire().await;
self.with_verify_branch(project_id, |project_repository, _| {
@ -781,7 +759,7 @@ impl ControllerInner {
project_id: ProjectId,
branch_id: BranchId,
target_commit_oid: git::Oid,
) -> Result<(), Error> {
) -> Result<()> {
let _permit = self.semaphore.acquire().await;
self.with_verify_branch(project_id, |project_repository, _| {
@ -797,7 +775,7 @@ impl ControllerInner {
&self,
project_id: ProjectId,
branch_id: BranchId,
) -> Result<(), Error> {
) -> Result<()> {
let _permit = self.semaphore.acquire().await;
self.with_verify_branch(project_id, |project_repository, _| {
@ -818,20 +796,13 @@ impl ControllerInner {
branch_id: BranchId,
with_force: bool,
askpass: Option<Option<BranchId>>,
) -> Result<(), Error> {
) -> Result<()> {
let _permit = self.semaphore.acquire().await;
let helper = self.helper.clone();
self.with_verify_branch_async(project_id, move |project_repository, _| {
Ok(super::push(
project_repository,
branch_id,
with_force,
&helper,
askpass,
)?)
super::push(project_repository, branch_id, with_force, &helper, askpass)
})?
.await
.map_err(Error::from_err)?
.await?
}
pub async fn cherry_pick(
@ -839,7 +810,7 @@ impl ControllerInner {
project_id: ProjectId,
branch_id: BranchId,
commit_oid: git::Oid,
) -> Result<Option<git::Oid>, Error> {
) -> Result<Option<git::Oid>> {
let _permit = self.semaphore.acquire().await;
self.with_verify_branch(project_id, |project_repository, _| {
@ -850,23 +821,20 @@ impl ControllerInner {
})
}
pub fn list_remote_branches(
&self,
project_id: ProjectId,
) -> Result<Vec<super::RemoteBranch>, Error> {
pub fn list_remote_branches(&self, project_id: ProjectId) -> Result<Vec<super::RemoteBranch>> {
let project = self.projects.get(project_id)?;
let project_repository = project_repository::Repository::open(&project)?;
Ok(super::list_remote_branches(&project_repository)?)
super::list_remote_branches(&project_repository)
}
pub fn get_remote_branch_data(
&self,
project_id: ProjectId,
refname: &git::Refname,
) -> Result<super::RemoteBranchData, Error> {
) -> Result<super::RemoteBranchData> {
let project = self.projects.get(project_id)?;
let project_repository = project_repository::Repository::open(&project)?;
Ok(super::get_branch_data(&project_repository, refname)?)
super::get_branch_data(&project_repository, refname)
}
pub async fn squash(
@ -874,7 +842,7 @@ impl ControllerInner {
project_id: ProjectId,
branch_id: BranchId,
commit_oid: git::Oid,
) -> Result<(), Error> {
) -> Result<()> {
let _permit = self.semaphore.acquire().await;
self.with_verify_branch(project_id, |project_repository, _| {
@ -891,7 +859,7 @@ impl ControllerInner {
branch_id: BranchId,
commit_oid: git::Oid,
message: &str,
) -> Result<(), Error> {
) -> Result<()> {
let _permit = self.semaphore.acquire().await;
self.with_verify_branch(project_id, |project_repository, _| {
let _ = project_repository
@ -906,7 +874,7 @@ impl ControllerInner {
&self,
project_id: ProjectId,
askpass: Option<String>,
) -> Result<BaseBranch, Error> {
) -> Result<BaseBranch> {
let project = self.projects.get(project_id)?;
let mut project_repository = project_repository::Repository::open(&project)?;
@ -966,7 +934,7 @@ impl ControllerInner {
project_id: ProjectId,
target_branch_id: BranchId,
commit_oid: git::Oid,
) -> Result<(), Error> {
) -> Result<()> {
let _permit = self.semaphore.acquire().await;
self.with_verify_branch(project_id, |project_repository, user| {
@ -983,8 +951,8 @@ impl ControllerInner {
fn with_verify_branch<T>(
&self,
project_id: ProjectId,
action: impl FnOnce(&project_repository::Repository, Option<&users::User>) -> Result<T, Error>,
) -> Result<T, Error> {
action: impl FnOnce(&project_repository::Repository, Option<&users::User>) -> Result<T>,
) -> Result<T> {
let project = self.projects.get(project_id)?;
let project_repository = project_repository::Repository::open(&project)?;
let user = self.users.get_user()?;
@ -995,10 +963,10 @@ impl ControllerInner {
fn with_verify_branch_async<T: Send + 'static>(
&self,
project_id: ProjectId,
action: impl FnOnce(&project_repository::Repository, Option<&users::User>) -> Result<T, Error>
action: impl FnOnce(&project_repository::Repository, Option<&users::User>) -> Result<T>
+ Send
+ 'static,
) -> Result<JoinHandle<Result<T, Error>>, Error> {
) -> Result<JoinHandle<Result<T>>> {
let project = self.projects.get(project_id)?;
let project_repository = project_repository::Repository::open(&project)?;
let user = self.users.get_user()?;

4
crates/gitbutler-core/src/virtual_branches/virtual.rs
View File

@ -26,6 +26,7 @@ use super::{
branch_to_remote_branch, target, RemoteBranch, VirtualBranchesHandle,
};
use crate::error::Code;
use crate::git::diff::GitHunk;
use crate::git::diff::{diff_files_into_hunks, trees, FileDiff};
use crate::git::{CommitExt, RepositoryExt};
use crate::time::now_since_unix_epoch_ms;
@ -41,7 +42,6 @@ use crate::{
project_repository::{self, conflicts, LogUntil},
users,
};
use crate::{error::Error, git::diff::GitHunk};
type AppliedStatuses = Vec<(branch::Branch, BranchStatus)>;
@ -1403,7 +1403,7 @@ pub fn update_branch(
pub fn delete_branch(
project_repository: &project_repository::Repository,
branch_id: BranchId,
) -> Result<(), Error> {
) -> Result<()> {
let vb_state = project_repository.project().virtual_branches();
let Some(branch) = vb_state.try_branch(branch_id)? else {
return Ok(());

8
crates/gitbutler-core/src/zip/controller.rs
View File

@ -1,4 +1,4 @@
use crate::error::Error;
use anyhow::Result;
use std::path;
use super::Zipper;
@ -28,12 +28,12 @@ impl Controller {
}
}
pub fn archive(&self, project_id: ProjectId) -> Result<path::PathBuf, Error> {
pub fn archive(&self, project_id: ProjectId) -> Result<path::PathBuf> {
let project = self.projects_controller.get(project_id)?;
self.zipper.zip(project.path).map_err(Into::into)
}
pub fn data_archive(&self, project_id: ProjectId) -> Result<path::PathBuf, Error> {
pub fn data_archive(&self, project_id: ProjectId) -> Result<path::PathBuf> {
let project = self.projects_controller.get(project_id)?;
self.zipper
.zip(
@ -44,7 +44,7 @@ impl Controller {
.map_err(Into::into)
}
pub fn logs_archive(&self) -> anyhow::Result<path::PathBuf> {
pub fn logs_archive(&self) -> Result<path::PathBuf> {
self.zipper.zip(&self.logs_dir).map_err(Into::into)
}
}

1
crates/gitbutler-core/tests/core.rs
View File

@ -3,7 +3,6 @@ mod suite {
mod virtual_branches;
}
mod error;
mod git;
mod keys;
mod lock;

49
crates/gitbutler-core/tests/error/mod.rs
View File

@ -1,49 +0,0 @@
mod into_anyhow {
use gitbutler_core::error::{into_anyhow, Code, Context, ErrorWithContext};
#[test]
fn code_as_context() {
#[derive(thiserror::Error, Debug)]
#[error("err")]
struct Error(Code);
impl ErrorWithContext for Error {
fn context(&self) -> Option<Context> {
Context::from(self.0).into()
}
}
let err = into_anyhow(Error(Code::Validation));
let ctx = err.downcast_ref::<Context>().unwrap();
assert_eq!(ctx.code, Code::Validation, "the context is attached");
assert_eq!(
ctx.message, None,
"there is no message when context was created from bare code"
);
}
#[test]
fn nested_code_as_context() {
#[derive(thiserror::Error, Debug)]
#[error("Err")]
struct Inner(Code);
#[derive(thiserror::Error, Debug)]
#[error(transparent)]
struct Outer(#[from] Inner);
impl ErrorWithContext for Outer {
fn context(&self) -> Option<Context> {
Context::from(self.0 .0).into()
}
}
let err = into_anyhow(Outer::from(Inner(Code::Validation)));
let ctx = err.downcast_ref::<Context>().unwrap();
assert_eq!(
ctx.code,
Code::Validation,
"there is no magic here, it's all about manually implementing the nesting :/"
);
}
}

24
crates/gitbutler-tauri/src/app.rs
View File

@ -1,5 +1,4 @@
use anyhow::{Context, Result};
use gitbutler_core::error::Error as CoreError;
use gitbutler_core::{
git,
project_repository::{self, conflicts},
@ -17,7 +16,7 @@ impl App {
Self { projects }
}
pub fn mark_resolved(&self, project_id: ProjectId, path: &str) -> Result<(), CoreError> {
pub fn mark_resolved(&self, project_id: ProjectId, path: &str) -> Result<()> {
let project = self.projects.get(project_id)?;
let project_repository = project_repository::Repository::open(&project)?;
// mark file as resolved
@ -25,13 +24,10 @@ impl App {
Ok(())
}
pub fn git_remote_branches(
&self,
project_id: ProjectId,
) -> Result<Vec<git::RemoteRefname>, CoreError> {
pub fn git_remote_branches(&self, project_id: ProjectId) -> Result<Vec<git::RemoteRefname>> {
let project = self.projects.get(project_id)?;
let project_repository = project_repository::Repository::open(&project)?;
Ok(project_repository.git_remote_branches()?)
project_repository.git_remote_branches()
}
pub fn git_test_push(
@ -41,10 +37,10 @@ impl App {
branch_name: &str,
credentials: &git::credentials::Helper,
askpass: Option<Option<BranchId>>,
) -> Result<(), CoreError> {
) -> Result<()> {
let project = self.projects.get(project_id)?;
let project_repository = project_repository::Repository::open(&project)?;
Ok(project_repository.git_test_push(credentials, remote_name, branch_name, askpass)?)
project_repository.git_test_push(credentials, remote_name, branch_name, askpass)
}
pub fn git_test_fetch(
@ -53,13 +49,13 @@ impl App {
remote_name: &str,
credentials: &git::credentials::Helper,
askpass: Option<String>,
) -> Result<(), CoreError> {
) -> Result<()> {
let project = self.projects.get(project_id)?;
let project_repository = project_repository::Repository::open(&project)?;
Ok(project_repository.fetch(remote_name, credentials, askpass)?)
project_repository.fetch(remote_name, credentials, askpass)
}
pub fn git_index_size(&self, project_id: ProjectId) -> Result<usize, CoreError> {
pub fn git_index_size(&self, project_id: ProjectId) -> Result<usize> {
let project = self.projects.get(project_id)?;
let project_repository = project_repository::Repository::open(&project)?;
let size = project_repository
@ -68,7 +64,7 @@ impl App {
Ok(size)
}
pub fn git_head(&self, project_id: ProjectId) -> Result<String, CoreError> {
pub fn git_head(&self, project_id: ProjectId) -> Result<String> {
let project = self.projects.get(project_id)?;
let project_repository = project_repository::Repository::open(&project)?;
let head = project_repository
@ -98,7 +94,7 @@ impl App {
}
}
pub async fn delete_all_data(&self) -> Result<(), CoreError> {
pub async fn delete_all_data(&self) -> Result<()> {
for project in self.projects.list().context("failed to list projects")? {
self.projects
.delete(project.id)

15
crates/gitbutler-tauri/src/error.rs
View File

@ -20,7 +20,7 @@
pub(crate) use frontend::Error;
mod frontend {
use gitbutler_core::error::{into_anyhow, AnyhowContextExt, ErrorWithContext};
use gitbutler_core::error::AnyhowContextExt;
use serde::{ser::SerializeMap, Serialize};
use std::borrow::Cow;
@ -35,20 +35,7 @@ mod frontend {
}
}
impl From<gitbutler_core::error::Error> for Error {
fn from(value: gitbutler_core::error::Error) -> Self {
Self(value.into())
}
}
impl Error {
/// Convert an error with context to our type.
///
/// Note that this is only needed as trait specialization isn't working well enough yet.
pub fn from_error_with_context(err: impl ErrorWithContext + Send + Sync + 'static) -> Self {
Self(into_anyhow(err))
}
/// Convert an error without context to our type.
///
/// For now, we avoid using a conversion as it would be so general, we'd miss errors with context

2
crates/gitbutler-watcher/src/handler.rs
View File

@ -107,7 +107,7 @@ impl Handler {
{
Ok(())
}
Err(err) => Err(err.context("failed to list virtual branches").into()),
Err(err) => Err(err.context("failed to list virtual branches")),
}
}