Skip to content

Commit 5d04b42

Browse files
FranciscoTGouveiarami3l
authored andcommitted
fix(downloads): correct faulty behavior when a download fails
Until now, the progress bars were not being correctly handled which would make an error (i.e. "failed to download file X") pollute the output.
1 parent 3fb8d65 commit 5d04b42

File tree

3 files changed

+42
-6
lines changed

3 files changed

+42
-6
lines changed

src/cli/download_tracker.rs

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,18 @@ impl DownloadTracker {
5555
}
5656
true
5757
}
58+
Notification::Install(In::Utils(Un::DownloadFailed(url))) => {
59+
self.download_failed(url);
60+
false
61+
}
5862
Notification::Install(In::DownloadingComponent(component, _, _, url)) => {
5963
self.create_progress_bar(component.to_owned(), url.to_owned());
6064
true
6165
}
62-
Notification::Install(In::RetryingDownload(_url)) => true,
66+
Notification::Install(In::RetryingDownload(url)) => {
67+
self.retrying_download(url);
68+
true
69+
}
6370
_ => false,
6471
}
6572
}
@@ -105,4 +112,30 @@ impl DownloadTracker {
105112
);
106113
pb.finish();
107114
}
115+
116+
/// Notifies self that the download has failed.
117+
pub(crate) fn download_failed(&mut self, url: &str) {
118+
let Some(pb) = self.file_progress_bars.get(url) else {
119+
return;
120+
};
121+
pb.set_style(
122+
ProgressStyle::with_template("{msg:>12.bold} download failed after {elapsed}")
123+
.unwrap(),
124+
);
125+
pb.finish();
126+
}
127+
128+
/// Notifies self that the download is being retried.
129+
pub(crate) fn retrying_download(&mut self, url: &str) {
130+
let Some(pb) = self.file_progress_bars.get(url) else {
131+
return;
132+
};
133+
pb.set_style(
134+
ProgressStyle::with_template(
135+
"{msg:>12.bold} [{bar:40}] {bytes}/{total_bytes} ({bytes_per_sec}, ETA: {eta})",
136+
)
137+
.unwrap()
138+
.progress_chars("## "),
139+
);
140+
}
108141
}

src/download/mod.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -219,9 +219,10 @@ async fn download_file_(
219219
.await;
220220

221221
// The notification should only be sent if the download was successful (i.e. didn't timeout)
222-
if res.is_ok() {
223-
notify_handler(Notification::DownloadFinished(Some(url.as_str())));
224-
}
222+
notify_handler(match &res {
223+
Ok(_) => Notification::DownloadFinished(Some(url.as_str())),
224+
Err(_) => Notification::DownloadFailed(url.as_str()),
225+
});
225226

226227
res
227228
}
@@ -558,7 +559,6 @@ mod reqwest_be {
558559
#[cfg(feature = "reqwest-rustls-tls")]
559560
use rustls_platform_verifier::BuilderVerifierExt;
560561
use tokio_stream::StreamExt;
561-
use tracing::error;
562562
use url::Url;
563563

564564
use super::{DownloadError, Event};
@@ -576,7 +576,6 @@ mod reqwest_be {
576576

577577
let res = request(url, resume_from, client)
578578
.await
579-
.inspect_err(|error| error!(?error, "failed to download file"))
580579
.context("error downloading file")?;
581580

582581
if !res.status().is_success() {

src/utils/notifications.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ pub enum Notification<'a> {
2020
DownloadDataReceived(&'a [u8], Option<&'a str>),
2121
/// Download has finished.
2222
DownloadFinished(Option<&'a str>),
23+
/// Download has failed.
24+
DownloadFailed(&'a str),
2325
NoCanonicalPath(&'a Path),
2426
ResumingPartialDownload,
2527
/// This would make more sense as a crate::notifications::Notification
@@ -50,6 +52,7 @@ impl Notification<'_> {
5052
| DownloadContentLengthReceived(_, _)
5153
| DownloadDataReceived(_, _)
5254
| DownloadFinished(_)
55+
| DownloadFailed(_)
5356
| ResumingPartialDownload
5457
| UsingCurl
5558
| UsingReqwest => NotificationLevel::Debug,
@@ -88,6 +91,7 @@ impl Display for Notification<'_> {
8891
DownloadContentLengthReceived(len, _) => write!(f, "download size is: '{len}'"),
8992
DownloadDataReceived(data, _) => write!(f, "received some data of size {}", data.len()),
9093
DownloadFinished(_) => write!(f, "download finished"),
94+
DownloadFailed(_) => write!(f, "download failed"),
9195
NoCanonicalPath(path) => write!(f, "could not canonicalize path: '{}'", path.display()),
9296
ResumingPartialDownload => write!(f, "resuming partial download"),
9397
UsingCurl => write!(f, "downloading with curl"),

0 commit comments

Comments
 (0)