Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[beta] Backport #6771 #6808

Merged
merged 4 commits into from
Apr 1, 2019
Merged

Conversation

sgrif
Copy link
Contributor

@sgrif sgrif commented Apr 1, 2019

No description provided.

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.
@rust-highfive
Copy link

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive
Copy link

⚠️ Warning ⚠️

  • Pull requests are usually filed against the master branch for this repo, but this one is against rust-1.34.0. Please double check that you specified the right target!

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 1, 2019
@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Apr 1, 2019

📌 Commit 4746289 has been approved by alexcrichton

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 1, 2019
@alexcrichton alexcrichton changed the title Backport #6771 to beta [beta] Backport #6771 Apr 1, 2019
@bors
Copy link
Contributor

bors commented Apr 1, 2019

⌛ Testing commit 4746289 with merge d3c0198a3e3386d96c19d7f887eff5b2e02c0c66...

@ehuss
Copy link
Contributor

ehuss commented Apr 1, 2019

I think this will fail for multiple reasons. @sgrif can you cherry-pick some other PRs to get the 1.34 branch working? I think it will need #6807. It looks like there is another unused item in src/cargo/util/paths.rs (build with the 2019-04-01 nightly, with build --features=deny-warnings --tests).

Also, it looks like the minimal versions CI entries are failing. Just delete the entire entry from .travis.yml and appveyor.yml.

Let me know if you want me to do it or have any questions.

@sgrif
Copy link
Contributor Author

sgrif commented Apr 1, 2019

Cherry picked #6807 and deleted the minimal version entries from travis and appveyor

@sgrif
Copy link
Contributor Author

sgrif commented Apr 1, 2019

Fixed the errors from cargo +nightly-2019-04-01 build --features=deny-warnings --tests

@ehuss
Copy link
Contributor

ehuss commented Apr 1, 2019

Thanks! Sorry, the beta branches always seem to have problems since they are so rarely built.

@bors r=alexcrichton

@bors
Copy link
Contributor

bors commented Apr 1, 2019

📌 Commit 067c199 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Apr 1, 2019

⌛ Testing commit 067c199 with merge 6789d8a...

bors added a commit that referenced this pull request Apr 1, 2019
@sgrif
Copy link
Contributor Author

sgrif commented Apr 1, 2019

No worries. This is mostly on me for letting this sit for so long.

@bors
Copy link
Contributor

bors commented Apr 1, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: alexcrichton
Pushing 6789d8a to rust-1.34.0...

@bors bors merged commit 067c199 into rust-lang:rust-1.34.0 Apr 1, 2019
@ehuss ehuss added this to the 1.34.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants