From e412168be3b005d53b20bc228e46cea53b317fd2 Mon Sep 17 00:00:00 2001
From: Fintan Halpenny <fintan.halpenny@gmail.com>
Date: Thu, 7 Nov 2024 14:26:43 +0000
Subject: [PATCH] node: do not fail on `set_head`

The failure to set a HEAD ref, due to quourum, during a fetch should not fail
the fetch entirely.

Instead, log the success and failure, or return an error in other cases.

In fact, any failure to set the head could have resulted in notifications not
being sent, nor caching to occur.
---
 radicle-cli/examples/git/git-push-converge.md | 12 ++++++------
 radicle-node/src/tests/e2e.rs                 |  7 +++++--
 radicle-node/src/worker/fetch.rs              | 16 ++++++++++++++--
 3 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/radicle-cli/examples/git/git-push-converge.md b/radicle-cli/examples/git/git-push-converge.md
index 3ae8468f..171baa1f 100644
--- a/radicle-cli/examples/git/git-push-converge.md
+++ b/radicle-cli/examples/git/git-push-converge.md
@@ -59,13 +59,13 @@ found` error is showing up:
 $ rad remote add did:key:z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk --name bob
 ✓ Follow policy updated for z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk (bob)
 ✓ Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from z6Mkux1…nVhib7Z..
-✗ Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from z6Mkt67…v4N1tRk.. error: no quorum was found
+✓ Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from z6Mkt67…v4N1tRk..
 ✓ Remote bob added
 ✓ Remote-tracking branch bob/master created for z6Mkt67…v4N1tRk
 $ rad remote add did:key:z6Mkux1aUQD2voWWukVb5nNUR7thrHveQG4pDQua8nVhib7Z --name eve
 ✓ Follow policy updated for z6Mkux1aUQD2voWWukVb5nNUR7thrHveQG4pDQua8nVhib7Z (eve)
-✗ Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from z6Mkux1…nVhib7Z.. error: no quorum was found
-✗ Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from z6Mkt67…v4N1tRk.. error: no quorum was found
+✓ Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from z6Mkux1…nVhib7Z..
+✓ Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from z6Mkt67…v4N1tRk..
 ✓ Remote eve added
 ✓ Remote-tracking branch eve/master created for z6Mkux1…nVhib7Z
 ```
@@ -91,7 +91,7 @@ commit:
 $ git push rad -f
 warn: no quorum was found for `refs/heads/master`
 warn: it is recommended to find a commit to agree upon
-✓ Synced with 1 node(s)
+✓ Synced with 2 node(s)
 To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi
    d09e634..0f9bd80  master -> master
 ```
@@ -99,8 +99,8 @@ To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkE
 ``` ~bob
 $ rad remote add did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi --name alice
 ✓ Follow policy updated for z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi (alice)
-✗ Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from z6Mkux1…nVhib7Z.. error: no quorum was found
-✗ Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from z6MknSL…StBU8Vi.. error: no quorum was found
+✓ Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from z6Mkux1…nVhib7Z..
+✓ Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from z6MknSL…StBU8Vi..
 ✓ Remote alice added
 ✓ Remote-tracking branch alice/master created for z6MknSL…StBU8Vi
 $ git reset --hard alice/master
diff --git a/radicle-node/src/tests/e2e.rs b/radicle-node/src/tests/e2e.rs
index 557a0939..8337b53a 100644
--- a/radicle-node/src/tests/e2e.rs
+++ b/radicle-node/src/tests/e2e.rs
@@ -1202,11 +1202,14 @@ fn missing_default_branch() {
         repo.sign_refs(&alice.signer).unwrap();
     }
 
-    // Then fetching from her will fail
+    // Fetching from her will still succeed.
     assert_matches!(
         bob.handle.fetch(rid, alice.id, DEFAULT_TIMEOUT).unwrap(),
-        FetchResult::Failed { .. }
+        FetchResult::Success { .. }
     );
+    let repo = bob.storage.repository(rid).unwrap();
+    // The canonical head cannot be computed, though.
+    assert!(repo.canonical_head().is_err());
 }
 
 #[test]
diff --git a/radicle-node/src/worker/fetch.rs b/radicle-node/src/worker/fetch.rs
index c71695a0..c180efed 100644
--- a/radicle-node/src/worker/fetch.rs
+++ b/radicle-node/src/worker/fetch.rs
@@ -7,11 +7,13 @@ use localtime::LocalTime;
 
 use radicle::cob::TypedId;
 use radicle::crypto::PublicKey;
+use radicle::git::canonical::QuorumError;
 use radicle::identity::DocAt;
 use radicle::prelude::RepoId;
 use radicle::storage::refs::RefsAt;
 use radicle::storage::{
-    ReadRepository, ReadStorage as _, RefUpdate, RemoteRepository, WriteRepository as _,
+    ReadRepository, ReadStorage as _, RefUpdate, RemoteRepository, RepositoryError,
+    WriteRepository as _,
 };
 use radicle::{cob, git, node, Storage};
 use radicle_fetch::{Allowed, BlockList, FetchLimit};
@@ -135,7 +137,17 @@ impl Handle {
                 // points to a repository that is temporary and gets moved by [`mv`].
                 let repo = storage.repository(rid)?;
                 repo.set_identity_head()?;
-                repo.set_head()?;
+                match repo.set_head() {
+                    Ok(head) => {
+                        if head.is_updated() {
+                            log::trace!(target: "worker", "Set HEAD to {}", head.new);
+                        }
+                    }
+                    Err(RepositoryError::Quorum(QuorumError::NoQuorum)) => {
+                        log::warn!(target: "worker", "Fetch could not set HEAD: no quorum found")
+                    }
+                    Err(e) => return Err(e.into()),
+                }
 
                 // Notifications are only posted for pulls, not clones.
                 if let Some(mut store) = notifs {