feat: retry failed targets and classify transient failures as user errors

docs/.vuepress/config.ts

@@ -56,7 +56,8 @@ export default defineUserConfig({
           '/advanced/filters.md',
           '/advanced/query-params.md',
           '/advanced/refspecs.md',
-          '/advanced/recovery.md'
+          '/advanced/recovery.md',
+          '/advanced/retries.md'
         ]
       },
       {
@@ -99,7 +100,8 @@ export default defineUserConfig({
             '/advanced/filters.md',
             '/advanced/query-params.md',
             '/advanced/refspecs.md',
-            '/advanced/recovery.md'
+            '/advanced/recovery.md',
+            '/advanced/retries.md'
           ]
         }
       ],

docs/advanced/retries.md

@@ -0,0 +1,44 @@
+# Retries
+When backing up large numbers of repositories, releases, and gists, it is normal to
+occasionally hit a transient failure: a network connection is dropped or reset, a remote
+server is momentarily overloaded, or a request times out. These failures usually clear up
+on their own, so `github-backup` will automatically retry an individual target which fails
+before reporting the error.
+
+How many times a failed target is retried is controlled by the `retries` property on your
+backup policy. By default each target is retried **once** (for a total of two attempts)
+before the failure is reported.
+
+```yaml{7-8} title="config.yaml"
+schedule: "0 * * * *"
+
+backups:
+  - kind: github/repo
+    from: "repos/my-org/repo"
+    to: /backups/work
+    properties:
+      retries: "3"
+```
+
+| Value       | Behaviour                                                                                     |
+|-------------|-----------------------------------------------------------------------------------------------|
+| `0`         | Disable retries; the first failure of a target is reported immediately.                        |
+| `1`         | Retry a failed target once before reporting the error. This is the **default**.                |
+| `n`         | Retry a failed target up to `n` times (for a total of `n + 1` attempts) before giving up.      |
+
+## How Retries Work
+Retries apply to each individual target independently. When a source entity (such as a
+repository) is mirrored to several targets, each target is retried on its own, so a
+transient failure writing to one destination will not affect the others.
+
+Retries wrap the entire backup of a target, which means they compose with the git
+[automatic recovery](./recovery.md) behaviour: each attempt performs its own recovery
+steps, and only once every attempt has been exhausted is the failure reported.
+
+::: tip
+Retries smooth over transient problems, but they will not paper over a persistent
+misconfiguration such as an invalid access token or an unreachable remote — those failures
+will simply be retried the configured number of times and then reported as usual. If you
+are seeing repeated failures, check the reported error rather than increasing the retry
+count.
+:::

src/config.rs

