Skip to content

Commit

Permalink
Give a better error message when crates.io requests time out
Browse files Browse the repository at this point in the history
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 [email protected] for assistance" in the error message.

Ref rust-lang/crates.io#1709
  • Loading branch information
sgrif committed May 13, 2019
1 parent d56af31 commit 35cb079
Showing 1 changed file with 7 additions and 0 deletions.
7 changes: 7 additions & 0 deletions src/crates-io/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ 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;
Expand Down Expand Up @@ -310,6 +311,7 @@ impl Registry {
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 started;
{
let mut handle = handle.transfer();
handle.read_function(|buf| Ok(read(buf)))?;
Expand All @@ -321,6 +323,7 @@ fn handle(handle: &mut Easy, read: &mut dyn FnMut(&mut [u8]) -> usize) -> Result
headers.push(String::from_utf8_lossy(data).into_owned());
true
})?;
started = Instant::now();
handle.perform()?;
}

Expand All @@ -334,6 +337,10 @@ fn handle(handle: &mut Easy, read: &mut dyn FnMut(&mut [u8]) -> usize) -> Result

match (handle.response_code()?, errors) {
(0, None) | (200, None) => {},
(503, _) if started.elapsed().as_secs() >= 29 => {
bail!("Request took more than 30 seconds to complete. If you're\
trying to upload a crate it may be too large")
}
(code, Some(errors)) => {
let code = StatusCode::from_u16(code as _)?;
bail!("api errors (status {}): {}", code, errors.join(", "))
Expand Down

0 comments on commit 35cb079

Please sign in to comment.