Auto merge of #6936 - sgrif:sg-timeout-error, r=alexcrichton

Give a better error message when crates.io requests time out

crates.io is hosted on Heroku, which means we have a hard 30 second time
limit on all requests. Typically this is only hit when someone is
attempting to upload a crate so large that it would have been eventually
rejected anyway, but it can also happen if a user is on a very slow
internet connection.

When this happens, the request is terminated by the platform and we have
no control over the response that gets sent. This results in the user
getting a very unhelpful error message from Cargo, and some generic
error page HTML spat out into their terminal. We could work around this
on our end by adding a 29 second timeout *somewhere else* in the stack,
but we have a lot of layers that buffer requests to protect against slow
client attacks, and it'd be a pretty decent amount of work. Since we
eventually want to switch over to having Cargo do the S3 upload instead
of us, it doesn't make sense to spend so much time on an error scenario
that eventually will go away.

I've tried to keep this uncoupled from crates.io as much as possible,
since alternate registries might not be hosted on Heroku or have the
same restricitions. But I figure "a 503 that took more than 30 seconds"
is a safe bet on this being hit. If we're ok with coupling this to
crates.io, I'd like to include "If your crate is less than 10MB you can
email help@crates.io for assistance" in the error message.

Ref rust-lang/crates.io#1709

Author: bors

src/cargo/ops/registry.rs

@@ -129,12 +129,7 @@ fn verify_dependencies(
                 // This extra hostname check is mostly to assist with testing,
                 // but also prevents someone using `--index` to specify
                 // something that points to crates.io.
-                let is_crates_io = registry
-                    .host()
-                    .to_url()
-                    .map(|u| u.host_str() == Some("crates.io"))
-                    .unwrap_or(false);
-                if registry_src.is_default_registry() || is_crates_io {
+                if registry_src.is_default_registry() || registry.host_is_crates_io() {
                     bail!("crates cannot be published to crates.io with dependencies sourced from other\n\
                            registries either publish `{}` on crates.io or pull it into this repository\n\
                            and specify it with a path and version\n\

src/crates-io/lib.rs

@@ -5,13 +5,15 @@ use std::collections::BTreeMap;
 use std::fs::File;
 use std::io::prelude::*;
 use std::io::Cursor;
+use std::time::Instant;
 
 use curl::easy::{Easy, List};
 use failure::bail;
 use http::status::StatusCode;
 use serde::{Deserialize, Serialize};
 use serde_json;
 use url::percent_encoding::{percent_encode, QUERY_ENCODE_SET};
+use url::Url;
 
 pub type Result<T> = std::result::Result<T, failure::Error>;
 
@@ -141,6 +143,12 @@ impl Registry {
         &self.host
     }
 
+    pub fn host_is_crates_io(&self) -> bool {
+        Url::parse(self.host())
+            .map(|u| u.host_str() == Some("crates.io"))
+            .unwrap_or(false)
+    }
+
     pub fn add_owners(&mut self, krate: &str, owners: &[&str]) -> Result<String> {
         let body = serde_json::to_string(&OwnersReq { users: owners })?;
         let body = self.put(&format!("/crates/{}/owners", krate), body.as_bytes())?;
@@ -207,7 +215,7 @@ impl Registry {
         headers.append(&format!("Authorization: {}", token))?;
         self.handle.http_headers(headers)?;
 
-        let body = handle(&mut self.handle, &mut |buf| body.read(buf).unwrap_or(0))?;
+        let body = self.handle(&mut |buf| body.read(buf).unwrap_or(0))?;
 
         let response = if body.is_empty() {
             "{}".parse()?
@@ -300,55 +308,62 @@ impl Registry {
             Some(mut body) => {
                 self.handle.upload(true)?;
                 self.handle.in_filesize(body.len() as u64)?;
-                handle(&mut self.handle, &mut |buf| body.read(buf).unwrap_or(0))
+                self.handle(&mut |buf| body.read(buf).unwrap_or(0))
             }
-            None => handle(&mut self.handle, &mut |_| 0),
+            None => self.handle(&mut |_| 0),
         }
     }
-}
 
-fn handle(handle: &mut Easy, read: &mut dyn FnMut(&mut [u8]) -> usize) -> Result<String> {
-    let mut headers = Vec::new();
-    let mut body = Vec::new();
-    {
-        let mut handle = handle.transfer();
-        handle.read_function(|buf| Ok(read(buf)))?;
-        handle.write_function(|data| {
-            body.extend_from_slice(data);
-            Ok(data.len())
-        })?;
-        handle.header_function(|data| {
-            headers.push(String::from_utf8_lossy(data).into_owned());
-            true
-        })?;
-        handle.perform()?;
-    }
+    fn handle(&mut self, read: &mut dyn FnMut(&mut [u8]) -> usize) -> Result<String> {
+        let mut headers = Vec::new();
+        let mut body = Vec::new();
+        let started;
+        {
+            let mut handle = self.handle.transfer();
+            handle.read_function(|buf| Ok(read(buf)))?;
+            handle.write_function(|data| {
+                body.extend_from_slice(data);
+                Ok(data.len())
+            })?;
+            handle.header_function(|data| {
+                headers.push(String::from_utf8_lossy(data).into_owned());
+                true
+            })?;
+            started = Instant::now();
+            handle.perform()?;
+        }
 
-    let body = match String::from_utf8(body) {
-        Ok(body) => body,
-        Err(..) => bail!("response body was not valid utf-8"),
-    };
-    let errors = serde_json::from_str::<ApiErrorList>(&body).ok().map(|s| {
-        s.errors.into_iter().map(|s| s.detail).collect::<Vec<_>>()
-    });
-
-    match (handle.response_code()?, errors) {
-        (0, None) | (200, None) => {},
-        (code, Some(errors)) => {
-            let code = StatusCode::from_u16(code as _)?;
-            bail!("api errors (status {}): {}", code, errors.join(", "))
+        let body = match String::from_utf8(body) {
+            Ok(body) => body,
+            Err(..) => bail!("response body was not valid utf-8"),
+        };
+        let errors = serde_json::from_str::<ApiErrorList>(&body).ok().map(|s| {
+            s.errors.into_iter().map(|s| s.detail).collect::<Vec<_>>()
+        });
+
+        match (self.handle.response_code()?, errors) {
+            (0, None) | (200, None) => {},
+            (503, None) if started.elapsed().as_secs() >= 29 && self.host_is_crates_io() => {
+                bail!("Request timed out after 30 seconds. If you're trying to \
+                       upload a crate it may be too large. If the crate is under \
+                       10MB in size, you can email help@crates.io for assistance.")
+            }
+            (code, Some(errors)) => {
+                let code = StatusCode::from_u16(code as _)?;
+                bail!("api errors (status {}): {}", code, errors.join(", "))
+            }
+            (code, None) => bail!(
+                "failed to get a 200 OK response, got {}\n\
+                 headers:\n\
+                 \t{}\n\
+                 body:\n\
+                 {}",
+                 code,
+                 headers.join("\n\t"),
+                 body,
+            ),
         }
-        (code, None) => bail!(
-            "failed to get a 200 OK response, got {}\n\
-             headers:\n\
-             \t{}\n\
-             body:\n\
-             {}",
-             code,
-             headers.join("\n\t"),
-             body,
-        ),
-    }
 
-    Ok(body)
+        Ok(body)
+    }
 }