Add basic HTTP retry due to EOF I/O errors

src/ctx.rs

@@ -21,10 +21,15 @@ pub struct Ctx {
     pub tempdir: Option<tempfile::TempDir>,
     pub client: ureq::Agent,
     pub draw_target: ProgressTarget,
+    pub http_retry: u8,
 }
 
 impl Ctx {
-    pub fn with_temp(dt: ProgressTarget, client: ureq::Agent) -> Result<Self, Error> {
+    pub fn with_temp(
+        dt: ProgressTarget,
+        client: ureq::Agent,
+        http_retry: u8,
+    ) -> Result<Self, Error> {
         let td = tempfile::TempDir::new()?;
 
         Ok(Self {
@@ -34,13 +39,15 @@ impl Ctx {
             tempdir: Some(td),
             client,
             draw_target: dt,
+            http_retry,
         })
     }
 
     pub fn with_dir(
         mut work_dir: PathBuf,
         dt: ProgressTarget,
         client: ureq::Agent,
+        http_retry: u8,
     ) -> Result<Self, Error> {
         work_dir.push("dl");
         std::fs::create_dir_all(&work_dir)?;
@@ -54,6 +61,7 @@ impl Ctx {
             tempdir: None,
             client,
             draw_target: dt,
+            http_retry,
         })
     }
 
@@ -62,7 +70,7 @@ impl Ctx {
         url: impl AsRef<str>,
         path: &P,
         checksum: Option<Sha256>,
-        progress: indicatif::ProgressBar,
+        mut progress: indicatif::ProgressBar,
     ) -> Result<bytes::Bytes, Error>
     where
         P: AsRef<Path> + std::fmt::Debug,
@@ -107,25 +115,25 @@ impl Ctx {
             }
         }
 
-        let res = self.client.get(url.as_ref()).call()?;
-
-        let content_length = res
-            .headers()
-            .get("content-length")
-            .and_then(|header| header.to_str().ok()?.parse().ok())
-            .unwrap_or_default();
-        progress.inc_length(content_length);
-
-        let body = bytes::BytesMut::with_capacity(content_length as usize);
+        use bytes::BufMut;
 
         struct ProgressCopy {
             progress: indicatif::ProgressBar,
             inner: bytes::buf::Writer<bytes::BytesMut>,
+            failed: usize,
+            written: usize,
         }
 
         impl std::io::Write for ProgressCopy {
             fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> {
-                self.progress.inc(buf.len() as u64);
+                self.written += buf.len();
+                if self.failed == 0 {
+                    self.progress.inc(buf.len() as u64);
+                } else if self.written > self.failed {
+                    self.progress.inc((self.written - self.failed) as u64);
+                    self.failed = 0;
+                }
+
                 self.inner.write(buf)
             }
 
@@ -134,32 +142,107 @@ impl Ctx {
             }
         }
 
-        use bytes::BufMut;
+        enum DownloadError {
+            Ureq(ureq::Error),
+            Io(std::io::Error),
+            Retry((bytes::BytesMut, indicatif::ProgressBar)),
+        }
+
+        let try_download = |mut body: bytes::BytesMut,
+                            progress: indicatif::ProgressBar|
+         -> Result<bytes::BytesMut, DownloadError> {
+            let res = self
+                .client
+                .get(url.as_ref())
+                .call()
+                .map_err(DownloadError::Ureq)?;
+
+            let content_length = res
+                .headers()
+                .get("content-length")
+                .and_then(|header| header.to_str().ok()?.parse().ok())
+                .unwrap_or_default();
+
+            if body.capacity() > 0 {
+                if body.capacity() as u64 != content_length {
+                    tracing::warn!(url = url.as_ref(), "a previous HTTP GET had a content-length of {}, but we now received a content-length of {content_length}", body.capacity());
+
+                    if body.capacity() as u64 > content_length {
+                        progress.inc_length(body.capacity() as u64 - content_length);
+                    } else {
+                        body.reserve(content_length as usize - body.capacity());
+                    }
+                }
+            } else {
+                body.reserve(content_length as usize);
+                progress.inc_length(content_length);
+            }
 
-        let mut pc = ProgressCopy {
-            progress,
-            inner: body.writer(),
+            let failed = body.len();
+            body.clear();
+
+            let mut pc = ProgressCopy {
+                progress,
+                inner: body.writer(),
+                failed,
+                written: 0,
+            };
+
+            let res = std::io::copy(&mut res.into_body().as_reader(), &mut pc);
+            let body = pc.inner.into_inner();
+
+            match res {
+                Ok(_) => Ok(body),
+                Err(ref err) if err.kind() == std::io::ErrorKind::UnexpectedEof => {
+                    Err(DownloadError::Retry((body, pc.progress)))
+                }
+                Err(err) => Err(DownloadError::Io(err)),
+            }
         };
 
-        std::io::copy(&mut res.into_body().as_reader(), &mut pc)?;
+        let mut tries = self.http_retry + 1;
+        let total = tries;
+        let mut body = bytes::BytesMut::new();
 
-        let body = pc.inner.into_inner().freeze();
+        while tries > 0 {
+            match try_download(body, progress) {
+                Ok(body) => {
+                    let body = body.freeze();
 
-        if let Some(expected) = checksum {
-            let chksum = Sha256::digest(&body);
+                    if let Some(expected) = checksum {
+                        let chksum = Sha256::digest(&body);
 
-            anyhow::ensure!(
-                chksum == expected,
-                "checksum mismatch, expected {expected} != actual {chksum}"
-            );
-        }
+                        anyhow::ensure!(
+                            chksum == expected,
+                            "checksum mismatch, expected {expected} != actual {chksum}"
+                        );
+                    }
 
-        if let Some(parent) = cache_path.parent() {
-            std::fs::create_dir_all(parent)?;
+                    if let Some(parent) = cache_path.parent() {
+                        std::fs::create_dir_all(parent)?;
+                    }
+
+                    std::fs::write(cache_path, &body)?;
+                    return Ok(body);
+                }
+                Err(DownloadError::Retry((b, prog))) => {
+                    tries -= 1;
+                    body = b;
+                    progress = prog;
+                    continue;
+                }
+                Err(DownloadError::Ureq(err)) => {
+                    return Err(err)
+                        .with_context(|| format!("HTTP GET request for {} failed", url.as_ref()));
+                }
+                Err(DownloadError::Io(err)) => {
+                    return Err(err)
+                        .with_context(|| format!("failed to retrieve body for {}", url.as_ref()));
+                }
+            }
         }
 
-        std::fs::write(cache_path, &body)?;
-        Ok(body)
+        anyhow::bail!("failed to retrieve {} after {total} tries due to I/O failures reading the response body, try using --http-retries to increase the retry count", url.as_ref());
     }
 
     #[allow(clippy::too_many_arguments)]

src/main.rs

@@ -230,6 +230,10 @@ pub struct Args {
     /// An HTTPS proxy to use
     #[arg(long, env = "HTTPS_PROXY")]
     https_proxy: Option<String>,
+    /// The number of times an HTTP get will be retried if it fails due to I/O
+    /// failures
+    #[arg(long, env = "XWIN_HTTP_RETRY", default_value = "0")]
+    http_retry: u8,
     /// The architectures to include
     #[arg(
         long,
@@ -287,13 +291,13 @@ fn main() -> Result<(), Error> {
     };
 
     let ctx = if args.temp {
-        xwin::Ctx::with_temp(draw_target, client)?
+        xwin::Ctx::with_temp(draw_target, client, args.http_retry)?
     } else {
         let cache_dir = match &args.cache_dir {
             Some(cd) => cd.clone(),
             None => cwd.join(".xwin-cache"),
         };
-        xwin::Ctx::with_dir(cache_dir, draw_target, client)?
+        xwin::Ctx::with_dir(cache_dir, draw_target, client, args.http_retry)?
     };
 
     let ctx = std::sync::Arc::new(ctx);

tests/compiles.rs

@@ -4,6 +4,7 @@ fn verify_compiles() {
         xwin::PathBuf::from(".xwin-cache/compile-test"),
         xwin::util::ProgressTarget::Hidden,
         ureq::agent(),
+        0,
     )
     .unwrap();
 
@@ -167,6 +168,7 @@ fn verify_compiles_minimized() {
         xwin::PathBuf::from(".xwin-cache/compile-test-minimized"),
         xwin::util::ProgressTarget::Hidden,
         ureq::agent(),
+        0,
     )
     .unwrap();
 

tests/deterministic.rs

@@ -7,6 +7,7 @@ fn verify_deterministic() {
         PathBuf::from(".xwin-cache/deterministic"),
         xwin::util::ProgressTarget::Hidden,
         ureq::agent(),
+        0,
     )
     .unwrap();
 

tests/snapshots/xwin.snap

@@ -84,6 +84,13 @@ Options:
 
           [env: HTTPS_PROXY]
 
+      --http-retry <HTTP_RETRY>
+          The number of times an HTTP get will be retried if it fails due to I/O
+          failures
+
+          [env: XWIN_HTTP_RETRY]
+          [default: 0]
+
       --arch <ARCH>
           The architectures to include