From b6a120fcbfab89cb0ae41212a83f780cfe13095e Mon Sep 17 00:00:00 2001 From: charles-gauthereau Date: Thu, 27 Aug 2026 19:01:10 +0200 Subject: [PATCH] feat: retry the restore backup download Extracts the download body to download_once and makes download_backup a retry wrapper around it. This path had no retry at all before, so a single dropped connection failed the whole restore job. Retrying is safe because File::create truncates and the target filename is derived from Content-Disposition or the URL, so it is stable across attempts. There is no Range resume: a download that fails at 90% starts over. --- src/services/restore/downloader.rs | 23 +++++++ src/tests/services/mod.rs | 1 + .../services/restore_downloader_tests.rs | 64 +++++++++++++++++++ 3 files changed, 88 insertions(+) create mode 100644 src/tests/services/restore_downloader_tests.rs diff --git a/src/services/restore/downloader.rs b/src/services/restore/downloader.rs index a154160..da65f96 100644 --- a/src/services/restore/downloader.rs +++ b/src/services/restore/downloader.rs @@ -8,6 +8,7 @@ use std::sync::Arc; use std::time::Instant; use tokio::io::AsyncWriteExt; use crate::services::backup::logger::JobLogger; +use crate::utils::retry::{RetryPolicy, retry}; fn human_size(bytes: u64) -> String { if bytes >= 1024 * 1024 { @@ -26,6 +27,28 @@ impl RestoreService { tmp_path: &Path, logger: Arc, expected_size: Option, + ) -> Result { + let policy = RetryPolicy::default(); + + let logger_ref = &logger; + + retry("Backup download", &logger, &policy, move |_| { + let expected = expected_size.clone(); + + async move { + self.download_once(file_url, tmp_path, Arc::clone(logger_ref), expected) + .await + } + }) + .await + } + + pub async fn download_once( + &self, + file_url: &str, + tmp_path: &Path, + logger: Arc, + expected_size: Option, ) -> Result { logger.log("info", "Start downloading backup archive".to_string()); diff --git a/src/tests/services/mod.rs b/src/tests/services/mod.rs index 9f6f9b6..aa4be3a 100644 --- a/src/tests/services/mod.rs +++ b/src/tests/services/mod.rs @@ -3,3 +3,4 @@ mod backup_runner_tests; mod backup_uploader_tests; mod config_tests; mod dashboard_config_tests; +mod restore_downloader_tests; diff --git a/src/tests/services/restore_downloader_tests.rs b/src/tests/services/restore_downloader_tests.rs new file mode 100644 index 0000000..905545e --- /dev/null +++ b/src/tests/services/restore_downloader_tests.rs @@ -0,0 +1,64 @@ +use crate::core::context::Context; +use crate::services::api::ApiClient; +use crate::services::backup::logger::JobLogger; +use crate::services::restore::RestoreService; +use crate::tests::init_tracing_for_test; +use crate::utils::edge_key::EdgeKey; + +use std::sync::Arc; +use tempfile::TempDir; +use wiremock::matchers::{method, path}; +use wiremock::{Mock, MockServer, ResponseTemplate}; + +#[tokio::test] +async fn a_failing_download_is_retried_until_it_succeeds() { + init_tracing_for_test(); + let server = MockServer::start().await; + + Mock::given(method("GET")) + .and(path("/backups/archive.tar.gz")) + .respond_with(ResponseTemplate::new(503)) + .up_to_n_times(2) + .with_priority(1) + .expect(2) + .mount(&server) + .await; + + Mock::given(method("GET")) + .and(path("/backups/archive.tar.gz")) + .respond_with(ResponseTemplate::new(200).set_body_bytes(b"portabase-archive".to_vec())) + .with_priority(2) + .expect(1) + .mount(&server) + .await; + + let ctx = Context { + edge_key: EdgeKey { + server_url: server.uri(), + agent_id: "agent-1".to_string(), + master_key_b64: String::new(), + }, + api: ApiClient::new(server.uri()), + }; + + let service = RestoreService::new(Arc::new(ctx)); + + let temp_dir = TempDir::new().unwrap(); + let logger = Arc::new(JobLogger::new()); + let url = format!("{}/backups/archive.tar.gz", server.uri()); + + let downloaded = service + .download_backup(&url, temp_dir.path(), Arc::clone(&logger), None) + .await + .unwrap(); + + assert_eq!(std::fs::read(&downloaded).unwrap(), b"portabase-archive"); + + let entries = Arc::try_unwrap(logger).unwrap().into_entries(); + assert_eq!(entries.iter().filter(|e| e.level == "warn").count(), 2); + assert!( + entries + .iter() + .any(|e| e.message == "Backup download succeeded on attempt 3/3") + ); +}