Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add delays to network retries. #11881

Merged
merged 6 commits into from
Apr 1, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Split the cargo::util::network module into submodules
This is intended to help grow with more stuff.
  • Loading branch information
ehuss committed Mar 31, 2023
commit b5d772f303588ce8057e80f8f3958c098ebf4bcc
2 changes: 1 addition & 1 deletion src/cargo/core/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use crate::ops;
use crate::util::config::PackageCacheLock;
use crate::util::errors::{CargoResult, HttpNotSuccessful};
use crate::util::interning::InternedString;
use crate::util::network::Retry;
use crate::util::network::retry::Retry;
use crate::util::{self, internal, Config, Progress, ProgressStyle};

pub const MANIFEST_PREAMBLE: &str = "\
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/sources/git/oxide.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ pub fn with_retry_and_progress(
) -> CargoResult<()> {
std::thread::scope(|s| {
let mut progress_bar = Progress::new("Fetch", config);
network::with_retry(config, || {
network::retry::with_retry(config, || {
let progress_root: Arc<gix::progress::tree::Root> =
gix::progress::tree::root::Options {
initial_capacity: 10,
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/sources/git/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -739,7 +739,7 @@ pub fn with_fetch_options(
let ssh_config = config.net_config()?.ssh.as_ref();
let config_known_hosts = ssh_config.and_then(|ssh| ssh.known_hosts.as_ref());
let diagnostic_home_config = config.diagnostic_home_config();
network::with_retry(config, || {
network::retry::with_retry(config, || {
with_authentication(config, url, git_config, |f| {
let port = Url::parse(url).ok().and_then(|url| url.port());
let mut last_update = Instant::now();
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/sources/registry/http_remote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::sources::registry::download;
use crate::sources::registry::MaybeLock;
use crate::sources::registry::{LoadResponse, RegistryConfig, RegistryData};
use crate::util::errors::{CargoResult, HttpNotSuccessful};
use crate::util::network::Retry;
use crate::util::network::retry::Retry;
use crate::util::{auth, Config, Filesystem, IntoUrl, Progress, ProgressStyle};
use anyhow::Context;
use cargo_util::paths;
Expand Down
37 changes: 37 additions & 0 deletions src/cargo/util/network/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
//! Utilities for networking.

use std::task::Poll;

pub mod retry;

pub trait PollExt<T> {
fn expect(self, msg: &str) -> T;
}

impl<T> PollExt<T> for Poll<T> {
#[track_caller]
fn expect(self, msg: &str) -> T {
match self {
Poll::Ready(val) => val,
Poll::Pending => panic!("{}", msg),
}
}
}

// When dynamically linked against libcurl, we want to ignore some failures
// when using old versions that don't support certain features.
#[macro_export]
macro_rules! try_old_curl {
($e:expr, $msg:expr) => {
let result = $e;
if cfg!(target_os = "macos") {
if let Err(e) = result {
warn!("ignoring libcurl {} error: {}", $msg, e);
}
} else {
result.with_context(|| {
anyhow::format_err!("failed to enable {}, is curl not built right?", $msg)
})?;
}
};
}
37 changes: 3 additions & 34 deletions src/cargo/util/network.rs → src/cargo/util/network/retry.rs
Original file line number Diff line number Diff line change
@@ -1,22 +1,9 @@
//! Utilities for retrying a network operation.

use anyhow::Error;

use crate::util::errors::{CargoResult, HttpNotSuccessful};
use crate::util::Config;
use std::task::Poll;

pub trait PollExt<T> {
fn expect(self, msg: &str) -> T;
}

impl<T> PollExt<T> for Poll<T> {
#[track_caller]
fn expect(self, msg: &str) -> T {
match self {
Poll::Ready(val) => val,
Poll::Pending => panic!("{}", msg),
}
}
}

pub struct Retry<'a> {
config: &'a Config,
Expand Down Expand Up @@ -105,7 +92,7 @@ fn maybe_spurious(err: &Error) -> bool {
/// # let download_something = || return Ok(());
/// # let config = Config::default().unwrap();
/// use cargo::util::network;
/// let cargo_result = network::with_retry(&config, || download_something());
/// let cargo_result = network::retry::with_retry(&config, || download_something());
/// ```
pub fn with_retry<T, F>(config: &Config, mut callback: F) -> CargoResult<T>
where
Expand All @@ -119,24 +106,6 @@ where
}
}

// When dynamically linked against libcurl, we want to ignore some failures
// when using old versions that don't support certain features.
#[macro_export]
macro_rules! try_old_curl {
($e:expr, $msg:expr) => {
let result = $e;
if cfg!(target_os = "macos") {
if let Err(e) = result {
warn!("ignoring libcurl {} error: {}", $msg, e);
}
} else {
result.with_context(|| {
anyhow::format_err!("failed to enable {}, is curl not built right?", $msg)
})?;
}
};
}

#[test]
fn with_retry_repeats_the_call_then_works() {
use crate::core::Shell;
Expand Down