refactor(download): cleanup resumption logic of partial download

src/dist/download.rs

@@ -21,7 +21,7 @@ use crate::{
         manifest::{Manifest, ManifestWithHash},
         temp,
     },
-    download::{DownloadOptions, is_network_failure},
+    download::{DownloadOptions, should_keep_partial},
     errors::RustupError,
     process::Process,
     utils,
@@ -87,9 +87,9 @@ impl<'a> DownloadCfg<'a> {
             .with_resume();
 
         if let Err(e) = download.download().await {
-            let is_network_failure = is_network_failure(&e);
+            let should_keep_partial = should_keep_partial(&e);
             let err = Err(e);
-            return match (partial_file_existed, is_network_failure) {
+            return match (partial_file_existed, should_keep_partial) {
                 (true, true) => err.context(RustupError::IncompletePartialFile),
                 (true, false) => err.context(RustupError::BrokenPartialFile),
                 (false, _) => err,

src/download/mod.rs

@@ -143,10 +143,7 @@ impl<'a> Download<'a> {
             status.failed();
         }
 
-        // TODO: Currently, we only refrain from removing the cached download
-        // if there was a network failure from the client side.
-        // It may be worth looking for other cases where removal is also not desired.
-        let e = if !(self.resume && is_network_failure(&err))
+        let e = if !(self.resume && should_keep_partial(&err))
             && let Err(file_err) = remove_file(self.path).context("cleaning up cached downloads")
         {
             file_err.context(err)
@@ -316,7 +313,7 @@ impl<'a> Download<'a> {
 
         let mut stream = res.bytes_stream();
         while let Some(item) = stream.next().await {
-            let bytes = item.map_err(DownloadError::Interrupted)?;
+            let bytes = item.map_err(|_| DownloadError::Interrupted)?;
             file.write_all(&bytes)
                 .context("unable to write download to disk")?;
             self.data_received(&bytes);
@@ -334,12 +331,17 @@ impl<'a> Download<'a> {
     }
 }
 
-pub(crate) fn is_network_failure(err: &anyhow::Error) -> bool {
+/// Returns whether the partial download should be kept for resumption.
+///
+/// This is true for errors that are likely to be transient, such as network timeouts or connection
+/// interruptions. For other errors, the partial download is deleted to avoid leaving a corrupted
+/// file on disk.
+pub(crate) fn should_keep_partial(err: &anyhow::Error) -> bool {
     match err.downcast_ref::<DownloadError>() {
         #[cfg(any(feature = "reqwest-rustls-tls", feature = "reqwest-native-tls"))]
         Some(DownloadError::Reqwest(e)) => e.is_timeout() || e.is_connect(),
         #[cfg(any(feature = "reqwest-rustls-tls", feature = "reqwest-native-tls"))]
-        Some(DownloadError::Interrupted(_)) => true,
+        Some(DownloadError::Interrupted) => true,
         _ => false,
     }
 }
@@ -450,8 +452,8 @@ enum DownloadError {
     #[error(transparent)]
     Reqwest(#[from] reqwest::Error),
     #[cfg(any(feature = "reqwest-rustls-tls", feature = "reqwest-native-tls"))]
-    #[error(transparent)]
-    Interrupted(reqwest::Error),
+    #[error("connection interrupted by server")]
+    Interrupted,
     #[cfg(feature = "reqwest-rustls-tls")]
     #[error("failed to initialize platform verifier")]
     PlatformVerifierInit(#[source] rustls::Error),