@@ -21,7 +21,7 @@ impl TryFrom<&Args> for Config {
 
     fn try_from(value: &Args) -> Result<Self, Self::Error> {
         let content = std::fs::read_to_string(&value.config).wrap_user_err(
-            format!("Failed to read the config file {}.", &value.config),
+            format!("Failed to read the config file {}.", value.config),
             &["Make sure that the configuration file exists and can be ready by the process."],
         )?;
         let config: Config = serde_yaml::from_str(&content).wrap_user_err(

src/engines/git.rs

@@ -84,8 +84,8 @@ impl GitEngine {
             repo.clone_url,
             target.display()
         );
-        let mut fetch = gix::prepare_clone(repo.clone_url.as_str(), target).wrap_system_err(
-            format!("Failed to clone the repository {}.", &repo.clone_url),
+        let mut fetch = gix::prepare_clone(repo.clone_url.as_str(), target).wrap_user_err(
+            format!("Failed to clone the repository {}.", repo.clone_url),
             &["Please make sure that the target directory is writable and that the repository is accessible."],
         )?;
 
@@ -101,7 +101,7 @@ impl GitEngine {
         }
 
         trace!("Running clone in bare mode (not checking out files)");
-        let (repository, _outcome) = fetch.fetch_only(Discard, cancel).wrap_system_err(
+        let (repository, _outcome) = fetch.fetch_only(Discard, cancel).wrap_user_err(
             format!("Unable to clone remote repository '{}'", repo.clone_url),
             &["Make sure that your internet connectivity is working correctly, and that your local git configuration is able to clone this repo."],
         )?;
@@ -120,7 +120,7 @@ impl GitEngine {
         })?;
 
         let head_id = repository.head_id().wrap_user_err(
-            format!("The repository '{}' did not have a valid HEAD, which may indicate that there is something wrong with the source repository.", &repo.clone_url),
+            format!("The repository '{}' did not have a valid HEAD, which may indicate that there is something wrong with the source repository.", repo.clone_url),
             &["Make sure that the remote repository is valid."],
         )?;
 
@@ -139,8 +139,8 @@ impl GitEngine {
             let repository = gix::open(target).wrap_user_err(
                 format!(
                     "Failed to open the repository '{}' at '{}'",
-                    &repo.clone_url,
-                    &target.display()
+                    repo.clone_url,
+                    target.display()
                 ),
                 &["Make sure that the target directory is a valid git repository."],
             )?;
@@ -154,8 +154,8 @@ impl GitEngine {
         let repository = gix::open(target).wrap_user_err(
             format!(
                 "Failed to open the repository '{}' at '{}'",
-                &repo.clone_url,
-                &target.display()
+                repo.clone_url,
+                target.display()
             ),
             &["Make sure that the target directory is a valid git repository."],
         )?;
@@ -172,7 +172,7 @@ impl GitEngine {
             format!(
                 "Failed to find the remote '{}' in the repository '{}'",
                 repo.clone_url,
-                &target.display()
+                target.display()
                 ),
             &["Make sure that the repository is correctly configured and that the remote exists."],
         )?
@@ -186,8 +186,8 @@ impl GitEngine {
             .wrap_user_err(
                 format!(
                     "Failed to configure the remote '{}' in the repository '{}' to fetch all branches.",
-                    &repo.clone_url,
-                    &target.display()
+                    repo.clone_url,
+                    target.display()
                     ),
                     &["Make sure that the repository is correctly configured and that the remote exists."],
                 )?;
@@ -198,7 +198,7 @@ impl GitEngine {
             .wrap_user_err(
                 format!(
                     "Unable to establish connection to remote git repository '{}'",
-                    &repo.clone_url
+                    repo.clone_url
                 ),
                 &["Make sure that the repository is available and correctly configured."],
             )?;
@@ -214,7 +214,7 @@ impl GitEngine {
             .wrap_user_err(
                 format!(
                     "Unable to prepare fetch from remote git repository '{}'",
-                    &repo.clone_url
+                    repo.clone_url
                 ),
                 &["Make sure that the repository is available and correctly configured."],
             )?
@@ -223,13 +223,13 @@ impl GitEngine {
             .wrap_user_err(
                 format!(
                     "Unable to fetch from remote git repository '{}'",
-                    &repo.clone_url
+                    repo.clone_url
                 ),
                 &["Make sure that the repository is available and correctly configured."],
             )?;
 
         let head_id = repository.head_id().wrap_user_err(
-            format!("The repository '{}' did not have a valid HEAD, which may indicate that there is something wrong with the source repository.", &repo.clone_url),
+            format!("The repository '{}' did not have a valid HEAD, which may indicate that there is something wrong with the source repository.", repo.clone_url),
             &["Make sure that the remote repository is valid."],
         )?;
 

src/errors.rs

@@ -14,12 +14,12 @@ impl HumanizableError for reqwest::Error {
                     "Make sure that your internet connection is working correctly and the service is not blocked by your firewall.",
                 ],
             )
-        } else if self.is_decode() {
-            human_errors::wrap_system(
+        } else if self.is_timeout() {
+            human_errors::wrap_user(
                 self,
-                "We could not decode the response from the remote server.",
+                "We timed out making a web request.",
                 &[
-                    "This is likely due to a problem with the remote server, please try again later and report the problem to us on GitHub if the issue persists.",
+                    "This is usually caused by a slow or unreliable network connection, or a remote server which is temporarily overloaded. Please try again later.",
                 ],
             )
         } else if self.is_redirect() {
@@ -30,12 +30,20 @@ impl HumanizableError for reqwest::Error {
                     "This is likely due to a problem with the remote server, please try again later and report the problem to us on GitHub if the issue persists.",
                 ],
             )
-        } else if self.is_timeout() {
+        } else if self.is_request() {
+            human_errors::wrap_user(
+                self,
+                "We could not complete a web request to the remote server.",
+                &[
+                    "This is usually caused by a transient network problem, such as a dropped or reset connection. Please make sure your internet connection is working correctly and try again later.",
+                ],
+            )
+        } else if self.is_decode() {
             human_errors::wrap_system(
                 self,
-                "We timed out making a web request.",
+                "We could not decode the response from the remote server.",
                 &[
-                    "This is likely due to a problem with the remote server or your internet connection, please try again later and report the problem to us on GitHub if the issue persists.",
+                    "This is likely due to a problem with the remote server, please try again later and report the problem to us on GitHub if the issue persists.",
                 ],
             )
         } else {
@@ -153,3 +161,57 @@ impl std::fmt::Display for ResponseError {
 }
 
 impl std::error::Error for ResponseError {}
+
+#[cfg(test)]
+mod tests {
+    use super::HumanizableError;
+
+    #[tokio::test]
+    async fn connect_failure_is_user_error() {
+        // Port 1 on localhost is (essentially) never listening, so the
+        // connection attempt fails before a request can even be sent. This
+        // stands in for the transient connectivity failures we now classify as
+        // user errors so that they are retried rather than reported as bugs.
+        let err = reqwest::Client::new()
+            .get("http://127.0.0.1:1/")
+            .send()
+            .await
+            .expect_err("the connection to a closed port should fail");
+
+        let human = err.to_human_error();
+        assert!(
+            human.is(human_errors::Kind::User),
+            "connectivity failures should be reported as user errors, got: {human}"
+        );
+    }
+
+    #[tokio::test]
+    async fn timeout_is_user_error() {
+        let server = wiremock::MockServer::start().await;
+        wiremock::Mock::given(wiremock::matchers::method("GET"))
+            .respond_with(
+                wiremock::ResponseTemplate::new(200).set_delay(std::time::Duration::from_secs(30)),
+            )
+            .mount(&server)
+            .await;
+
+        let client = reqwest::Client::builder()
+            .timeout(std::time::Duration::from_millis(50))
+            .build()
+            .expect("a reqwest client");
+
+        let err = client
+            .get(server.uri())
+            .send()
+            .await
+            .expect_err("the request should time out");
+
+        assert!(err.is_timeout(), "expected a timeout error, got: {err}");
+
+        let human = err.to_human_error();
+        assert!(
+            human.is(human_errors::Kind::User),
+            "timeouts should be reported as user errors, got: {human}"
+        );
+    }
+}

src/helpers/github/client.rs

@@ -83,7 +83,7 @@ impl GitHubClient {
                 Err(err) => {
                   Err(human_errors::wrap_system(
                     err,
-                    format!("Unable to parse GitHub response into the expected structure when requesting '{}'.", &url),
+                    format!("Unable to parse GitHub response into the expected structure when requesting '{}'.", url),
                     &["Please report this issue to us on GitHub."],
                 ))?;
                 }