Merge pull request #8 from bennyz/oci-retries

add retries to OCI network hiccups

Author: mangelajo

Cargo.lock

@@ -23,6 +23,7 @@ openssl = { version = "0.10", features = ["vendored"] }
 xz2 = "0.1"
 
 [dev-dependencies]
+http-body = "1.0.1"
 wiremock = "0.6"
 tempfile = "3"
 tokio = { version = "1.0", features = ["full", "rt-multi-thread"] }

Cargo.toml

@@ -35,17 +35,43 @@ pub(crate) async fn start_decompressor_process(
 ) -> Result<(Child, &'static str), Box<dyn std::error::Error>> {
     let cmd = get_decompressor_command(url);
 
-    // Check if binary is available before attempting to spawn
     check_binary_available(cmd)?;
+    eprintln!("Using decompressor: {}", cmd);
+    spawn_decompressor(cmd)
+}
 
-    println!("Using decompressor: {}", cmd);
+/// Maps a Compression enum to the corresponding decompressor command
+pub(crate) fn decompressor_for_compression(
+    compression: crate::fls::compression::Compression,
+) -> &'static str {
+    use crate::fls::compression::Compression;
+    match compression {
+        Compression::Gzip => "zcat",
+        Compression::Xz => "xzcat",
+        Compression::Zstd => "zstdcat",
+        Compression::None => "cat",
+    }
+}
 
+/// Starts a decompressor process based on detected compression type
+pub(crate) fn start_decompressor_for_compression(
+    compression: crate::fls::compression::Compression,
+) -> Result<(Child, &'static str), Box<dyn std::error::Error>> {
+    let cmd = decompressor_for_compression(compression);
+    check_binary_available(cmd)?;
+    eprintln!("Using decompressor: {}", cmd);
+    spawn_decompressor(cmd)
+}
+
+/// Spawns a decompressor subprocess with piped stdin/stdout/stderr
+fn spawn_decompressor(
+    cmd: &'static str,
+) -> Result<(Child, &'static str), Box<dyn std::error::Error>> {
     let process = Command::new(cmd)
         .stdin(std::process::Stdio::piped())
         .stdout(std::process::Stdio::piped())
         .stderr(std::process::Stdio::piped())
         .spawn()?;
-
     Ok((process, cmd))
 }
 

src/fls/decompress.rs

@@ -76,8 +76,13 @@ impl DownloadError {
         Self::from_http_status(status)
     }
 
