From c62adb4531389d2ae20f1ce081f22d3bb19068a9 Mon Sep 17 00:00:00 2001 From: Nikita Galaiko Date: Wed, 22 Nov 2023 15:05:06 +0100 Subject: [PATCH] handle git network errors --- packages/tauri/src/gb_repository/mod.rs | 2 +- .../tauri/src/gb_repository/repository.rs | 48 ++++++++++++------- .../src/project_repository/repository.rs | 36 +++++++++++--- .../watcher/handlers/fetch_gitbutler_data.rs | 22 +++++---- .../watcher/handlers/fetch_project_data.rs | 14 +++--- .../watcher/handlers/push_gitbutler_data.rs | 8 ++-- .../handlers/push_project_to_gitbutler.rs | 22 +++++---- 7 files changed, 101 insertions(+), 51 deletions(-) diff --git a/packages/tauri/src/gb_repository/mod.rs b/packages/tauri/src/gb_repository/mod.rs index f2ff66018..ec18fdd61 100644 --- a/packages/tauri/src/gb_repository/mod.rs +++ b/packages/tauri/src/gb_repository/mod.rs @@ -3,4 +3,4 @@ mod repository; #[cfg(test)] mod repository_tests; -pub use repository::{Error, Repository}; +pub use repository::{Error, RemoteError, Repository}; diff --git a/packages/tauri/src/gb_repository/repository.rs b/packages/tauri/src/gb_repository/repository.rs index 10d0b4dc3..e93d478a6 100644 --- a/packages/tauri/src/gb_repository/repository.rs +++ b/packages/tauri/src/gb_repository/repository.rs @@ -6,7 +6,7 @@ use std::{ path, time, }; -use anyhow::{anyhow, Context, Ok, Result}; +use anyhow::{anyhow, Context, Result}; use filetime::FileTime; use sha2::{Digest, Sha256}; @@ -131,10 +131,10 @@ impl Repository { Ok(Some((remote, access_token))) } - pub fn fetch(&self, user: Option<&users::User>) -> Result { + pub fn fetch(&self, user: Option<&users::User>) -> Result<(), RemoteError> { let (mut remote, access_token) = match self.remote(user)? { Some((remote, access_token)) => (remote, access_token), - None => return Ok(false), + None => return Result::Ok(()), }; let mut callbacks = git2::RemoteCallbacks::new(); @@ -165,21 +165,23 @@ impl Repository { remote .fetch(&["refs/heads/*:refs/remotes/*"], Some(&mut fetch_opts)) - .context(format!( - "failed to pull from remote {}", - remote.url()?.unwrap() - ))?; + .map_err(|error| match error { + git::Error::Network(error) => { + tracing::warn!(project_id = %self.project.id, error = %error, "failed to fetch gb repo"); + RemoteError::Network + } + error => RemoteError::Other(error.into()), + })?; tracing::info!( project_id = %self.project.id, - remote = %remote.url()?.unwrap(), "gb repo fetched", ); - Ok(true) + Result::Ok(()) } - pub fn push(&self, user: Option<&users::User>) -> Result<()> { + pub fn push(&self, user: Option<&users::User>) -> Result<(), RemoteError> { let (mut remote, access_token) = match self.remote(user)? { Some((remote, access_token)) => (remote, access_token), None => return Ok(()), @@ -216,13 +218,15 @@ impl Repository { // Push to the remote remote - .push(&[&remote_refspec], Some(&mut push_options)) - .context(format!( - "failed to push refs/heads/current to {}", - remote.url()?.unwrap() - ))?; + .push(&[&remote_refspec], Some(&mut push_options)).map_err(|error| match error { + git::Error::Network(error) => { + tracing::warn!(project_id = %self.project.id, error = %error, "failed to push gb repo"); + RemoteError::Network + } + error => RemoteError::Other(error.into()), + })?; - tracing::info!(project_id = %self.project.id, remote = %remote.url()?.unwrap(), "gb repository pushed"); + tracing::info!(project_id = %self.project.id, "gb repository pushed"); Ok(()) } @@ -466,14 +470,14 @@ impl Repository { } pub fn get_sessions_iterator(&self) -> Result> { - Ok(sessions::SessionsIterator::new(&self.git_repository)?) + sessions::SessionsIterator::new(&self.git_repository) } pub fn get_current_session(&self) -> Result> { let _lock = self.lock(); let reader = reader::DirReader::open(self.root()); match sessions::Session::try_from(reader) { - Result::Ok(session) => Ok(Some(session)), + Ok(session) => Ok(Some(session)), Err(sessions::SessionError::NoSession) => Ok(None), Err(sessions::SessionError::Err(err)) => Err(err), } @@ -841,6 +845,14 @@ fn write_gb_commit( } } +#[derive(Debug, thiserror::Error)] +pub enum RemoteError { + #[error("network error")] + Network, + #[error(transparent)] + Other(#[from] anyhow::Error), +} + #[cfg(test)] mod test { use anyhow::Result; diff --git a/packages/tauri/src/project_repository/repository.rs b/packages/tauri/src/project_repository/repository.rs index 1dc9f5fd3..83792d649 100644 --- a/packages/tauri/src/project_repository/repository.rs +++ b/packages/tauri/src/project_repository/repository.rs @@ -386,7 +386,7 @@ impl Repository { let access_token = user .map(|user| user.access_token.clone()) - .ok_or(RemoteError::AuthError)?; + .ok_or(RemoteError::Auth)?; let mut callbacks = git2::RemoteCallbacks::new(); let bytes_pushed = Arc::new(AtomicUsize::new(0)); @@ -410,7 +410,17 @@ impl Repository { remote .push(ref_specs, Some(&mut push_options)) - .map_err(|e| RemoteError::Other(e.into()))?; + .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()), + })?; tracing::debug!( project_id = %self.project.id, @@ -459,11 +469,15 @@ impl Repository { tracing::warn!(project_id = %self.project.id, ?error, "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(error) => return Err(RemoteError::Other(error.into())), } } - Err(RemoteError::AuthError) + Err(RemoteError::Auth) } pub fn fetch( @@ -528,16 +542,22 @@ impl Repository { tracing::warn!(project_id = %self.project.id, ?error, "fetch failed"); continue; } + Err(git::Error::Network(error)) => { + tracing::warn!(project_id = %self.project.id, ?error, "fetch failed"); + return Err(RemoteError::Network); + } Err(error) => return Err(RemoteError::Other(error.into())), } } - Err(RemoteError::AuthError) + Err(RemoteError::Auth) } } #[derive(Debug, thiserror::Error)] pub enum RemoteError { + #[error("network failed")] + Network, #[error("git url is empty")] NoUrl, #[error("git url is not ssh: {0}")] @@ -545,7 +565,7 @@ pub enum RemoteError { #[error("git url is not https: {0}")] NonHttpsUrl(String), #[error("authentication failed")] - AuthError, + Auth, #[error(transparent)] Other(#[from] anyhow::Error), } @@ -553,11 +573,15 @@ pub enum RemoteError { impl From for crate::error::Error { fn from(value: RemoteError) -> Self { match value { + RemoteError::Network => crate::error::Error::UserError { + code: crate::error::Code::ProjectGitRemote, + message: "Network erorr occured".to_string(), + }, RemoteError::NonHttpsUrl(url) => crate::error::Error::UserError { code: crate::error::Code::ProjectGitRemote, message: format!("Project has non-https remote url: {}", url), }, - RemoteError::AuthError => crate::error::Error::UserError { + RemoteError::Auth => crate::error::Error::UserError { code: crate::error::Code::ProjectGitAuth, message: "Project remote authentication error".to_string(), }, diff --git a/packages/tauri/src/watcher/handlers/fetch_gitbutler_data.rs b/packages/tauri/src/watcher/handlers/fetch_gitbutler_data.rs index ef42e2de0..b2b75ae31 100644 --- a/packages/tauri/src/watcher/handlers/fetch_gitbutler_data.rs +++ b/packages/tauri/src/watcher/handlers/fetch_gitbutler_data.rs @@ -73,7 +73,6 @@ impl HandlerInner { .context("failed to get project")?; if !project.api.as_ref().map(|api| api.sync).unwrap_or_default() { - //TODO: make the whole handler use a typesafe error anyhow::bail!("sync disabled"); } @@ -95,19 +94,26 @@ impl HandlerInner { .with_max_elapsed_time(Some(time::Duration::from_secs(10 * 60))) .build(); - let fetch_result = if let Err(error) = backoff::retry(policy, || { + let fetch_result = match backoff::retry(policy, || { gb_repo.fetch(user.as_ref()).map_err(|err| { tracing::warn!(%project_id, ?err, will_retry=true, "failed to fetch gitbutler data" ); backoff::Error::transient(err) }) }) { - tracing::error!(%project_id, ?error, will_retry=false, "failed to fetch gitbutler data"); - projects::FetchResult::Error { - timestamp: *now, - error: error.to_string(), + Ok(()) => projects::FetchResult::Fetched { timestamp: *now }, + Err(backoff::Error::Permanent(gb_repository::RemoteError::Network)) => { + projects::FetchResult::Error { + timestamp: *now, + error: "network error".to_string(), + } + } + Err(error) => { + tracing::error!(%project_id, ?error, will_retry=false, "failed to fetch gitbutler data"); + projects::FetchResult::Error { + timestamp: *now, + error: error.to_string(), + } } - } else { - projects::FetchResult::Fetched { timestamp: *now } }; self.projects diff --git a/packages/tauri/src/watcher/handlers/fetch_project_data.rs b/packages/tauri/src/watcher/handlers/fetch_project_data.rs index fe69c6603..94b6cc6d0 100644 --- a/packages/tauri/src/watcher/handlers/fetch_project_data.rs +++ b/packages/tauri/src/watcher/handlers/fetch_project_data.rs @@ -110,12 +110,14 @@ impl HandlerInner { }) }) { Ok(()) => projects::FetchResult::Fetched { timestamp: *now }, - Err(backoff::Error::Permanent(RemoteError::AuthError)) => { - projects::FetchResult::Error { - timestamp: *now, - error: RemoteError::AuthError.to_string(), - } - } + Err(backoff::Error::Permanent(RemoteError::Network)) => projects::FetchResult::Error { + timestamp: *now, + error: RemoteError::Network.to_string(), + }, + Err(backoff::Error::Permanent(RemoteError::Auth)) => projects::FetchResult::Error { + timestamp: *now, + error: RemoteError::Auth.to_string(), + }, Err(error) => { tracing::error!(%project_id, ?error, will_retry = false, "failed to fetch project data"); projects::FetchResult::Error { diff --git a/packages/tauri/src/watcher/handlers/push_gitbutler_data.rs b/packages/tauri/src/watcher/handlers/push_gitbutler_data.rs index 6fb7c4d53..05d78c89f 100644 --- a/packages/tauri/src/watcher/handlers/push_gitbutler_data.rs +++ b/packages/tauri/src/watcher/handlers/push_gitbutler_data.rs @@ -3,6 +3,7 @@ use std::sync::{Arc, Mutex, TryLockError}; use anyhow::{Context, Result}; use tauri::AppHandle; +use crate::gb_repository::RemoteError; use crate::paths::DataDir; use crate::projects::ProjectId; use crate::{gb_repository, project_repository, projects, users}; @@ -77,8 +78,9 @@ impl HandlerInner { ) .context("failed to open repository")?; - gb_repo.push(user.as_ref()).context("failed to push")?; - - Ok(vec![]) + match gb_repo.push(user.as_ref()) { + Ok(()) | Err(RemoteError::Network) => Ok(vec![]), + Err(err) => Err(err).context("failed to push"), + } } } diff --git a/packages/tauri/src/watcher/handlers/push_project_to_gitbutler.rs b/packages/tauri/src/watcher/handlers/push_project_to_gitbutler.rs index 5c1163453..b65fdfb06 100644 --- a/packages/tauri/src/watcher/handlers/push_project_to_gitbutler.rs +++ b/packages/tauri/src/watcher/handlers/push_project_to_gitbutler.rs @@ -108,9 +108,11 @@ impl HandlerInner { for (idx, id) in ids.iter().enumerate().rev() { let refspec = format!("+{}:refs/push-tmp/{}", id, project_id); - project_repository - .push_to_gitbutler_server(user.as_ref(), &[&refspec]) - .context("failed to push project to gitbutler")?; + match project_repository.push_to_gitbutler_server(user.as_ref(), &[&refspec]) { + Ok(()) => {} + Err(project_repository::RemoteError::Network) => return Ok(vec![]), + Err(err) => return Err(err).context("failed to push"), + }; self.project_store .update(&projects::UpdateRequest { @@ -131,12 +133,14 @@ impl HandlerInner { } // push refs/{project_id} - project_repository - .push_to_gitbutler_server( - user.as_ref(), - &[&format!("+{}:refs/{}", default_target.sha, project_id)], - ) - .context("failed to push project (head) to gitbutler")?; + match project_repository.push_to_gitbutler_server( + user.as_ref(), + &[&format!("+{}:refs/{}", default_target.sha, project_id)], + ) { + Ok(()) => {} + Err(project_repository::RemoteError::Network) => return Ok(vec![]), + Err(err) => return Err(err).context("failed to push"), + }; let refs = gb_refs(&project_repository)?;