Remove all custom errors from virtual branches module.

In order to do that, errors from other modules have to go as well.
This commit is contained in:
Sebastian Thiel 2024-05-31 16:11:37 +02:00
parent f82d25741e
commit 939624f725
No known key found for this signature in database
GPG Key ID: 9CB5EE7895E8268B
6 changed files with 49 additions and 132 deletions

View File

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

View File

@ -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<bool, RemoteError> {
) -> Result<bool> {
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::<Url>()
.map_err(|e| RemoteError::Other(e.into()))?;
.parse::<Url>()?;
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<String>,
askpass_broker: Option<Option<BranchId>>,
) -> 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<String>,
) -> 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<Vec<String>> {
@ -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<error::Context> {
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<bool>;
pub enum LogUntil {

View File

@ -61,7 +61,7 @@ async fn push_target(
project_id: Id<projects::Project>,
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<projects::Project>,
) -> 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<projects::Project>,
id: Oid,
) -> Result<(), project_repository::RemoteError> {
) -> Result<()> {
projects
.update(&projects::UpdateRequest {
id: project_id,

View File

@ -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<Result<(), errors::FetchFromTargetError>> = remotes
let fetch_results: Vec<Result<(), _>> = 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

View File

@ -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<Context> {
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<Context> {
match self {
FetchFromTargetError::Remote(error) => error.context(),
FetchFromTargetError::Other(error) => error.custom_context_or_root_cause().into(),
}
}
}

View File

@ -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<Option<BranchId>>,
) -> Result<(), errors::PushError> {
) -> Result<()> {
let vb_state = project_repository.project().virtual_branches();
let mut vbranch = vb_state.get_branch(branch_id)?;