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

Automatically retry HTTP requests returning 5xx #4032

Merged
merged 1 commit into from
May 11, 2017
Merged
Changes from all commits
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
Automatically retry HTTP requests returning 5xx
This commit implements auto-retry for downloading crates from crates.io whenever
a 5xx response is returned. This should help assist with automatic retries
whenever Cargo attempts to download directly from S3 but S3 returns a 500 error,
which is defined as "please retry again".

This logic may be a little eager to retry *all* 500 errors, but there's a
maximum cap on all retries regardless, so hopefully it doesn't result in too
many problems.

Closes #3962
alexcrichton committed May 11, 2017
commit 6155653653acd073c9888b2da73b9661d52e55aa
31 changes: 19 additions & 12 deletions src/cargo/sources/registry/remote.rs
Original file line number Diff line number Diff line change
@@ -16,6 +16,7 @@ use util::network;
use util::paths;
use util::{FileLock, Filesystem};
use util::{Config, CargoResult, ChainError, human, Sha256, ToUrl};
use util::errors::HttpError;

pub struct RemoteRegistry<'cfg> {
index_path: Filesystem,
@@ -153,26 +154,32 @@ impl<'cfg> RegistryData for RemoteRegistry<'cfg> {
// TODO: don't download into memory, but ensure that if we ctrl-c a
// download we should resume either from the start or the middle
// on the next time
let url = url.to_string();
handle.get(true)?;
handle.url(&url.to_string())?;
handle.url(&url)?;
handle.follow_location(true)?;
let mut state = Sha256::new();
let mut body = Vec::new();
network::with_retry(self.config, || {
state = Sha256::new();
body = Vec::new();
let mut handle = handle.transfer();
handle.write_function(|buf| {
state.update(buf);
body.extend_from_slice(buf);
Ok(buf.len())
})?;
handle.perform()
{
let mut handle = handle.transfer();
handle.write_function(|buf| {
state.update(buf);
body.extend_from_slice(buf);
Ok(buf.len())
})?;
handle.perform()?;
}
let code = handle.response_code()?;
if code != 200 && code != 0 {
let url = handle.effective_url()?.unwrap_or(&url);
Err(HttpError::Not200(code, url.to_string()))
} else {
Ok(())
}
})?;
let code = handle.response_code()?;
if code != 200 && code != 0 {
bail!("failed to get 200 response from `{}`, got {}", url, code)
}

// Verify what we just downloaded
if state.finish().to_hex() != checksum {
58 changes: 58 additions & 0 deletions src/cargo/util/errors.rs
Original file line number Diff line number Diff line change
@@ -334,6 +334,7 @@ impl NetworkError for git2::Error {
}
}
}

impl NetworkError for curl::Error {
fn maybe_spurious(&self) -> bool {
self.is_couldnt_connect() ||
@@ -344,6 +345,63 @@ impl NetworkError for curl::Error {
}
}

#[derive(Debug)]
pub enum HttpError {
Not200(u32, String),
Curl(curl::Error),
}

impl fmt::Display for HttpError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match *self {
HttpError::Not200(code, ref url) => {
write!(f, "failed to get 200 response from `{}`, got {}",
url, code)
}
HttpError::Curl(ref e) => e.fmt(f),
}
}
}

impl Error for HttpError {
fn description(&self) -> &str {
match *self {
HttpError::Not200(..) => "failed to get a 200 response",
HttpError::Curl(ref e) => e.description(),
}
}

fn cause(&self) -> Option<&Error> {
match *self {
HttpError::Not200(..) => None,
HttpError::Curl(ref e) => e.cause(),
}
}
}

impl CargoError for HttpError {
fn is_human(&self) -> bool {
true
}
}

impl NetworkError for HttpError {
fn maybe_spurious(&self) -> bool {
match *self {
HttpError::Not200(code, ref _url) => {
500 <= code && code < 600
}
HttpError::Curl(ref e) => e.maybe_spurious(),
}
}
}

impl From<curl::Error> for HttpError {
fn from(err: curl::Error) -> HttpError {
HttpError::Curl(err)
}
}

// =============================================================================
// various impls