Auto merge of #361 - pietroalbini:unify-http, r=pietroalbini

Move most http handling to utils::http

Author: bors

src/agent/api.rs

@@ -1,20 +1,14 @@
 use base64;
 use crates::{Crate, GitHubRepo};
 use experiments::Experiment;
-use http::header::AUTHORIZATION;
-use http::header::USER_AGENT;
-use http::StatusCode;
+use http::{header::AUTHORIZATION, Method, StatusCode};
 use prelude::*;
-use reqwest::{Client, Method, RequestBuilder};
+use reqwest::RequestBuilder;
 use results::TestResult;
 use serde::de::DeserializeOwned;
 use server::api_types::{AgentConfig, ApiResponse, CraterToken};
 use toolchain::Toolchain;
-
-lazy_static! {
-    static ref CRATER_USER_AGENT: String =
-        format!("crater-agent/{}", ::GIT_REVISION.unwrap_or("unknown"));
-}
+use utils::http;
 
 #[derive(Debug, Fail)]
 pub enum AgentApiError {
@@ -68,29 +62,25 @@ impl ResponseExt for ::reqwest::Response {
 const RETRY_AFTER: u64 = 5;
 
 pub struct AgentApi {
-    client: Client,
     url: String,
     token: String,
 }
 
 impl AgentApi {
     pub fn new(url: &str, token: &str) -> Self {
         AgentApi {
-            client: Client::new(),
             url: url.to_string(),
             token: token.to_string(),
         }
     }
 
     fn build_request(&self, method: Method, url: &str) -> RequestBuilder {
-        self.client
-            .request(method, &format!("{}/agent-api/{}", self.url, url))
-            .header(
-                AUTHORIZATION,
-                (CraterToken {
-                    token: self.token.clone(),
-                }).to_string(),
-            ).header(USER_AGENT, CRATER_USER_AGENT.clone())
+        http::prepare_sync(method, &format!("{}/agent-api/{}", self.url, url)).header(
+            AUTHORIZATION,
+            (CraterToken {
+                token: self.token.clone(),
+            }).to_string(),
+        )
     }
 
     fn retry<T, F: Fn(&Self) -> Fallible<T>>(&self, f: F) -> Fallible<T> {

src/crates/sources/github.rs

@@ -33,7 +33,7 @@ impl List for GitHubList {
     fn fetch(&self) -> Fallible<Vec<Crate>> {
         info!("loading cached GitHub list from {}", self.source);
 
-        let mut resp = ::utils::http::get(&self.source)
+        let mut resp = ::utils::http::get_sync(&self.source)
             .with_context(|_| format!("failed to fetch GitHub crates list from {}", self.source))?;
         let mut reader = ::csv::Reader::from_reader(&mut resp);
 

src/crates/sources/registry.rs

@@ -87,7 +87,8 @@ fn dl_registry(name: &str, vers: &str, dir: &Path) -> Fallible<()> {
     }
     info!("downloading crate {}-{} to {}", name, vers, dir.display());
     let url = format!("{0}/{1}/{1}-{2}.crate", CRATES_ROOT, name, vers);
-    let bin = ::utils::http::get(&url).with_context(|_| format!("unable to download {}", url))?;
+    let bin =
+        ::utils::http::get_sync(&url).with_context(|_| format!("unable to download {}", url))?;
 
     fs::create_dir_all(&dir)?;
 

src/server/github.rs

@@ -1,15 +1,11 @@
-use http::header::{AUTHORIZATION, USER_AGENT};
+use http::header::AUTHORIZATION;
 use http::Method;
 use http::StatusCode;
 use prelude::*;
-use reqwest::{Client, RequestBuilder};
+use reqwest::RequestBuilder;
 use server::tokens::Tokens;
 use std::collections::HashMap;
-
-lazy_static! {
-    static ref CRATER_USER_AGENT: String =
-        format!("crater/{}", ::GIT_REVISION.unwrap_or("unknown"));
-}
+use utils::http;
 
 #[derive(Debug, Fail)]
 pub enum GitHubError {
@@ -24,14 +20,12 @@ pub enum GitHubError {
 #[derive(Clone)]
 pub struct GitHubApi {
     token: String,
-    client: Client,
 }
 
 impl GitHubApi {
     pub fn new(tokens: &Tokens) -> Self {
         GitHubApi {
             token: tokens.bot.api_token.clone(),
-            client: Client::new(),
         }
     }
 
@@ -42,10 +36,7 @@ impl GitHubApi {
             url.to_string()
         };
 
-        self.client
-            .request(method, &url)
-            .header(AUTHORIZATION, format!("token {}", self.token))
-            .header(USER_AGENT, CRATER_USER_AGENT.clone())
+        http::prepare_sync(method, &url).header(AUTHORIZATION, format!("token {}", self.token))
     }
 
     pub fn username(&self) -> Fallible<String> {

src/tools/rustup.rs

@@ -48,7 +48,8 @@ impl InstallableTool for Rustup {
             ::HOST_TARGET,
             EXE_SUFFIX
         );
-        let mut resp = ::utils::http::get(&url).with_context(|_| "unable to download rustup")?;
+        let mut resp =
+            ::utils::http::get_sync(&url).with_context(|_| "unable to download rustup")?;
 
         let tempdir = tempdir()?;
         let installer = &tempdir.path().join(format!("rustup-init{}", EXE_SUFFIX));

src/utils/http.rs

@@ -1,5 +1,6 @@
+use http::{header::USER_AGENT, Method, StatusCode};
 use prelude::*;
-use reqwest::{Client, ClientBuilder, RedirectPolicy, Response, StatusCode};
+use reqwest::{Client, ClientBuilder, RedirectPolicy, RequestBuilder, Response};
 
 const MAX_REDIRECTS: usize = 4;
 
@@ -15,27 +16,36 @@ pub struct InvalidStatusCode {
 }
 
 lazy_static! {
-    static ref HTTP_CLIENT: Client = setup_client();
+    static ref HTTP_SYNC_CLIENT: Client = setup_sync_client();
+    static ref USER_AGENT_CONTENT: String = format!(
+        "crater/{} ({})",
+        ::GIT_REVISION.unwrap_or("unknown"),
+        ::CRATER_REPO_URL
+    );
 }
 
-fn setup_client() -> Client {
+fn setup_sync_client() -> Client {
     ClientBuilder::new()
         .redirect(RedirectPolicy::limited(MAX_REDIRECTS))
         .build()
         .unwrap()
 }
 
-pub(crate) fn get(url: &str) -> Fallible<Response> {
-    ::utils::try_hard(|| {
-        let resp = HTTP_CLIENT.get(url).send()?;
+pub(crate) fn prepare_sync(method: Method, url: &str) -> RequestBuilder {
+    HTTP_SYNC_CLIENT
+        .request(method, url)
+        .header(USER_AGENT, USER_AGENT_CONTENT.clone())
+}
+
+pub(crate) fn get_sync(url: &str) -> Fallible<Response> {
+    let resp = prepare_sync(Method::GET, url).send()?;
 
-        // Return an error if the response wasn't a 200 OK
-        match resp.status() {
-            StatusCode::OK => Ok(resp),
-            status => Err(InvalidStatusCode {
-                url: url.to_string(),
-                status,
-            }.into()),
-        }
-    })
+    // Return an error if the response wasn't a 200 OK
+    match resp.status() {
+        StatusCode::OK => Ok(resp),
+        status => Err(InvalidStatusCode {
+            url: url.to_string(),
+            status,
+        }.into()),
+    }
 }