fix(remote): improve protocol selction / fallback

abstract the protocols and order to try under different scenarios

add some additional scenarios eg hardcoded http

tweak error reporting

3 files changed, 143 insertions, 91 deletions

diff --git a/src/bin/git_remote_nostr/fetch.rs b/src/bin/git_remote_nostr/fetch.rs
index 6836456..f591fed 100644
--- a/src/bin/git_remote_nostr/fetch.rs
+++ b/src/bin/git_remote_nostr/fetch.rs
@@ -1,6 +1,6 @@
 use std::io::Stdin;
 
-use anyhow::{anyhow, bail, Context, Result};
+use anyhow::{bail, Result};
 use auth_git2::GitAuthenticator;
 use git2::Repository;
 use ngit::{
@@ -16,6 +16,7 @@ use ngit::{
 
 use crate::utils::{
     find_proposal_and_patches_by_branch_name, get_oids_from_fetch_batch, get_open_proposals,
+    get_read_protocols_to_try, join_with_and,
 };
 
 pub async fn run_fetch(
@@ -58,10 +59,10 @@ pub async fn run_fetch(
         && !errors.is_empty()
     {
         bail!(
-            "failed to fetch objects in nostr state event from:\r\n{}",
+            "fetch: failed to fetch objects in nostr state event from:\r\n{}",
             errors
                 .iter()
-                .map(std::string::ToString::to_string)
+                .map(|e| format!(" - {e}"))
                 .collect::<Vec<String>>()
                 .join("\r\n")
         );
@@ -114,97 +115,45 @@ fn fetch_from_git_server(
 ) -> Result<()> {
     let server_url = git_server_url.parse::<CloneUrl>()?;
 
-    // if protocol is local - just try local
-    if server_url.protocol() == ServerProtocol::Local {
-        let formatted_url = server_url.format_as(&ServerProtocol::Local, &None)?;
-        term.write_line(format!("fetching from {formatted_url}...").as_str())?;
-        if let Err(error) = fetch_from_git_server_url(git_repo, oids, &formatted_url) {
-            term.write_line(
-                format!("WARNING: failed to fetch from {formatted_url} error:{error}").as_str(),
-            )?;
-            return Err(error).context(format!("{formatted_url}: failed to fetch"));
-        }
-        return Ok(());
-    }
+    let protocols_to_attempt = get_read_protocols_to_try(&server_url, decoded_nostr_url);
 
-    term.write_line(format!("fetching from {}...", server_url.domain()).as_str())?;
+    let mut failed_protocols = vec![];
+    let mut success = false;
+    for protocol in &protocols_to_attempt {
+        term.write_line(
+            format!("fetching from {} over {protocol}...", server_url.domain(),).as_str(),
+        )?;
 
-    // use overide protocol if specified
-    if let Some(protocol) = &decoded_nostr_url.protocol {
         let formatted_url = server_url.format_as(protocol, &decoded_nostr_url.user)?;
-        let res = fetch_from_git_server_url(git_repo, oids, &formatted_url);
+        let res = if [ServerProtocol::UnauthHttps, ServerProtocol::UnauthHttp].contains(protocol) {
+            fetch_from_git_server_url_unauthenticated(git_repo, oids, &formatted_url)
+        } else {
+            fetch_from_git_server_url(git_repo, oids, &formatted_url)
+        };
         term.clear_last_lines(1)?;
         if let Err(error) = res {
             term.write_line(
-                format!(
-                    "WARNING: {formatted_url} failed to fetch over {protocol}{} as specified in nostr url. error:{error}",
-                    if let Some(user) = &decoded_nostr_url.user {
-                        format!(" with user '{user}'")
-                    } else {
-                        String::new()
-                    }
-                ).as_str(),
+                format!("fetch: {formatted_url} failed over {protocol}: {error}").as_str(),
             )?;
-            return Err(error).context(format!("{formatted_url}: failed to fetch"));
-        }
-        return Ok(());
-    }
-
-    // Try https unauthenticated
-    let formatted_url = server_url.format_as(&ServerProtocol::Https, &None)?;
-    let res = fetch_from_git_server_url_unauthenticated(git_repo, oids, &formatted_url);
-    term.clear_last_lines(1)?;
-    if let Err(unauth_error) = res {
-        term.write_line(
-            format!(
-                "WARNING: {formatted_url} failed to fetch over unauthenticated https. {unauth_error}",
-            ).as_str(),
-        )?;
-        // TODO what about timeout errors?
-        // try over ssh
-        let mut ssh_error = None;
-        if check_ssh_keys() {
-            term.write_line(format!("fetching from {} over ssh...", server_url.domain()).as_str())?;
-            let formatted_url = server_url.format_as(&ServerProtocol::Ssh, &None)?;
-            let res = fetch_from_git_server_url(git_repo, oids, &formatted_url);
-            term.clear_last_lines(1)?;
-            if let Err(error) = res {
-                term.write_line(
-                    format!("WARNING: {formatted_url} failed to fetch over ssh. error:{error}")
-                        .as_str(),
-                )?;
-                term.write_line(
-                    format!("fetching from {} over ssh...", server_url.domain()).as_str(),
-                )?;
-                ssh_error = Some(error);
-            } else {
-                return Ok(());
+            failed_protocols.push(protocol);
+        } else {
+            success = true;
+            if !failed_protocols.is_empty() {
+                term.write_line(format!("fetch: succeeded over {protocol}").as_str())?;
             }
         }
-        // try over https authenticated
-        term.write_line(
-            format!(
-                "fetching from {} over authenticated https...",
-                server_url.domain()
-            )
-            .as_str(),
-        )?;
-        let formatted_url = server_url.format_as(&ServerProtocol::Ssh, &None)?;
-        let res = fetch_from_git_server_url(git_repo, oids, &formatted_url);
-        term.clear_last_lines(1)?;
-        if let Err(auth_https_error) = res {
+    }
+    if !success {
+        if decoded_nostr_url.protocol.is_some() {
             term.write_line(
-                format!("WARNING: {formatted_url} failed to fetch over authenticated https. error:{auth_https_error}",)
-                    .as_str(),
+                "fetch: protocol override in nostr url so not attempting with any other protocols",
             )?;
-            let error_message = format!(
-                "{} failed to fetch over unauthenticated https ({unauth_error}), ssh ({}) and authenticated https ({auth_https_error})",
-                server_url.format_as(&ServerProtocol::Unspecified, &None)?,
-                ssh_error.unwrap_or(anyhow!("no keys found"))
-            );
-
-            bail!(error_message)
         }
+        bail!(
+            "{} failed over {}",
+            server_url.domain(),
+            join_with_and(&failed_protocols)
+        );
     }
     Ok(())
 }
@@ -214,6 +163,9 @@ fn fetch_from_git_server_url(
     oids: &[String],
     git_server_url: &str,
 ) -> Result<()> {
+    if git_server_url.parse::<CloneUrl>()?.protocol() == ServerProtocol::Ssh && !check_ssh_keys() {
+        bail!("no ssh keys found");
+    }
     let git_config = git_repo.config()?;
     let mut git_server_remote = git_repo.remote_anonymous(git_server_url)?;
     let auth = GitAuthenticator::default();
diff --git a/src/bin/git_remote_nostr/utils.rs b/src/bin/git_remote_nostr/utils.rs
index c53c34f..a31dcbf 100644
--- a/src/bin/git_remote_nostr/utils.rs
+++ b/src/bin/git_remote_nostr/utils.rs
@@ -10,7 +10,10 @@ use ngit::{
         get_all_proposal_patch_events_from_cache, get_events_from_cache,
         get_proposals_and_revisions_from_cache,
     },
-    git::{Repo, RepoActions},
+    git::{
+        nostr_url::{CloneUrl, NostrUrlDecoded, ServerProtocol},
+        Repo, RepoActions,
+    },
     git_events::{
         event_is_revision_root, event_to_cover_letter, get_most_recent_patch_with_ancestors,
         status_kinds,
@@ -246,3 +249,91 @@ pub fn find_proposal_and_patches_by_branch_name<'a>(
         }
     })
 }
+
+pub fn join_with_and<T: ToString>(items: &[T]) -> String {
+    match items.len() {
+        0 => String::new(),
+        1 => items[0].to_string(),
+        _ => {
+            let last_item = items.last().unwrap().to_string();
+            let rest = &items[..items.len() - 1];
+            format!(
+                "{} and {}",
+                rest.iter()
+                    .map(std::string::ToString::to_string)
+                    .collect::<Vec<_>>()
+                    .join(", "),
+                last_item
+            )
+        }
+    }
+}
+
+/// get an ordered vector of server protocols to attempt
+pub fn get_read_protocols_to_try(
+    server_url: &CloneUrl,
+    decoded_nostr_url: &NostrUrlDecoded,
+) -> Vec<ServerProtocol> {
+    if server_url.protocol() == ServerProtocol::Filesystem {
+        vec![(ServerProtocol::Filesystem)]
+    } else if let Some(protocol) = &decoded_nostr_url.protocol {
+        vec![protocol.clone()]
+    } else if server_url.protocol() == ServerProtocol::Http {
+        vec![
+            ServerProtocol::UnauthHttp,
+            ServerProtocol::Ssh,
+            ServerProtocol::Http,
+        ]
+    } else if server_url.protocol() == ServerProtocol::Ftp {
+        vec![ServerProtocol::Ftp, ServerProtocol::Ssh]
+    } else {
+        vec![
+            ServerProtocol::UnauthHttps,
+            ServerProtocol::Ssh,
+            ServerProtocol::Https,
+        ]
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    mod join_with_and {
+        use super::*;
+        #[test]
+        fn test_empty() {
+            let items: Vec<&str> = vec![];
+            assert_eq!(join_with_and(&items), "");
+        }
+
+        #[test]
+        fn test_single_item() {
+            let items = vec!["apple"];
+            assert_eq!(join_with_and(&items), "apple");
+        }
+
+        #[test]
+        fn test_two_items() {
+            let items = vec!["apple", "banana"];
+            assert_eq!(join_with_and(&items), "apple and banana");
+        }
+
+        #[test]
+        fn test_three_items() {
+            let items = vec!["apple", "banana", "cherry"];
+            assert_eq!(join_with_and(&items), "apple, banana and cherry");
+        }
+
+        #[test]
+        fn test_four_items() {
+            let items = vec!["apple", "banana", "cherry", "date"];
+            assert_eq!(join_with_and(&items), "apple, banana, cherry and date");
+        }
+
+        #[test]
+        fn test_multiple_items() {
+            let items = vec!["one", "two", "three", "four", "five"];
+            assert_eq!(join_with_and(&items), "one, two, three, four and five");
+        }
+    }
+}
diff --git a/src/lib/git/nostr_url.rs b/src/lib/git/nostr_url.rs
index c7e1bf2..ea8d221 100644
--- a/src/lib/git/nostr_url.rs
+++ b/src/lib/git/nostr_url.rs
@@ -12,9 +12,11 @@ pub enum ServerProtocol {
     Http,
     Git,
     Ftp,
-    Local,
+    Filesystem,
     #[default]
     Unspecified,
+    UnauthHttps, // used for read to enable non-interactive failures over https
+    UnauthHttp,  // used for read to enable non-interactive failures over https
 }
 impl fmt::Display for ServerProtocol {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
@@ -24,8 +26,10 @@ impl fmt::Display for ServerProtocol {
             ServerProtocol::Ftp => write!(f, "ftp"),
             ServerProtocol::Ssh => write!(f, "ssh"),
             ServerProtocol::Git => write!(f, "git"),
-            ServerProtocol::Local => write!(f, "local"),
+            ServerProtocol::Filesystem => write!(f, "filesystem"),
             ServerProtocol::Unspecified => write!(f, "unsepcified"),
+            ServerProtocol::UnauthHttps => write!(f, "unauthenticated https"),
+            ServerProtocol::UnauthHttp => write!(f, "unauthenticated http"),
         }
     }
 }
@@ -168,7 +172,7 @@ impl FromStr for CloneUrl {
         if s.starts_with('/') || s.starts_with("./") || s.starts_with("../") {
             return Ok(Self {
                 original_string: s.to_string(),
-                protocol: ServerProtocol::Local,
+                protocol: ServerProtocol::Filesystem,
                 ..CloneUrl::default()
             });
         }
@@ -248,13 +252,16 @@ fn contains_port(s: &str) -> bool {
 impl CloneUrl {
     pub fn format_as(&self, protocol: &ServerProtocol, user: &Option<String>) -> Result<String> {
         // Check for incompatible protocol conversions
-        if *protocol == ServerProtocol::Local {
-            if self.protocol == ServerProtocol::Local {
-                // If converting from Local to Local, return the original string
+        if *protocol == ServerProtocol::Filesystem {
+            if self.protocol == ServerProtocol::Filesystem {
+                // If converting from Filesystem to Filesystem, return the original string
                 return Ok(self.original_string.clone());
             } else {
-                // If converting to Local from any other protocol, return an error
-                bail!("Cannot convert to Local protocol from {:?}", self.protocol);
+                // If converting to Filesystem from any other protocol, return an error
+                bail!(
+                    "Cannot convert to Filesystem protocol from {:?}",
+                    self.protocol
+                );
             }
         }
 
@@ -262,7 +269,9 @@ impl CloneUrl {
             "{}{}",
             match protocol {
                 ServerProtocol::Https => "https://",
+                ServerProtocol::UnauthHttps => "https://",
                 ServerProtocol::Http => "http://",
+                ServerProtocol::UnauthHttp => "http://",
                 ServerProtocol::Git => "git://",
                 ServerProtocol::Ftp => "ftp://",
                 ServerProtocol::Ssh => "ssh://",