From 4746289871538a4db7680c187372d6dda93525fd Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Thu, 21 Mar 2019 12:10:43 -0600 Subject: [PATCH 1/4] Always nicely show errors from crates.io if possible Currently if Cargo ever gets a non-200 response, it will either not show the error at all (if it was a 403 or 404), or spit out the entire response body. Historically crates.io has served a 200 for most errors to work around this, but we've stopped doing this as it causes problems for other clients. Additionally, we're starting to server more errors that have semantic meaning (429 for rate limiting, 503 when we're in read only mode). If the request specifies "Accept: application/json", we should ideally return the errors formatted nicely. This isn't always true, but it's what we'd like to do going forward. While the output that Cargo puts out at least contains the actual message, it's buried under a ton of useless info. This changes the behavior so that if the response was valid JSON in the format that Cargo expects, it just shows that (along with a description of the response status), and only falls back to spitting out everything if it can't parse the response body. I'd love to add some more tests for this, but I've had trouble finding anywhere in the test suite that exercises these paths. --- src/crates-io/Cargo.toml | 1 + src/crates-io/lib.rs | 36 +++++++++++++++++++----------------- 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/src/crates-io/Cargo.toml b/src/crates-io/Cargo.toml index dddac0d5cc1..042333347e0 100644 --- a/src/crates-io/Cargo.toml +++ b/src/crates-io/Cargo.toml @@ -16,6 +16,7 @@ path = "lib.rs" [dependencies] curl = "0.4" failure = "0.1.1" +http = "0.1" serde = { version = "1.0", features = ['derive'] } serde_derive = "1.0" serde_json = "1.0" diff --git a/src/crates-io/lib.rs b/src/crates-io/lib.rs index 342442519a5..1b4d860d692 100644 --- a/src/crates-io/lib.rs +++ b/src/crates-io/lib.rs @@ -8,6 +8,7 @@ use std::io::Cursor; 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}; @@ -323,30 +324,31 @@ fn handle(handle: &mut Easy, read: &mut dyn FnMut(&mut [u8]) -> usize) -> Result handle.perform()?; } - match handle.response_code()? { - 0 => {} // file upload url sometimes - 200 => {} - 403 => bail!("received 403 unauthorized response code"), - 404 => bail!("received 404 not found response code"), - code => bail!( + 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::(&body).ok().map(|s| { + s.errors.into_iter().map(|s| s.detail).collect::>() + }); + + 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(", ")) + } + (code, None) => bail!( "failed to get a 200 OK response, got {}\n\ headers:\n\ \t{}\n\ body:\n\ {}", - code, - headers.join("\n\t"), - String::from_utf8_lossy(&body) + code, + headers.join("\n\t"), + body, ), } - let body = match String::from_utf8(body) { - Ok(body) => body, - Err(..) => bail!("response body was not valid utf-8"), - }; - if let Ok(errors) = serde_json::from_str::(&body) { - let errors = errors.errors.into_iter().map(|s| s.detail); - bail!("api errors: {}", errors.collect::>().join(", ")); - } Ok(body) } From 1b1abcf8f980b0a591d56395e873a73edfc155ad Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Sun, 31 Mar 2019 19:37:26 -0700 Subject: [PATCH 2/4] Fix unused import warning. --- tests/testsuite/death.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/testsuite/death.rs b/tests/testsuite/death.rs index 98316860b3e..2afddf64ae1 100644 --- a/tests/testsuite/death.rs +++ b/tests/testsuite/death.rs @@ -131,8 +131,6 @@ fn ctrl_c_kills_everyone() { #[cfg(unix)] fn ctrl_c(child: &mut Child) { - use libc; - let r = unsafe { libc::kill(-(child.id() as i32), libc::SIGINT) }; if r < 0 { panic!("failed to kill: {}", io::Error::last_os_error()); From f720855ab76ff78c7d34ed3d8c7d3046b12960a1 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Mon, 1 Apr 2019 15:21:51 -0600 Subject: [PATCH 3/4] Delete the minimal version tests --- .travis.yml | 14 -------------- appveyor.yml | 3 --- 2 files changed, 17 deletions(-) diff --git a/.travis.yml b/.travis.yml index 969a16cceb3..ba205f0d9fa 100644 --- a/.travis.yml +++ b/.travis.yml @@ -23,20 +23,6 @@ matrix: rust: beta if: branch != master OR type = pull_request - # Minimum Rust supported channel. We enable these to make sure we - # continue to work on the advertised minimum Rust version. - # However cargo only supports the latest stable so this will get - # increased every 6 weeks or so when the first PR to use a new feature. - - env: TARGET=x86_64-unknown-linux-gnu - ALT=i686-unknown-linux-gnu - rust: 1.31.0 - script: - - rustup toolchain install nightly || travis_terminate 1 - - cargo +nightly generate-lockfile -Z minimal-versions || travis_terminate 1 - - cargo -V || travis_terminate 1 - - cargo test --features=deny-warnings || travis_terminate 1 - if: branch != master OR type = pull_request - - env: TARGET=x86_64-unknown-linux-gnu ALT=i686-unknown-linux-gnu rust: nightly diff --git a/appveyor.yml b/appveyor.yml index e05d4791672..5d2ec309b8a 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -2,9 +2,6 @@ environment: matrix: - TARGET: x86_64-pc-windows-msvc OTHER_TARGET: i686-pc-windows-msvc - - TARGET: x86_64-pc-windows-msvc - MINIMAL_VERSIONS: true - CFG_DISABLE_CROSS_TESTS: 1 install: - if NOT defined APPVEYOR_PULL_REQUEST_NUMBER if "%APPVEYOR_REPO_BRANCH%" == "master" appveyor exit From 067c199b5733ddc12f549c52bfd6a7390b4a3396 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Mon, 1 Apr 2019 15:26:00 -0600 Subject: [PATCH 4/4] Remove unused imports --- src/cargo/util/errors.rs | 1 - src/cargo/util/paths.rs | 1 - 2 files changed, 2 deletions(-) diff --git a/src/cargo/util/errors.rs b/src/cargo/util/errors.rs index fb50157ff50..a3f071c9fe5 100644 --- a/src/cargo/util/errors.rs +++ b/src/cargo/util/errors.rs @@ -307,7 +307,6 @@ pub fn process_error( #[cfg(unix)] fn status_to_string(status: ExitStatus) -> String { - use libc; use std::os::unix::process::*; if let Some(signal) = status.signal() { diff --git a/src/cargo/util/paths.rs b/src/cargo/util/paths.rs index 961ded48c97..00ee3f5ce1b 100644 --- a/src/cargo/util/paths.rs +++ b/src/cargo/util/paths.rs @@ -217,7 +217,6 @@ pub fn path2bytes(path: &Path) -> CargoResult<&[u8]> { #[cfg(unix)] pub fn bytes2path(bytes: &[u8]) -> CargoResult { - use std::ffi::OsStr; use std::os::unix::prelude::*; Ok(PathBuf::from(OsStr::from_bytes(bytes))) }