From d15a2cef71b231fa517d37d6ec07c5aa30c5e6c0 Mon Sep 17 00:00:00 2001 From: rami3l Date: Mon, 2 Nov 2026 17:51:56 +0100 Subject: [PATCH 1/2] refactor(download): drop unused field from `DownloadError::Interrupted` --- src/download/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/download/mod.rs b/src/download/mod.rs index e517c70d7d..0c3dcf3064 100644 --- a/src/download/mod.rs +++ b/src/download/mod.rs @@ -316,7 +316,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); @@ -339,7 +339,7 @@ pub(crate) fn is_network_failure(err: &anyhow::Error) -> bool { #[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 +450,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), From d1db797860f6b56b25ec913e852b104366e541bf Mon Sep 17 00:00:00 2001 From: rami3l Date: Mon, 2 Nov 2026 17:56:37 +0100 Subject: [PATCH 2/2] refactor(download): rename `is_network_failure()` to `should_keep_partial()` The actual intention of this function is whether a `Error` should allow keeping a partial download, so it's better to name it accordingly. --- src/dist/download.rs | 6 +++--- src/download/mod.rs | 12 +++++++----- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/dist/download.rs b/src/dist/download.rs index 07ef22d8a5..6434e12406 100644 --- a/src/dist/download.rs +++ b/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, diff --git a/src/download/mod.rs b/src/download/mod.rs index 0c3dcf3064..93ef2b6211 100644 --- a/src/download/mod.rs +++ b/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) @@ -334,7 +331,12 @@ 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::() { #[cfg(any(feature = "reqwest-rustls-tls", feature = "reqwest-native-tls"))] Some(DownloadError::Reqwest(e)) => e.is_timeout() || e.is_connect(),