-    /// Create a DownloadError from a reqwest::Error
+    /// Create a DownloadError from a reqwest::Error (owned)
     pub fn from_reqwest(error: reqwest::Error) -> Self {
+        Self::from_reqwest_ref(&error)
+    }
+
+    /// Create a DownloadError from a reqwest::Error reference
+    pub fn from_reqwest_ref(error: &reqwest::Error) -> Self {
         if error.is_status() {
             if let Some(status) = error.status() {
                 return Self::from_http_status(status);
@@ -93,22 +98,17 @@ impl DownloadError {
             let error_str = error.to_string();
 
             // Check error source chain for TLS/SSL/Certificate errors
-            // We use a hybrid approach: check both type names and error messages
-            // - Type names catch concrete types before trait object erasure
-            // - Error messages catch issues when types are erased to dyn Error
-            let mut current_error: Option<&dyn std::error::Error> = Some(&error);
+            let mut current_error: Option<&dyn std::error::Error> = Some(error);
             let mut is_tls_error = false;
 
             while let Some(err) = current_error {
                 let error_msg = err.to_string().to_lowercase();
 
-                // Check error message for TLS-related keywords
-                // This works even when types are erased to trait objects
                 let message_indicates_tls = error_msg.contains("certificate")
                     || error_msg.contains("tls")
                     || error_msg.contains("ssl")
-                    || error_msg.contains("trust setting")  // macOS Security Framework
-                    || error_msg.contains("trust policy"); // macOS Security Framework
+                    || error_msg.contains("trust setting")
+                    || error_msg.contains("trust policy");
 
                 if message_indicates_tls {
                     is_tls_error = true;
@@ -122,14 +122,12 @@ impl DownloadError {
                 return DownloadError::TlsError(error_str);
             }
 
-            // Check for DNS errors
             if error_str.contains("dns") || error_str.contains("failed to lookup address") {
                 return DownloadError::DnsError(error_str);
             }
             return DownloadError::ConnectionError(error_str);
         }
 
-        // Fallback for other error types
         DownloadError::Other(error.to_string())
     }
 
@@ -231,6 +229,42 @@ impl fmt::Display for DownloadError {
 
 impl std::error::Error for DownloadError {}
 
+/// Shared retry handler for download errors.
+///
+/// Returns `Some(Duration)` with the delay to wait before retrying, or `None` if
+/// the error is non-retryable or max retries have been exceeded.
+pub fn handle_download_retry(
+    error: &DownloadError,
+    retry_count: &mut usize,
+    max_retries: usize,
+    default_retry_delay_secs: u64,
+) -> Option<Duration> {
+    if !error.is_retryable() {
+        eprintln!(
+            "\nDownload failed with non-retryable error: {}",
+            error.format_error()
+        );
+        return None;
+    }
+    if *retry_count >= max_retries {
+        eprintln!("\nMax retries ({}) reached, giving up", max_retries);
+        eprintln!("Last error: {}", error.format_error());
+        return None;
+    }
+    let retry_delay = error
+        .suggested_retry_delay()
+        .unwrap_or_else(|| Duration::from_secs(default_retry_delay_secs));
+    eprintln!("\nDownload failed: {}", error.format_error());
+    eprintln!(
+        "Retrying in {} seconds... (attempt {}/{})",
+        retry_delay.as_secs(),
+        *retry_count + 1,
+        max_retries
+    );
+    *retry_count += 1;
+    Some(retry_delay)
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

src/fls/download_error.rs

@@ -86,6 +86,8 @@ fn build_oci_options(options: &FastbootOptions) -> crate::fls::options::OciOptio
         username: options.username.clone(),
         password: options.password.clone(),
         file_pattern: None,
+        max_retries: crate::fls::options::DEFAULT_MAX_RETRIES,
+        retry_delay_secs: crate::fls::options::DEFAULT_RETRY_DELAY_SECS,
     }
 }
 

src/fls/fastboot.rs

@@ -49,44 +49,7 @@ async fn get_decompressor_error(
     }
 }
 
-/// Handles retry logic for download errors.
-///
-/// Returns `Some(Duration)` with the delay to wait before retrying, or `None` if
-/// the error is non-retryable or max retries have been exceeded.
-fn handle_retry_error(
-    error: &DownloadError,
-    retry_count: &mut usize,
-    max_retries: usize,
-    default_retry_delay_secs: u64,
-) -> Option<Duration> {
-    if !error.is_retryable() {
-        eprintln!(
-            "\nDownload failed with non-retryable error: {}",
-            error.format_error()
-        );
-        return None;
-    }
-
-    if *retry_count >= max_retries {
-        eprintln!("\nMax retries ({}) reached, giving up", max_retries);
-        eprintln!("Last error: {}", error.format_error());
-        return None;
-    }
-
-    let retry_delay = error
-        .suggested_retry_delay()
-        .unwrap_or_else(|| Duration::from_secs(default_retry_delay_secs));
-
-    eprintln!("\nDownload failed: {}", error.format_error());
-    eprintln!(
-        "Retrying in {} seconds... (attempt {}/{})",
-        retry_delay.as_secs(),
-        *retry_count + 1,
-        max_retries
-    );
-    *retry_count += 1;
-    Some(retry_delay)
-}
+use crate::fls::download_error::handle_download_retry;
 
 /// Execute a sequence of write commands on the block writer
 async fn execute_write_commands(
@@ -407,7 +370,7 @@ pub async fn flash_from_url(
             match start_download(url, &client, resume_from, &options.headers, debug).await {
                 Ok(r) => r,
                 Err(e) => {
-                    match handle_retry_error(
+                    match handle_download_retry(
                         &e,
                         &mut retry_count,
                         options.max_retries,
@@ -551,7 +514,7 @@ pub async fn flash_from_url(
 
             if let Some(e) = connection_error {
                 eprintln!("\nConnection interrupted: {}", e.format_error());
-                match handle_retry_error(
+                match handle_download_retry(
                     &e,
                     &mut retry_count,
                     options.max_retries,

src/fls/from_url.rs

@@ -24,6 +24,7 @@ pub use from_url::flash_from_url;
 pub use oci::flash_from_oci;
 pub use options::{
     BlockFlashOptions, FastbootOptions, FlashOptions, HttpClientOptions, OciOptions,
+    DEFAULT_MAX_RETRIES, DEFAULT_RETRY_DELAY_SECS,
 };
 
 #[cfg(test)]