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

Automatically retry HTTP requests returning 5xx #4032

Merged
merged 1 commit into from
May 11, 2017

Conversation

alexcrichton
Copy link
Member

This commit implements auto-retry for downloading crates from crates.io whenever
a 5xx response is returned. This should help assist with automatic retries
whenever Cargo attempts to download directly from S3 but S3 returns a 500 error,
which is defined as "please retry again".

This logic may be a little eager to retry all 500 errors, but there's a
maximum cap on all retries regardless, so hopefully it doesn't result in too
many problems.

Closes #3962

This commit implements auto-retry for downloading crates from crates.io whenever
a 5xx response is returned. This should help assist with automatic retries
whenever Cargo attempts to download directly from S3 but S3 returns a 500 error,
which is defined as "please retry again".

This logic may be a little eager to retry *all* 500 errors, but there's a
maximum cap on all retries regardless, so hopefully it doesn't result in too
many problems.

Closes rust-lang#3962
@alexcrichton
Copy link
Member Author

r? @matklad

@rust-highfive rust-highfive assigned matklad and unassigned brson May 11, 2017
@rust-highfive
Copy link

r? @brson

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

@matklad
Copy link
Member

matklad commented May 11, 2017

Could we test it somehow?

@alexcrichton
Copy link
Member Author

I wish! Do you have thoughts on how do to do? Unfortunately I don't :(

@matklad
Copy link
Member

matklad commented May 11, 2017

Yep, nothing simple comes to mind :( We could roll mini dependency injection to substitute curl::Easy in tests with a stub, but that's too much work and no guarantee that it'll work with real S3.

@bors r+

@bors
Copy link
Contributor

bors commented May 11, 2017

📌 Commit 6155653 has been approved by matklad

@bors
Copy link
Contributor

bors commented May 11, 2017

⌛ Testing commit 6155653 with merge c00e56d...

bors added a commit that referenced this pull request May 11, 2017
Automatically retry HTTP requests returning 5xx

This commit implements auto-retry for downloading crates from crates.io whenever
a 5xx response is returned. This should help assist with automatic retries
whenever Cargo attempts to download directly from S3 but S3 returns a 500 error,
which is defined as "please retry again".

This logic may be a little eager to retry *all* 500 errors, but there's a
maximum cap on all retries regardless, so hopefully it doesn't result in too
many problems.

Closes #3962
@bors
Copy link
Contributor

bors commented May 11, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: matklad
Pushing c00e56d to master...

@bors bors merged commit 6155653 into rust-lang:master May 11, 2017
@alexcrichton alexcrichton added the relnotes Release-note worthy label May 12, 2017
@alexcrichton alexcrichton deleted the retry-500 branch May 12, 2017 16:50
@ishitatsuyuki
Copy link
Contributor

ishitatsuyuki commented May 15, 2017

There are reports that this didn't work as expected. Maybe we need to have a test.
rust-lang/rust#41996 (comment)

@ehuss ehuss added this to the 1.19.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
relnotes Release-note worthy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Retry if S3 returns 500
7 participants