Retry if S3 returns 500

[carols10cents]

Crates.io redirects to S3 for /download requests. Sometimes S3 returns a 500, and cargo should automatically retry.

Cargo has network retry logic, but right now it only retries if the error is "spurious".

We should tweak maybe_spurious to count 500 as something that should be retried, not sure which one of the curl errors corresponds to that or if we should just check the status code like this check does.

[alexcrichton]

Currently the way curl operates a 500 isn't actually an error, but rather the perform function returns Ok(()), and then later when we extract the status code from the handle it says that it 500'd. Sort of like how if you .wait() on a process it'll return Ok even though the exit status may indicate failure.

I think the best way to solve this will be to first create a new error type:

enum CargoCurlError {
    Io(curl::Error),
    Code(u32),
}

Next implement the CargoError trait for this type as well as the network error trait. The closure here would then be updated to return this new error, moving the 500 checking logic into the closure. The implementation of the network error trait would then return "true" for "maybe_spurious" when the code is 500.

I'm gonna tag this as E-easy as I think it's a good starter bug, but if help is needed please just let me know!

[0X1A]

@alexcrichton I'm willing to offer a PR for this

[alexcrichton]

Awesome thanks @0X1A! Lemme know if you have any questions

[nagisa]

500, 502, 503, 504 status codes are all “spurious”.

[aturon]

@0X1A Just checking in, are you still interested in working on this?

[0X1A]

@aturon Sorry, I've been a bit swamped recently, someone else is free to take this one

[alexcrichton]

I've opened a PR for this at #4032