Skip to content

Commit

Permalink
fix(install): retry downloads of registry info / tarballs (#26278)
Browse files Browse the repository at this point in the history
Fixes #26085.

Adds a basic retry utility with some defaults, starts off with a 100ms
wait, then 250ms, then 500ms

I've applied the retry in the http client, reusing an existing function,
so this also applies to retrying downloads of deno binaries in `upgrade`
and `compile`. I can make a separate function that doesn't retry so this
doesn't affect `upgrade` and `compile`, but it seemed desirable to have
retries there too, so I left it in.
  • Loading branch information
nathanwhit authored Oct 15, 2024
1 parent 40b1c42 commit 7c3c13c
Show file tree
Hide file tree
Showing 7 changed files with 68 additions and 11 deletions.
16 changes: 12 additions & 4 deletions cli/http_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -470,15 +470,23 @@ impl HttpClient {
}
}

pub async fn download_with_progress(
pub async fn download_with_progress_and_retries(
&self,
url: Url,
maybe_header: Option<(HeaderName, HeaderValue)>,
progress_guard: &UpdateGuard,
) -> Result<Option<Vec<u8>>, DownloadError> {
self
.download_inner(url, maybe_header, Some(progress_guard))
.await
crate::util::retry::retry(
|| {
self.download_inner(
url.clone(),
maybe_header.clone(),
Some(progress_guard),
)
},
|e| matches!(e, DownloadError::BadResponse(_) | DownloadError::Fetch(_)),
)
.await
}

pub async fn get_redirected_url(
Expand Down
11 changes: 7 additions & 4 deletions cli/npm/managed/cache/registry_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,10 +202,13 @@ impl RegistryInfoDownloader {
let guard = self.progress_bar.update(package_url.as_str());
let name = name.to_string();
async move {
let maybe_bytes = downloader
.http_client_provider
.get_or_create()?
.download_with_progress(package_url, maybe_auth_header, &guard)
let client = downloader.http_client_provider.get_or_create()?;
let maybe_bytes = client
.download_with_progress_and_retries(
package_url,
maybe_auth_header,
&guard,
)
.await?;
match maybe_bytes {
Some(bytes) => {
Expand Down
2 changes: 1 addition & 1 deletion cli/npm/managed/cache/tarball.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ impl TarballCache {
let guard = tarball_cache.progress_bar.update(&dist.tarball);
let result = tarball_cache.http_client_provider
.get_or_create()?
.download_with_progress(tarball_uri, maybe_auth_header, &guard)
.download_with_progress_and_retries(tarball_uri, maybe_auth_header, &guard)
.await;
let maybe_bytes = match result {
Ok(maybe_bytes) => maybe_bytes,
Expand Down
6 changes: 5 additions & 1 deletion cli/standalone/binary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,11 @@ impl<'a> DenoCompileBinaryWriter<'a> {
self
.http_client_provider
.get_or_create()?
.download_with_progress(download_url.parse()?, None, &progress)
.download_with_progress_and_retries(
download_url.parse()?,
None,
&progress,
)
.await?
};
let bytes = match maybe_bytes {
Expand Down
2 changes: 1 addition & 1 deletion cli/tools/upgrade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -913,7 +913,7 @@ async fn download_package(
// text above which will stay alive after the progress bars are complete
let progress = progress_bar.update("");
let maybe_bytes = client
.download_with_progress(download_url.clone(), None, &progress)
.download_with_progress_and_retries(download_url.clone(), None, &progress)
.await
.with_context(|| format!("Failed downloading {download_url}. The version you requested may not have been built for the current architecture."))?;
Ok(maybe_bytes)
Expand Down
1 change: 1 addition & 0 deletions cli/util/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ pub mod logger;
pub mod path;
pub mod progress_bar;
pub mod result;
pub mod retry;
pub mod sync;
pub mod text_encoding;
pub mod unix;
Expand Down
41 changes: 41 additions & 0 deletions cli/util/retry.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.

use std::future::Future;
use std::time::Duration;

pub fn retry<
F: FnMut() -> Fut,
T,
E,
Fut: Future<Output = Result<T, E>>,
ShouldRetry: FnMut(&E) -> bool,
>(
mut f: F,
mut should_retry: ShouldRetry,
) -> impl Future<Output = Result<T, E>> {
const WAITS: [Duration; 3] = [
Duration::from_millis(100),
Duration::from_millis(250),
Duration::from_millis(500),
];

let mut waits = WAITS.into_iter();
async move {
let mut first_result = None;
loop {
let result = f().await;
match result {
Ok(r) => return Ok(r),
Err(e) if !should_retry(&e) => return Err(e),
_ => {}
}
if first_result.is_none() {
first_result = Some(result);
}
let Some(wait) = waits.next() else {
return first_result.unwrap();
};
tokio::time::sleep(wait).await;
}
}
}

0 comments on commit 7c3c13c

Please sign in to comment.