Skip to content

Commit

Permalink
Auto merge of #11069 - arlosi:retry, r=Eh2406
Browse files Browse the repository at this point in the history
Add retry support to sparse registries

Sparse (HTTP) registries currently do not respect Cargo's retry policy for http requests.

This change makes sparse registries use the same retry system as package downloads.
  • Loading branch information
bors committed Oct 7, 2022
2 parents 3cdf1ab + dcc512b commit d1a2592
Show file tree
Hide file tree
Showing 10 changed files with 196 additions and 103 deletions.
36 changes: 24 additions & 12 deletions crates/cargo-test-support/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ pub struct RegistryBuilder {
/// Write the registry in configuration.
configure_registry: bool,
/// API responders.
custom_responders: HashMap<&'static str, Box<dyn Send + Fn(&Request) -> Response>>,
custom_responders: HashMap<&'static str, Box<dyn Send + Fn(&Request, &HttpServer) -> Response>>,
}

pub struct TestRegistry {
Expand Down Expand Up @@ -117,7 +117,7 @@ impl RegistryBuilder {

/// Adds a custom HTTP response for a specific url
#[must_use]
pub fn add_responder<R: 'static + Send + Fn(&Request) -> Response>(
pub fn add_responder<R: 'static + Send + Fn(&Request, &HttpServer) -> Response>(
mut self,
url: &'static str,
responder: R,
Expand Down Expand Up @@ -497,20 +497,23 @@ pub struct Response {
pub body: Vec<u8>,
}

struct HttpServer {
pub struct HttpServer {
listener: TcpListener,
registry_path: PathBuf,
dl_path: PathBuf,
token: Option<String>,
custom_responders: HashMap<&'static str, Box<dyn Send + Fn(&Request) -> Response>>,
custom_responders: HashMap<&'static str, Box<dyn Send + Fn(&Request, &HttpServer) -> Response>>,
}

impl HttpServer {
pub fn new(
registry_path: PathBuf,
dl_path: PathBuf,
token: Option<String>,
api_responders: HashMap<&'static str, Box<dyn Send + Fn(&Request) -> Response>>,
api_responders: HashMap<
&'static str,
Box<dyn Send + Fn(&Request, &HttpServer) -> Response>,
>,
) -> HttpServerHandle {
let listener = TcpListener::bind("127.0.0.1:0").unwrap();
let addr = listener.local_addr().unwrap();
Expand Down Expand Up @@ -620,7 +623,7 @@ impl HttpServer {

// Check for custom responder
if let Some(responder) = self.custom_responders.get(req.url.path()) {
return responder(&req);
return responder(&req, self);
}
let path: Vec<_> = req.url.path()[1..].split('/').collect();
match (req.method.as_str(), path.as_slice()) {
Expand Down Expand Up @@ -668,7 +671,7 @@ impl HttpServer {
}

/// Unauthorized response
fn unauthorized(&self, _req: &Request) -> Response {
pub fn unauthorized(&self, _req: &Request) -> Response {
Response {
code: 401,
headers: vec![],
Expand All @@ -677,7 +680,7 @@ impl HttpServer {
}

/// Not found response
fn not_found(&self, _req: &Request) -> Response {
pub fn not_found(&self, _req: &Request) -> Response {
Response {
code: 404,
headers: vec![],
Expand All @@ -686,16 +689,25 @@ impl HttpServer {
}

/// Respond OK without doing anything
fn ok(&self, _req: &Request) -> Response {
pub fn ok(&self, _req: &Request) -> Response {
Response {
code: 200,
headers: vec![],
body: br#"{"ok": true, "msg": "completed!"}"#.to_vec(),
}
}

/// Return an internal server error (HTTP 500)
pub fn internal_server_error(&self, _req: &Request) -> Response {
Response {
code: 500,
headers: vec![],
body: br#"internal server error"#.to_vec(),
}
}

/// Serve the download endpoint
fn dl(&self, req: &Request) -> Response {
pub fn dl(&self, req: &Request) -> Response {
let file = self
.dl_path
.join(req.url.path().strip_prefix("/dl/").unwrap());
Expand All @@ -711,7 +723,7 @@ impl HttpServer {
}

/// Serve the registry index
fn index(&self, req: &Request) -> Response {
pub fn index(&self, req: &Request) -> Response {
let file = self
.registry_path
.join(req.url.path().strip_prefix("/index/").unwrap());
Expand Down Expand Up @@ -761,7 +773,7 @@ impl HttpServer {
}
}

fn publish(&self, req: &Request) -> Response {
pub fn publish(&self, req: &Request) -> Response {
if let Some(body) = &req.body {
// Get the metadata of the package
let (len, remaining) = body.split_at(4);
Expand Down
9 changes: 5 additions & 4 deletions src/cargo/core/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use crate::core::{Dependency, Manifest, PackageId, SourceId, Target};
use crate::core::{SourceMap, Summary, Workspace};
use crate::ops;
use crate::util::config::PackageCacheLock;
use crate::util::errors::{CargoResult, HttpNot200};
use crate::util::errors::{CargoResult, HttpNotSuccessful};
use crate::util::interning::InternedString;
use crate::util::network::Retry;
use crate::util::{self, internal, Config, Progress, ProgressStyle};
Expand Down Expand Up @@ -868,18 +868,19 @@ impl<'a, 'cfg> Downloads<'a, 'cfg> {
let code = handle.response_code()?;
if code != 200 && code != 0 {
let url = handle.effective_url()?.unwrap_or(url);
return Err(HttpNot200 {
return Err(HttpNotSuccessful {
code,
url: url.to_string(),
body: data,
}
.into());
}
Ok(())
Ok(data)
})
.with_context(|| format!("failed to download from `{}`", dl.url))?
};
match ret {
Some(()) => break (dl, data),
Some(data) => break (dl, data),
None => {
self.pending_ids.insert(dl.id);
self.enqueue(dl, handle)?
Expand Down
14 changes: 1 addition & 13 deletions src/cargo/ops/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use crate::sources::{RegistrySource, SourceConfigMap, CRATES_IO_DOMAIN, CRATES_I
use crate::util::config::{self, Config, SslVersionConfig, SslVersionConfigRange};
use crate::util::errors::CargoResult;
use crate::util::important_paths::find_root_manifest_for_wd;
use crate::util::IntoUrl;
use crate::util::{truncate_with_ellipsis, IntoUrl};
use crate::{drop_print, drop_println, version};

mod auth;
Expand Down Expand Up @@ -963,18 +963,6 @@ pub fn search(
limit: u32,
reg: Option<String>,
) -> CargoResult<()> {
fn truncate_with_ellipsis(s: &str, max_width: usize) -> String {
// We should truncate at grapheme-boundary and compute character-widths,
// yet the dependencies on unicode-segmentation and unicode-width are
// not worth it.
let mut chars = s.chars();
let mut prefix = (&mut chars).take(max_width - 1).collect::<String>();
if chars.next().is_some() {
prefix.push('…');
}
prefix
}

let (mut registry, _, source_id) =
registry(config, None, index.as_deref(), reg.as_deref(), false, false)?;
let (crates, total_crates) = registry.search(query, limit).with_context(|| {
Expand Down

0 comments on commit d1a2592

Please sign in to comment.