From 8c37a11f0131805b96d7b2f419039f4ab2ff8a2b Mon Sep 17 00:00:00 2001 From: Francisco Gouveia Date: Wed, 11 Feb 2026 11:47:04 +0000 Subject: [PATCH 1/4] fix(downloads): substitute `DEK` alias for `DownloadError` --- src/download/mod.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/download/mod.rs b/src/download/mod.rs index 37ae9b32d4..2a90fc94da 100644 --- a/src/download/mod.rs +++ b/src/download/mod.rs @@ -44,18 +44,19 @@ pub(crate) async fn download_file_with_resume( status: Option<&DownloadStatus>, process: &Process, ) -> anyhow::Result<()> { - use crate::download::DownloadError as DEK; match download_file_(url, path, hasher, resume_from_partial, status, process).await { Ok(_) => Ok(()), Err(e) => { if e.downcast_ref::().is_some() { return Err(e); } - let is_client_error = match e.downcast_ref::() { + let is_client_error = match e.downcast_ref::() { // Specifically treat the bad partial range error as not our // fault in case it was something odd which happened. - Some(DEK::HttpStatus(416)) => false, - Some(DEK::HttpStatus(400..=499)) | Some(DEK::FileNotFound) => true, + Some(DownloadError::HttpStatus(416)) => false, + Some(DownloadError::HttpStatus(400..=499)) | Some(DownloadError::FileNotFound) => { + true + } _ => false, }; Err(e).with_context(|| { From 286db8ac1029942d90ee661cd376a0ee9a0cbeec Mon Sep 17 00:00:00 2001 From: Francisco Gouveia Date: Wed, 11 Feb 2026 11:47:28 +0000 Subject: [PATCH 2/4] feat(downloads): do not delete partial download when network fails --- src/download/mod.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/download/mod.rs b/src/download/mod.rs index 2a90fc94da..6f460adeed 100644 --- a/src/download/mod.rs +++ b/src/download/mod.rs @@ -262,9 +262,18 @@ impl Backend { return Ok(()); }; - // TODO: We currently clear up the cached download on any error, should we restrict it to a subset? + let is_network_failure = match err.downcast_ref::() { + Some(DownloadError::Reqwest(e)) => e.is_timeout() || e.is_connect(), + _ => false, + }; + + // 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. Err( - if let Err(file_err) = remove_file(path).context("cleaning up cached downloads") { + if !(resume_from_partial && is_network_failure) + && let Err(file_err) = remove_file(path).context("cleaning up cached downloads") + { file_err.context(err) } else { err @@ -587,7 +596,7 @@ mod reqwest_be { let mut stream = res.bytes_stream(); while let Some(item) = stream.next().await { - let bytes = item?; + let bytes = item.map_err(DownloadError::Reqwest)?; callback(Event::DownloadDataReceived(&bytes))?; } Ok(()) From 9396e7f1194b9de83a46b7d06473edf4066f234a Mon Sep 17 00:00:00 2001 From: Francisco Gouveia Date: Mon, 9 Feb 2026 20:32:51 +0000 Subject: [PATCH 3/4] test(downloads): ensure that partial files are not removed when network fails --- src/download/tests.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/download/tests.rs b/src/download/tests.rs index dd95c67f02..83eebd3198 100644 --- a/src/download/tests.rs +++ b/src/download/tests.rs @@ -257,6 +257,23 @@ mod reqwest { assert_eq!(observed_bytes, vec![b'1', b'2', b'3', b'4', b'5']); assert_eq!(std::fs::read_to_string(&target_path).unwrap(), "12345"); } + + #[tokio::test] + async fn network_failure_does_not_delete_partial_file() { + let _guard = scrub_env().await; + let tmpdir = tmp_dir(); + let target_path = tmpdir.path().join("downloaded.partial"); + write_file(&target_path, "123"); + + let from_url = "http://240.0.0.0:1080".parse().unwrap(); + Backend::Reqwest(TlsBackend::NativeTls) + .download_to_path(&from_url, &target_path, true, None, Duration::from_secs(1)) + .await + .expect_err("download should fail with a connect error"); + + assert!(target_path.exists(), "partial file should not be deleted"); + assert_eq!(std::fs::read_to_string(&target_path).unwrap(), "123"); + } } pub fn tmp_dir() -> TempDir { From 17a4b01562a7f1c68fae916c9d10c66d62c09777 Mon Sep 17 00:00:00 2001 From: Francisco Gouveia Date: Mon, 9 Feb 2026 21:03:08 +0000 Subject: [PATCH 4/4] fix(downloads): adjust error message for partial files in network failures --- src/dist/download.rs | 13 +++++++------ src/download/mod.rs | 15 +++++++++------ src/errors.rs | 2 ++ 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/dist/download.rs b/src/dist/download.rs index 7467a76c44..e10f027854 100644 --- a/src/dist/download.rs +++ b/src/dist/download.rs @@ -15,7 +15,7 @@ use url::Url; use crate::config::Cfg; use crate::dist::manifest::Manifest; use crate::dist::{Channel, DEFAULT_DIST_SERVER, ToolchainDesc, temp}; -use crate::download::{download_file, download_file_with_resume}; +use crate::download::{download_file, download_file_with_resume, is_network_failure}; use crate::errors::RustupError; use crate::process::Process; use crate::utils; @@ -93,12 +93,13 @@ impl<'a> DownloadCfg<'a> { ) .await { + let is_network_failure = is_network_failure(&e); let err = Err(e); - if partial_file_existed { - return err.context(RustupError::BrokenPartialFile); - } else { - return err; - } + return match (partial_file_existed, is_network_failure) { + (true, true) => err.context(RustupError::IncompletePartialFile), + (true, false) => err.context(RustupError::BrokenPartialFile), + (false, _) => err, + }; }; let actual_hash = format!("{:x}", hasher.finalize()); diff --git a/src/download/mod.rs b/src/download/mod.rs index 6f460adeed..09f6ac9951 100644 --- a/src/download/mod.rs +++ b/src/download/mod.rs @@ -76,6 +76,14 @@ pub(crate) async fn download_file_with_resume( } } +pub(crate) fn is_network_failure(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(), + _ => false, + } +} + async fn download_file_( url: &Url, path: &Path, @@ -262,16 +270,11 @@ impl Backend { return Ok(()); }; - let is_network_failure = match err.downcast_ref::() { - Some(DownloadError::Reqwest(e)) => e.is_timeout() || e.is_connect(), - _ => false, - }; - // 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. Err( - if !(resume_from_partial && is_network_failure) + if !(resume_from_partial && is_network_failure(&err)) && let Err(file_err) = remove_file(path).context("cleaning up cached downloads") { file_err.context(err) diff --git a/src/errors.rs b/src/errors.rs index db09015af8..79e6664d64 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -27,6 +27,8 @@ pub struct OperationError(pub anyhow::Error); pub enum RustupError { #[error("partially downloaded file may have been damaged and was removed, please try again")] BrokenPartialFile, + #[error("partially downloaded file was kept for resumption, please try again")] + IncompletePartialFile, #[error("component download failed for {0}")] ComponentDownloadFailed(String), #[error("failure removing component '{name}', directory does not exist: '{}'", .path.display())]