From 836c4ab7b692b1d0fa2227b92d62bfa2e5b38933 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Wed, 6 May 2026 21:48:15 +0200 Subject: [PATCH 1/8] download: inline partial download resumption event --- src/download/mod.rs | 4 +--- src/download/tests.rs | 8 +------- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/src/download/mod.rs b/src/download/mod.rs index ce3c69f16c..8374bba9f8 100644 --- a/src/download/mod.rs +++ b/src/download/mod.rs @@ -183,7 +183,6 @@ impl<'a> Download<'a> { status.received_data(data.len()) } } - Event::ResumingPartialDownload => debug!("resuming partial download"), } Ok(()) @@ -231,7 +230,7 @@ impl<'a> Download<'a> { let downloaded_so_far = if let Ok(mut partial) = possible_partial { if let Some(cb) = callback { - cb(Event::ResumingPartialDownload)?; + debug!("resuming partial download"); let mut buf = vec![0; 32768]; let mut downloaded_so_far = 0; @@ -409,7 +408,6 @@ enum Tls { #[derive(Debug, Copy, Clone)] enum Event<'a> { - ResumingPartialDownload, /// Received the Content-Length of the to-be downloaded data. DownloadContentLengthReceived(u64), /// Received some data. diff --git a/src/download/tests.rs b/src/download/tests.rs index 0375cf9218..1932d2e938 100644 --- a/src/download/tests.rs +++ b/src/download/tests.rs @@ -21,7 +21,7 @@ mod reqwest { use std::error::Error; use std::net::TcpListener; use std::sync::Mutex; - use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; + use std::sync::atomic::{AtomicUsize, Ordering}; use std::thread; use std::time::Duration; @@ -136,7 +136,6 @@ mod reqwest { let from_url = format!("http://{addr}").parse().unwrap(); - let callback_partial = AtomicBool::new(false); let callback_len = Mutex::new(None); let received_in_callback = Mutex::new(Vec::new()); @@ -145,10 +144,6 @@ mod reqwest { .with_resume() .download_to_path(Some(&|msg| { match msg { - Event::ResumingPartialDownload => { - assert!(!callback_partial.load(Ordering::SeqCst)); - callback_partial.store(true, Ordering::SeqCst); - } Event::DownloadContentLengthReceived(len) => { let mut flag = callback_len.lock().unwrap(); assert!(flag.is_none()); @@ -166,7 +161,6 @@ mod reqwest { .await .expect("Test download failed"); - assert!(callback_partial.into_inner()); assert_eq!(*callback_len.lock().unwrap(), Some(5)); let observed_bytes = received_in_callback.into_inner().unwrap(); assert_eq!(observed_bytes, vec![b'1', b'2', b'3', b'4', b'5']); From 8cd4b7c4e09983430ae32e56feeb95cc0b9f6aae Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Wed, 6 May 2026 21:52:54 +0200 Subject: [PATCH 2/8] download: inline callback wrapper --- src/download/mod.rs | 44 ++++++++++++++++++++------------------------ 1 file changed, 20 insertions(+), 24 deletions(-) diff --git a/src/download/mod.rs b/src/download/mod.rs index 8374bba9f8..cd641bc6e0 100644 --- a/src/download/mod.rs +++ b/src/download/mod.rs @@ -224,7 +224,7 @@ impl<'a> Download<'a> { } async fn download_impl(&self, callback: Option>) -> anyhow::Result<()> { - let (file, resume_from) = if self.resume { + let (mut file, resume_from) = if self.resume { // TODO: blocking call let possible_partial = OpenOptions::new().read(true).open(self.path); @@ -275,7 +275,6 @@ impl<'a> Download<'a> { ) }; - let file = RefCell::new(file); let client = match self.options.tls { #[cfg(feature = "reqwest-rustls-tls")] Tls::Rustls => rustls_client(self.options.timeout)?, @@ -284,25 +283,10 @@ impl<'a> Download<'a> { }; // TODO: the sync callback will stall the async runtime if IO calls block, which is OS dependent. Rearrange. - self.execute( - resume_from, - &|event| { - if let Event::DownloadDataReceived(data) = event { - file.borrow_mut() - .write_all(data) - .context("unable to write download to disk")?; - } - match callback { - Some(cb) => cb(event), - None => Ok(()), - } - }, - client, - ) - .await?; + self.execute(&mut file, resume_from, callback, client) + .await?; - file.borrow_mut() - .sync_data() + file.sync_data() .context("unable to sync download to disk")?; Ok::<(), anyhow::Error>(()) @@ -310,8 +294,9 @@ impl<'a> Download<'a> { async fn execute( &self, + file: &mut fs::File, resume_from: u64, - callback: &dyn Fn(Event<'_>) -> anyhow::Result<()>, + callback: Option<&dyn Fn(Event<'_>) -> anyhow::Result<()>>, client: &Client, ) -> anyhow::Result<()> { // Short-circuit reqwest for the "file:" URL scheme @@ -338,7 +323,12 @@ impl<'a> Download<'a> { if bytes_read == 0 { break; } - callback(Event::DownloadDataReceived(&buffer[0..bytes_read]))?; + + file.write_all(&buffer[0..bytes_read]) + .context("unable to write download to disk")?; + if let Some(callback) = callback { + callback(Event::DownloadDataReceived(&buffer[0..bytes_read]))?; + } } return Ok(()); @@ -365,13 +355,19 @@ impl<'a> Download<'a> { if let Some(len) = res.content_length() { let len = len + resume_from; - callback(Event::DownloadContentLengthReceived(len))?; + if let Some(callback) = callback { + callback(Event::DownloadContentLengthReceived(len))?; + } } let mut stream = res.bytes_stream(); while let Some(item) = stream.next().await { let bytes = item.map_err(DownloadError::Reqwest)?; - callback(Event::DownloadDataReceived(&bytes))?; + file.write_all(&bytes) + .context("unable to write download to disk")?; + if let Some(callback) = callback { + callback(Event::DownloadDataReceived(&bytes))?; + } } Ok(()) } From 3c153f365980a20e424b2b698cf5e98b34e03b21 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Wed, 6 May 2026 21:55:35 +0200 Subject: [PATCH 3/8] download: inline length received event --- src/download/mod.rs | 11 ++--------- src/download/tests.rs | 7 ------- 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/src/download/mod.rs b/src/download/mod.rs index cd641bc6e0..fe93e0ea36 100644 --- a/src/download/mod.rs +++ b/src/download/mod.rs @@ -173,11 +173,6 @@ impl<'a> Download<'a> { } match msg { - Event::DownloadContentLengthReceived(len) => { - if let Some(status) = self.status { - status.received_length(len) - } - } Event::DownloadDataReceived(data) => { if let Some(status) = self.status { status.received_data(data.len()) @@ -355,8 +350,8 @@ impl<'a> Download<'a> { if let Some(len) = res.content_length() { let len = len + resume_from; - if let Some(callback) = callback { - callback(Event::DownloadContentLengthReceived(len))?; + if let Some(status) = self.status { + status.received_length(len); } } @@ -404,8 +399,6 @@ enum Tls { #[derive(Debug, Copy, Clone)] enum Event<'a> { - /// Received the Content-Length of the to-be downloaded data. - DownloadContentLengthReceived(u64), /// Received some data. DownloadDataReceived(&'a [u8]), } diff --git a/src/download/tests.rs b/src/download/tests.rs index 1932d2e938..9946cdfd0e 100644 --- a/src/download/tests.rs +++ b/src/download/tests.rs @@ -136,7 +136,6 @@ mod reqwest { let from_url = format!("http://{addr}").parse().unwrap(); - let callback_len = Mutex::new(None); let received_in_callback = Mutex::new(Vec::new()); OPTIONS @@ -144,11 +143,6 @@ mod reqwest { .with_resume() .download_to_path(Some(&|msg| { match msg { - Event::DownloadContentLengthReceived(len) => { - let mut flag = callback_len.lock().unwrap(); - assert!(flag.is_none()); - *flag = Some(len); - } Event::DownloadDataReceived(data) => { for b in data.iter() { received_in_callback.lock().unwrap().push(*b); @@ -161,7 +155,6 @@ mod reqwest { .await .expect("Test download failed"); - assert_eq!(*callback_len.lock().unwrap(), Some(5)); let observed_bytes = received_in_callback.into_inner().unwrap(); 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"); From c24e8ea6eb9989db4674e49e63cecb767a1cb293 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Wed, 6 May 2026 22:02:27 +0200 Subject: [PATCH 4/8] download: replace callback with simple method --- src/download/mod.rs | 61 +++++++++++++------------------------------ src/download/tests.rs | 44 +++---------------------------- 2 files changed, 22 insertions(+), 83 deletions(-) diff --git a/src/download/mod.rs b/src/download/mod.rs index fe93e0ea36..6cd12b6e66 100644 --- a/src/download/mod.rs +++ b/src/download/mod.rs @@ -163,29 +163,9 @@ impl<'a> Download<'a> { async fn download_file_(&self) -> anyhow::Result<()> { debug!(url = %self.url, "downloading file"); - // This callback will write the download to disk and optionally - // hash the contents, then forward the notification up the stack - let callback: &dyn Fn(Event<'_>) -> anyhow::Result<()> = &|msg| { - if let Event::DownloadDataReceived(data) = msg - && let Some(h) = self.hasher.as_ref() - { - h.borrow_mut().update(data); - } - - match msg { - Event::DownloadDataReceived(data) => { - if let Some(status) = self.status { - status.received_data(data.len()) - } - } - } - - Ok(()) - }; - // Download the file - let res = self.download_to_path(Some(callback)).await; + let res = self.download_to_path().await; // The notification should only be sent if the download was successful (i.e. didn't timeout) if let Some(status) = self.status { @@ -198,8 +178,8 @@ impl<'a> Download<'a> { res } - async fn download_to_path(&self, callback: Option>) -> anyhow::Result<()> { - let Err(err) = self.download_impl(callback).await else { + async fn download_to_path(&self) -> anyhow::Result<()> { + let Err(err) = self.download_impl().await else { return Ok(()); }; @@ -218,13 +198,13 @@ impl<'a> Download<'a> { ) } - async fn download_impl(&self, callback: Option>) -> anyhow::Result<()> { + async fn download_impl(&self) -> anyhow::Result<()> { let (mut file, resume_from) = if self.resume { // TODO: blocking call let possible_partial = OpenOptions::new().read(true).open(self.path); let downloaded_so_far = if let Ok(mut partial) = possible_partial { - if let Some(cb) = callback { + if self.status.is_some() || self.hasher.is_some() { debug!("resuming partial download"); let mut buf = vec![0; 32768]; @@ -235,7 +215,7 @@ impl<'a> Download<'a> { if n == 0 { break; } - cb(Event::DownloadDataReceived(&buf[..n]))?; + self.data_received(&buf[..n]); } downloaded_so_far @@ -278,8 +258,7 @@ impl<'a> Download<'a> { }; // TODO: the sync callback will stall the async runtime if IO calls block, which is OS dependent. Rearrange. - self.execute(&mut file, resume_from, callback, client) - .await?; + self.execute(&mut file, resume_from, client).await?; file.sync_data() .context("unable to sync download to disk")?; @@ -291,7 +270,6 @@ impl<'a> Download<'a> { &self, file: &mut fs::File, resume_from: u64, - callback: Option<&dyn Fn(Event<'_>) -> anyhow::Result<()>>, client: &Client, ) -> anyhow::Result<()> { // Short-circuit reqwest for the "file:" URL scheme @@ -321,9 +299,7 @@ impl<'a> Download<'a> { file.write_all(&buffer[0..bytes_read]) .context("unable to write download to disk")?; - if let Some(callback) = callback { - callback(Event::DownloadDataReceived(&buffer[0..bytes_read]))?; - } + self.data_received(&buffer[0..bytes_read]); } return Ok(()); @@ -360,12 +336,19 @@ impl<'a> Download<'a> { let bytes = item.map_err(DownloadError::Reqwest)?; file.write_all(&bytes) .context("unable to write download to disk")?; - if let Some(callback) = callback { - callback(Event::DownloadDataReceived(&bytes))?; - } + self.data_received(&bytes); } Ok(()) } + + fn data_received(&self, data: &[u8]) { + if let Some(hasher) = &self.hasher { + hasher.borrow_mut().update(data); + } + if let Some(status) = self.status { + status.received_data(data.len()); + } + } } pub(crate) fn is_network_failure(err: &anyhow::Error) -> bool { @@ -397,14 +380,6 @@ enum Tls { NativeTls, } -#[derive(Debug, Copy, Clone)] -enum Event<'a> { - /// Received some data. - DownloadDataReceived(&'a [u8]), -} - -type DownloadCallback<'a> = &'a dyn Fn(Event<'_>) -> anyhow::Result<()>; - fn client_generic() -> ClientBuilder { Client::builder() // HACK: set `pool_max_idle_per_host` to `0` to avoid an issue in the underlying diff --git a/src/download/tests.rs b/src/download/tests.rs index 9946cdfd0e..00675fe18a 100644 --- a/src/download/tests.rs +++ b/src/download/tests.rs @@ -20,7 +20,6 @@ mod reqwest { use std::env::set_var; use std::error::Error; use std::net::TcpListener; - use std::sync::Mutex; use std::sync::atomic::{AtomicUsize, Ordering}; use std::thread; use std::time::Duration; @@ -30,7 +29,7 @@ mod reqwest { use url::Url; use super::{scrub_env, serve_file, tmp_dir, write_file}; - use crate::download::{DownloadOptions, Event, Tls}; + use crate::download::{DownloadOptions, Tls}; const OPTIONS: DownloadOptions = DownloadOptions { tls: DOWNLOAD_BACKEND, @@ -118,48 +117,13 @@ mod reqwest { OPTIONS .start(&from_url, &target_path) .with_resume() - .download_to_path(None) + .download_to_path() .await .expect("Test download failed"); assert_eq!(std::fs::read_to_string(&target_path).unwrap(), "12345"); } - #[tokio::test] - async fn callback_gets_all_data_as_if_the_download_happened_all_at_once() { - let _guard = scrub_env().await; - let tmpdir = tmp_dir(); - let target_path = tmpdir.path().join("downloaded"); - write_file(&target_path, "123"); - - let addr = serve_file(b"xxx45".to_vec(), true); - - let from_url = format!("http://{addr}").parse().unwrap(); - - let received_in_callback = Mutex::new(Vec::new()); - - OPTIONS - .start(&from_url, &target_path) - .with_resume() - .download_to_path(Some(&|msg| { - match msg { - Event::DownloadDataReceived(data) => { - for b in data.iter() { - received_in_callback.lock().unwrap().push(*b); - } - } - } - - Ok(()) - })) - .await - .expect("Test download failed"); - - let observed_bytes = received_in_callback.into_inner().unwrap(); - 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 resume_partial_fails_if_server_ignores_range() { let _guard = scrub_env().await; @@ -173,7 +137,7 @@ mod reqwest { OPTIONS .start(&from_url, &target_path) .with_resume() - .download_to_path(None) + .download_to_path() .await .expect_err("download should fail if server ignores range"); @@ -197,7 +161,7 @@ mod reqwest { } .start(&from_url, &target_path) .with_resume() - .download_to_path(None) + .download_to_path() .await .expect_err("download should fail with a connect error"); From 1a397c079b4549c4b970e9a9d6cef07d210f61c2 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Wed, 6 May 2026 22:05:37 +0200 Subject: [PATCH 5/8] download: avoid wrapping hasher in RefCell --- src/dist/download.rs | 4 ++-- src/download/mod.rs | 21 ++++++++++----------- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/dist/download.rs b/src/dist/download.rs index f1d25f10e8..04456692e5 100644 --- a/src/dist/download.rs +++ b/src/dist/download.rs @@ -82,7 +82,7 @@ impl<'a> DownloadCfg<'a> { let partial_file_existed = partial_file_path.exists(); let mut hasher = Sha256::new(); - let download = DownloadOptions::try_from(self.process)? + let mut download = DownloadOptions::try_from(self.process)? .start(url, &partial_file_path) .with_hasher(&mut hasher) .with_status(status) @@ -268,7 +268,7 @@ impl<'a> DownloadCfg<'a> { .start(&url, &file) .with_hasher(&mut hasher); - let download = match status { + let mut download = match status { Some(status) => download.with_status(status), None => download, }; diff --git a/src/download/mod.rs b/src/download/mod.rs index 6cd12b6e66..5fbcc60ca3 100644 --- a/src/download/mod.rs +++ b/src/download/mod.rs @@ -1,6 +1,5 @@ //! Easy file downloading -use std::cell::RefCell; use std::fs::{self, OpenOptions, remove_file}; use std::io::{self, Read, Seek, SeekFrom, Write}; use std::num::NonZero; @@ -106,7 +105,7 @@ impl TryFrom<&Process> for DownloadOptions { pub struct Download<'a> { url: &'a Url, path: &'a Path, - hasher: Option>, + hasher: Option<&'a mut Sha256>, status: Option<&'a DownloadStatus>, resume: bool, options: DownloadOptions, @@ -114,7 +113,7 @@ pub struct Download<'a> { impl<'a> Download<'a> { pub(crate) fn with_hasher(mut self, hasher: &'a mut Sha256) -> Self { - self.hasher = Some(RefCell::new(hasher)); + self.hasher = Some(hasher); self } @@ -128,7 +127,7 @@ impl<'a> Download<'a> { self } - pub(crate) async fn download(&self) -> anyhow::Result<()> { + pub(crate) async fn download(&mut self) -> anyhow::Result<()> { match self.download_file_().await { Ok(_) => Ok(()), Err(e) => { @@ -160,7 +159,7 @@ impl<'a> Download<'a> { } } - async fn download_file_(&self) -> anyhow::Result<()> { + async fn download_file_(&mut self) -> anyhow::Result<()> { debug!(url = %self.url, "downloading file"); // Download the file @@ -178,7 +177,7 @@ impl<'a> Download<'a> { res } - async fn download_to_path(&self) -> anyhow::Result<()> { + async fn download_to_path(&mut self) -> anyhow::Result<()> { let Err(err) = self.download_impl().await else { return Ok(()); }; @@ -198,7 +197,7 @@ impl<'a> Download<'a> { ) } - async fn download_impl(&self) -> anyhow::Result<()> { + async fn download_impl(&mut self) -> anyhow::Result<()> { let (mut file, resume_from) = if self.resume { // TODO: blocking call let possible_partial = OpenOptions::new().read(true).open(self.path); @@ -267,7 +266,7 @@ impl<'a> Download<'a> { } async fn execute( - &self, + &mut self, file: &mut fs::File, resume_from: u64, client: &Client, @@ -341,9 +340,9 @@ impl<'a> Download<'a> { Ok(()) } - fn data_received(&self, data: &[u8]) { - if let Some(hasher) = &self.hasher { - hasher.borrow_mut().update(data); + fn data_received(&mut self, data: &[u8]) { + if let Some(hasher) = &mut self.hasher { + hasher.update(data); } if let Some(status) = self.status { status.received_data(data.len()); From 94b987ea757baf8728f175677b27be0e70870786 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Wed, 6 May 2026 22:08:08 +0200 Subject: [PATCH 6/8] download: inline download_file_() --- src/download/mod.rs | 32 +++++++++++++------------------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/src/download/mod.rs b/src/download/mod.rs index 5fbcc60ca3..bb7e587214 100644 --- a/src/download/mod.rs +++ b/src/download/mod.rs @@ -128,7 +128,19 @@ impl<'a> Download<'a> { } pub(crate) async fn download(&mut self) -> anyhow::Result<()> { - match self.download_file_().await { + debug!(url = %self.url, "downloading file"); + + let res = self.download_to_path().await; + + // The notification should only be sent if the download was successful (i.e. didn't timeout) + if let Some(status) = self.status { + match &res { + Ok(_) => status.finished(), + Err(_) => status.failed(), + }; + } + + match res { Ok(_) => Ok(()), Err(e) => { if e.downcast_ref::().is_some() { @@ -159,24 +171,6 @@ impl<'a> Download<'a> { } } - async fn download_file_(&mut self) -> anyhow::Result<()> { - debug!(url = %self.url, "downloading file"); - - // Download the file - - let res = self.download_to_path().await; - - // The notification should only be sent if the download was successful (i.e. didn't timeout) - if let Some(status) = self.status { - match &res { - Ok(_) => status.finished(), - Err(_) => status.failed(), - }; - } - - res - } - async fn download_to_path(&mut self) -> anyhow::Result<()> { let Err(err) = self.download_impl().await else { return Ok(()); From 384bf607a56c34396d3929c2b948baf1096ea99f Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Wed, 6 May 2026 22:09:08 +0200 Subject: [PATCH 7/8] download: reduce rightward drift in download() --- src/download/mod.rs | 54 ++++++++++++++++++++++----------------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/src/download/mod.rs b/src/download/mod.rs index bb7e587214..c2e554c000 100644 --- a/src/download/mod.rs +++ b/src/download/mod.rs @@ -140,35 +140,35 @@ impl<'a> Download<'a> { }; } - match res { - Ok(_) => Ok(()), - Err(e) => { - if e.downcast_ref::().is_some() { - return Err(e); + let Err(e) = res else { + return Ok(()); + }; + + if e.downcast_ref::().is_some() { + return Err(e); + } + + 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(DownloadError::HttpStatus(416)) => false, + Some(DownloadError::HttpStatus(400..=499)) | Some(DownloadError::FileNotFound) => true, + _ => false, + }; + + Err(e).with_context(|| { + if is_client_error { + RustupError::DownloadNotExists { + url: self.url.clone(), + path: self.path.to_path_buf(), + } + } else { + RustupError::DownloadingFile { + url: self.url.clone(), + path: self.path.to_path_buf(), } - 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(DownloadError::HttpStatus(416)) => false, - Some(DownloadError::HttpStatus(400..=499)) - | Some(DownloadError::FileNotFound) => true, - _ => false, - }; - Err(e).with_context(|| { - if is_client_error { - RustupError::DownloadNotExists { - url: self.url.clone(), - path: self.path.to_path_buf(), - } - } else { - RustupError::DownloadingFile { - url: self.url.clone(), - path: self.path.to_path_buf(), - } - } - }) } - } + }) } async fn download_to_path(&mut self) -> anyhow::Result<()> { From 887173dac5a063df54cbef418eb829dd4bf25cac Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Wed, 6 May 2026 22:11:52 +0200 Subject: [PATCH 8/8] download: inline download_to_path() --- src/download/mod.rs | 44 ++++++++++++++++--------------------------- src/download/tests.rs | 6 +++--- 2 files changed, 19 insertions(+), 31 deletions(-) diff --git a/src/download/mod.rs b/src/download/mod.rs index c2e554c000..8871a936f5 100644 --- a/src/download/mod.rs +++ b/src/download/mod.rs @@ -130,18 +130,26 @@ impl<'a> Download<'a> { pub(crate) async fn download(&mut self) -> anyhow::Result<()> { debug!(url = %self.url, "downloading file"); - let res = self.download_to_path().await; + let Err(err) = self.download_impl().await else { + if let Some(status) = self.status { + status.finished(); + } + return Ok(()); + }; - // The notification should only be sent if the download was successful (i.e. didn't timeout) if let Some(status) = self.status { - match &res { - Ok(_) => status.finished(), - Err(_) => status.failed(), - }; + status.failed(); } - let Err(e) = res else { - return Ok(()); + // 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 Err(file_err) = remove_file(self.path).context("cleaning up cached downloads") + { + file_err.context(err) + } else { + err }; if e.downcast_ref::().is_some() { @@ -171,26 +179,6 @@ impl<'a> Download<'a> { }) } - async fn download_to_path(&mut self) -> anyhow::Result<()> { - let Err(err) = self.download_impl().await else { - return Ok(()); - }; - - // 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 !(self.resume && is_network_failure(&err)) - && let Err(file_err) = - remove_file(self.path).context("cleaning up cached downloads") - { - file_err.context(err) - } else { - err - }, - ) - } - async fn download_impl(&mut self) -> anyhow::Result<()> { let (mut file, resume_from) = if self.resume { // TODO: blocking call diff --git a/src/download/tests.rs b/src/download/tests.rs index 00675fe18a..a49ec7cee6 100644 --- a/src/download/tests.rs +++ b/src/download/tests.rs @@ -117,7 +117,7 @@ mod reqwest { OPTIONS .start(&from_url, &target_path) .with_resume() - .download_to_path() + .download() .await .expect("Test download failed"); @@ -137,7 +137,7 @@ mod reqwest { OPTIONS .start(&from_url, &target_path) .with_resume() - .download_to_path() + .download() .await .expect_err("download should fail if server ignores range"); @@ -161,7 +161,7 @@ mod reqwest { } .start(&from_url, &target_path) .with_resume() - .download_to_path() + .download() .await .expect_err("download should fail with a connect error");