diff --git a/crates/gitbutler-core/src/project_repository/mod.rs b/crates/gitbutler-core/src/project_repository/mod.rs index 1e86f4c40..6b823024f 100644 --- a/crates/gitbutler-core/src/project_repository/mod.rs +++ b/crates/gitbutler-core/src/project_repository/mod.rs @@ -3,6 +3,6 @@ pub mod conflicts; mod repository; pub use config::Config; -pub use repository::{LogUntil, RemoteError, Repository}; +pub use repository::{LogUntil, Repository}; pub mod signatures; diff --git a/crates/gitbutler-core/src/project_repository/repository.rs b/crates/gitbutler-core/src/project_repository/repository.rs index 4a65ab6a5..df0cb1291 100644 --- a/crates/gitbutler-core/src/project_repository/repository.rs +++ b/crates/gitbutler-core/src/project_repository/repository.rs @@ -9,16 +9,13 @@ use anyhow::{anyhow, Context, Result}; use super::conflicts; use crate::virtual_branches::errors::Marker; use crate::{ - askpass, error, - git::{self, credentials::HelpError, Url}, + askpass, + git::{self, Url}, projects::{self, AuthKey}, ssh, users, virtual_branches::{Branch, BranchId}, }; -use crate::{ - error::{AnyhowContextExt, Code, ErrorWithContext}, - git::Oid, -}; +use crate::{error::Code, git::Oid}; pub struct Repository { pub git_repository: git::Repository, @@ -349,18 +346,17 @@ impl Repository { &self, user: Option<&users::User>, ref_specs: &[&str], - ) -> Result { + ) -> Result { let url = self .project .api .as_ref() - .ok_or(RemoteError::Other(anyhow::anyhow!("api not set")))? + .context("api not set")? .code_git_url .as_ref() - .ok_or(RemoteError::Other(anyhow::anyhow!("code_git_url not set")))? + .context("code_git_url not set")? .as_str() - .parse::() - .map_err(|e| RemoteError::Other(e.into()))?; + .parse::()?; tracing::debug!( project_id = %self.project.id, @@ -370,7 +366,8 @@ impl Repository { let access_token = user .map(|user| user.access_token.clone()) - .ok_or(RemoteError::Auth)?; + .context("access token is missing") + .context(Code::ProjectGitAuth)?; let mut callbacks = git2::RemoteCallbacks::new(); if self.project.omit_certificate_check.unwrap_or(false) { @@ -393,23 +390,16 @@ impl Repository { let headers = &[auth_header.as_str()]; push_options.custom_headers(headers); - let mut remote = self - .git_repository - .remote_anonymous(&url) - .map_err(|e| RemoteError::Other(e.into()))?; + let mut remote = self.git_repository.remote_anonymous(&url)?; remote .push(ref_specs, Some(&mut push_options)) - .map_err(|error| match error { - git::Error::Network(error) => { - tracing::warn!(project_id = %self.project.id, ?error, "git push failed",); - RemoteError::Network - } - git::Error::Auth(error) => { - tracing::warn!(project_id = %self.project.id, ?error, "git push failed",); - RemoteError::Auth - } - error => RemoteError::Other(error.into()), + .map_err(|err| match err { + git::Error::Network(err) => anyhow!("network failed").context(err), + git::Error::Auth(err) => anyhow!("authentication failed") + .context(Code::ProjectGitAuth) + .context(err), + err => anyhow::Error::from(err), })?; let bytes_pushed = bytes_pushed.load(std::sync::atomic::Ordering::Relaxed); @@ -434,7 +424,7 @@ impl Repository { credentials: &git::credentials::Helper, refspec: Option, askpass_broker: Option>, - ) -> Result<(), RemoteError> { + ) -> Result<()> { let refspec = refspec.unwrap_or_else(|| { if with_force { format!("+{}:refs/heads/{}", head, branch.branch()) @@ -466,7 +456,7 @@ impl Repository { }) .join() .unwrap() - .map_err(|e| RemoteError::Other(e.into())); + .map_err(Into::into); } let auth_flows = credentials.help(self, branch.remote())?; @@ -505,25 +495,24 @@ impl Repository { ); return Ok(()); } - Err(git::Error::Auth(error) | git::Error::Http(error)) => { - tracing::warn!(project_id = %self.project.id, ?error, "git push failed"); + Err(git::Error::Auth(err) | git::Error::Http(err)) => { + tracing::warn!(project_id = %self.project.id, ?err, "git push failed"); continue; } - Err(git::Error::Network(error)) => { - tracing::warn!(project_id = %self.project.id, ?error, "git push failed"); - return Err(RemoteError::Network); + Err(git::Error::Network(err)) => { + return Err(anyhow!("network failed")).context(err); } Err(err) => { - if let Some(err) = update_refs_error.as_ref() { - return Err(RemoteError::Other(anyhow!(err.to_string()))); + if let Some(update_refs_err) = update_refs_error { + return Err(update_refs_err).context(err); } - return Err(RemoteError::Other(err.into())); + return Err(err.into()); } } } } - Err(RemoteError::Auth) + Err(anyhow!("authentication failed").context(Code::ProjectGitAuth)) } pub fn fetch( @@ -531,7 +520,7 @@ impl Repository { remote_name: &str, credentials: &git::credentials::Helper, askpass: Option, - ) -> Result<(), RemoteError> { + ) -> Result<()> { let refspec = format!("+refs/heads/*:refs/remotes/{}/*", remote_name); // NOTE(qix-): This is a nasty hack, however the codebase isn't structured @@ -556,7 +545,7 @@ impl Repository { }) .join() .unwrap() - .map_err(|e| RemoteError::Other(e.into())); + .map_err(Into::into); } let auth_flows = credentials.help(self, remote_name)?; @@ -580,20 +569,20 @@ impl Repository { tracing::info!(project_id = %self.project.id, %refspec, "git fetched"); return Ok(()); } - Err(git::Error::Auth(error) | git::Error::Http(error)) => { - tracing::warn!(project_id = %self.project.id, ?error, "fetch failed"); + Err(git::Error::Auth(err) | git::Error::Http(err)) => { + tracing::warn!(project_id = %self.project.id, ?err, "fetch failed"); continue; } - Err(git::Error::Network(error)) => { - tracing::warn!(project_id = %self.project.id, ?error, "fetch failed"); - return Err(RemoteError::Network); + Err(git::Error::Network(err)) => { + tracing::warn!(project_id = %self.project.id, ?err, "fetch failed"); + return Err(anyhow!("network failed")).context(err); } - Err(error) => return Err(RemoteError::Other(error.into())), + Err(err) => return Err(err.into()), } } } - Err(RemoteError::Auth) + Err(anyhow!("authentication failed")).context(Code::ProjectGitAuth) } pub fn remotes(&self) -> Result> { @@ -609,37 +598,6 @@ impl Repository { } } -#[derive(Debug, thiserror::Error)] -pub enum RemoteError { - #[error(transparent)] - Help(#[from] HelpError), - #[error("network failed")] - Network, - #[error("authentication failed")] - Auth, - #[error("Git failed")] - Git(#[from] git::Error), - #[error(transparent)] - Other(#[from] anyhow::Error), -} - -impl ErrorWithContext for RemoteError { - fn context(&self) -> Option { - Some(match self { - RemoteError::Help(_) | RemoteError::Network | RemoteError::Git(_) => { - error::Context::default() - } - RemoteError::Auth => error::Context::new_static( - Code::ProjectGitAuth, - "Project remote authentication error", - ), - RemoteError::Other(error) => { - return error.custom_context_or_root_cause().into(); - } - }) - } -} - type OidFilter = dyn Fn(&git2::Commit) -> Result; pub enum LogUntil { diff --git a/crates/gitbutler-core/src/synchronize/mod.rs b/crates/gitbutler-core/src/synchronize/mod.rs index ba79e84ca..03105a2d7 100644 --- a/crates/gitbutler-core/src/synchronize/mod.rs +++ b/crates/gitbutler-core/src/synchronize/mod.rs @@ -61,7 +61,7 @@ async fn push_target( project_id: Id, user: &users::User, batch_size: usize, -) -> Result<(), project_repository::RemoteError> { +) -> Result<()> { let ids = batch_rev_walk( &project_repository.git_repository, batch_size, @@ -147,7 +147,7 @@ fn push_all_refs( project_repository: &project_repository::Repository, user: &users::User, project_id: Id, -) -> Result<(), project_repository::RemoteError> { +) -> Result<()> { let gb_references = collect_refs(project_repository)?; let all_refs: Vec<_> = gb_references .iter() @@ -175,7 +175,7 @@ async fn update_project( projects: &projects::Controller, project_id: Id, id: Oid, -) -> Result<(), project_repository::RemoteError> { +) -> Result<()> { projects .update(&projects::UpdateRequest { id: project_id, diff --git a/crates/gitbutler-core/src/virtual_branches/controller.rs b/crates/gitbutler-core/src/virtual_branches/controller.rs index b63a3fc8d..425303f3d 100644 --- a/crates/gitbutler-core/src/virtual_branches/controller.rs +++ b/crates/gitbutler-core/src/virtual_branches/controller.rs @@ -9,8 +9,7 @@ use tokio::{sync::Semaphore, task::JoinHandle}; use super::{ branch::{BranchId, BranchOwnershipClaims}, - errors, target, target_to_base_branch, BaseBranch, Branch, RemoteBranchFile, - VirtualBranchesHandle, + target, target_to_base_branch, BaseBranch, Branch, RemoteBranchFile, VirtualBranchesHandle, }; use crate::{ git, project_repository, @@ -912,13 +911,9 @@ impl ControllerInner { let mut project_repository = project_repository::Repository::open(&project)?; let remotes = project_repository.remotes()?; - let fetch_results: Vec> = remotes + let fetch_results: Vec> = remotes .iter() - .map(|remote| { - project_repository - .fetch(remote, &self.helper, askpass.clone()) - .map_err(errors::FetchFromTargetError::Remote) - }) + .map(|remote| project_repository.fetch(remote, &self.helper, askpass.clone())) .collect(); let project_data_last_fetched = if fetch_results.iter().any(Result::is_err) { @@ -943,9 +938,9 @@ impl ControllerInner { // if we have a push remote, let's fetch from this too if let Some(push_remote) = &default_target.push_remote_name { - let _ = project_repository - .fetch(push_remote, &self.helper, askpass.clone()) - .map_err(errors::FetchFromTargetError::Remote); + if let Err(err) = project_repository.fetch(push_remote, &self.helper, askpass.clone()) { + tracing::warn!(?err, "fetch from push-remote failed"); + } } let updated_project = self diff --git a/crates/gitbutler-core/src/virtual_branches/errors.rs b/crates/gitbutler-core/src/virtual_branches/errors.rs index 2ce6e3f22..0b467f1f5 100644 --- a/crates/gitbutler-core/src/virtual_branches/errors.rs +++ b/crates/gitbutler-core/src/virtual_branches/errors.rs @@ -1,7 +1,5 @@ -use crate::error::{AnyhowContextExt, Context, ErrorWithContext}; -use crate::{project_repository::RemoteError, projects::ProjectId}; - -/// A way to mark errors using `[anyhow::Context::context]` for later retrieval. +/// A way to mark errors using `[anyhow::Context::context]` for later retrieval, e.g. to know +/// that a certain even happened. /// /// Note that the display implementation is visible to users in logs, so it's a bit 'special' /// to signify its marker status. @@ -23,37 +21,3 @@ impl std::fmt::Display for Marker { } } } - -#[derive(Debug, thiserror::Error)] -pub enum PushError { - #[error(transparent)] - Remote(#[from] RemoteError), - #[error(transparent)] - Other(#[from] anyhow::Error), -} - -impl ErrorWithContext for PushError { - fn context(&self) -> Option { - match self { - PushError::Remote(error) => error.context(), - PushError::Other(error) => error.custom_context_or_root_cause().into(), - } - } -} - -#[derive(Debug, thiserror::Error)] -pub enum FetchFromTargetError { - #[error("failed to fetch")] - Remote(RemoteError), - #[error(transparent)] - Other(#[from] anyhow::Error), -} - -impl ErrorWithContext for FetchFromTargetError { - fn context(&self) -> Option { - match self { - FetchFromTargetError::Remote(error) => error.context(), - FetchFromTargetError::Other(error) => error.custom_context_or_root_cause().into(), - } - } -} diff --git a/crates/gitbutler-core/src/virtual_branches/virtual.rs b/crates/gitbutler-core/src/virtual_branches/virtual.rs index f76001e0c..54894a926 100644 --- a/crates/gitbutler-core/src/virtual_branches/virtual.rs +++ b/crates/gitbutler-core/src/virtual_branches/virtual.rs @@ -23,7 +23,7 @@ use super::{ branch::{ self, Branch, BranchCreateRequest, BranchId, BranchOwnershipClaims, Hunk, OwnershipClaim, }, - branch_to_remote_branch, errors, target, RemoteBranch, VirtualBranchesHandle, + branch_to_remote_branch, target, RemoteBranch, VirtualBranchesHandle, }; use crate::error::Code; use crate::git::diff::{diff_files_into_hunks, trees, FileDiff}; @@ -2343,7 +2343,7 @@ pub fn push( with_force: bool, credentials: &git::credentials::Helper, askpass: Option>, -) -> Result<(), errors::PushError> { +) -> Result<()> { let vb_state = project_repository.project().virtual_branches(); let mut vbranch = vb_state.get_branch(branch_id)?;