handle git network errors

This commit is contained in:
Nikita Galaiko 2023-11-22 15:05:06 +01:00 committed by GitButler
parent 47ce0c49e8
commit c62adb4531
7 changed files with 101 additions and 51 deletions

View File

@ -3,4 +3,4 @@ mod repository;
#[cfg(test)]
mod repository_tests;
pub use repository::{Error, Repository};
pub use repository::{Error, RemoteError, Repository};

View File

@ -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<bool> {
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<sessions::SessionsIterator<'_>> {
Ok(sessions::SessionsIterator::new(&self.git_repository)?)
sessions::SessionsIterator::new(&self.git_repository)
}
pub fn get_current_session(&self) -> Result<Option<sessions::Session>> {
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;

View File

@ -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<RemoteError> 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(),
},

View File

@ -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

View File

@ -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 {

View File

@ -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"),
}
}
}

View File

@ -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)?;