Radish alpha
h
rad:z3gqcJUoA1n9HaHKufZs5FCSGazv5
Radicle Heartwood Protocol & Stack
Radicle
Git
fetch: collect update errors
Draft fintohaps opened 2 months ago

A single error while updating a reference can mean that every other update will fail.

Instead of shortcutting, collect any errors that can occur, and log them as warnings.

4 files changed +42 -24 993428df fd449233
modified crates/radicle-fetch/src/git/refs/update.rs
@@ -24,6 +24,8 @@ use radicle::prelude::PublicKey;

pub use radicle::storage::RefUpdate;

+
use crate::git::repository::error;
+

/// The set of applied changes from a reference store update.
#[derive(Debug, Default)]
pub struct Applied<'a> {
@@ -33,6 +35,8 @@ pub struct Applied<'a> {
    pub rejected: Vec<Update<'a>>,
    /// Set of successfully updated references.
    pub updated: Vec<RefUpdate>,
+
    /// Collected errors while attempting to update references.
+
    pub errors: Vec<error::Update>,
}

impl Applied<'_> {
modified crates/radicle-fetch/src/git/repository.rs
@@ -117,29 +117,31 @@ where
    }
}

-
pub fn update<'a, I>(repo: &Repository, updates: I) -> Result<Applied<'a>, error::Update>
+
pub fn update<'a, I>(repo: &Repository, updates: I) -> Applied<'a>
where
    I: IntoIterator<Item = Update<'a>>,
{
-
    let mut applied = Applied::default();
-
    for up in updates.into_iter() {
-
        match up {
-
            Update::Direct {
-
                name,
-
                target,
-
                no_ff,
-
            } => match direct(repo, name, target, no_ff)? {
-
                Updated::Rejected(r) => applied.rejected.push(r),
-
                Updated::Accepted(u) => applied.updated.push(u),
-
            },
-
            Update::Prune { name, prev } => match prune(repo, name, prev)? {
-
                Updated::Rejected(r) => applied.rejected.push(r),
-
                Updated::Accepted(u) => applied.updated.push(u),
-
            },
-
        }
-
    }
-

-
    Ok(applied)
+
    updates
+
        .into_iter()
+
        .fold(Applied::default(), |mut applied, up| {
+
            match up {
+
                Update::Direct {
+
                    name,
+
                    target,
+
                    no_ff,
+
                } => match direct(repo, name, target, no_ff) {
+
                    Ok(Updated::Rejected(r)) => applied.rejected.push(r),
+
                    Ok(Updated::Accepted(u)) => applied.updated.push(u),
+
                    Err(err) => applied.errors.push(err),
+
                },
+
                Update::Prune { name, prev } => match prune(repo, name, prev) {
+
                    Ok(Updated::Rejected(r)) => applied.rejected.push(r),
+
                    Ok(Updated::Accepted(u)) => applied.updated.push(u),
+
                    Err(err) => applied.errors.push(err),
+
                },
+
            };
+
            applied
+
        })
}

fn direct<'a>(
modified crates/radicle-fetch/src/state.rs
@@ -579,7 +579,7 @@ impl FetchState {
                    .clone()
                    .into_values()
                    .flat_map(|ups| ups.into_iter()),
-
            )?;
+
            );
            log::debug!("Applied updates ({}ms)", start.elapsed().as_millis());
            Ok(FetchResult::Success {
                applied,
modified crates/radicle-node/src/worker/fetch.rs
@@ -1,11 +1,12 @@
+
use std::collections::BTreeSet;
+
use std::error::Error as _;
+
use std::str::FromStr;
+

use radicle::identity::doc::CanonicalRefsError;
use radicle::identity::CanonicalRefs;
use radicle::storage::git::TempRepository;
pub(crate) use radicle_protocol::worker::fetch::error;

-
use std::collections::BTreeSet;
-
use std::str::FromStr;
-

use localtime::LocalTime;

use radicle::cob::TypedId;
@@ -122,6 +123,17 @@ impl Handle {
                    log::debug!(target: "worker", "Validation error: {warn}");
                }

+
                for e in &applied.errors {
+
                    match e.source() {
+
                        None => {
+
                            log::warn!(target: "worker", "Failed to update reference in {rid}: {e}")
+
                        }
+
                        Some(source) => {
+
                            log::warn!(target: "worker", "Failed to update reference in {rid}: {e}, due to {source}")
+
                        }
+
                    }
+
                }
+

                // N.b. We do not go through handle for this since the cloning handle
                // points to a repository that is temporary and gets moved by [`mv`].
                let repo = storage.repository(rid)?;