Fetch Ruff from an Astral mirror

[zsol]

Apply the hardcoded Astral mirror pattern to urls coming from the ndjson, then use the same fallback-first-then-retry loop in download_and_unpack_with_retry as for Python downloads (extracting this as shared logic, since we'll be needing this in at least one more place: #18358)

[konstin]

iirc we retryable_on_request_failure on the whole error type, or at least that's what we usually do to catch all cases. In this file, we have e.g.:

uv/crates/uv-bin-install/src/lib.rs

Lines 707 to 709 in 401661e

	let reader = response
	.bytes_stream()
	.map_err(std::io::Error::other)

which would bubble into the wrong path.

[zsol]

Are you saying we don't need to investigate the uv-bin-isntall-level Error at all and simply delegate to retryable_on_request_failure from should_try_next_url?
Or to do something like:

match self {
  Self::Download { .. } => true,
  Self::RetriedError { err, .. } => err.should_try_next_url(),
  _ => retryable_on_request_failure(self) == Some(Retryable::Transient),
}

(I don't really understand how the example you gave will bubble into the wrong path - AFAICT it will match Self::Extract, where source will be something that eventually wraps std::io::Error::other which itself wraps the reqwest/h2/... error, but that will be extracted by retryable_on_request_failure, no?)

[zsol]

Alternatively, we could simply change the stream reader to always wrap its error with Error::Download instead of (or in addition to?) io::Error::other, and then do a very simple source iteration in should_try_next_url similar to what retryable_on_request_failure does - but now it only needs to check for Error::Download. How does that sound?

Edit: actually response already wraps its errors like that, so we don't need to change the download side.

[konstin]

Are you saying we don't need to investigate the uv-bin-isntall-level Error at all and simply delegate to retryable_on_request_failure from should_try_next_url?

I think so, at least I can't come up with an example where retryable_on_request_failure doesn't work.

[zsol]

I got the LLM to conjure an error case that isn't currently classified as retriable but we should still fall back to the next url for. I added a test case for this (invalid chunk size during a chunked-encoded http response).

I made this work by inventing a new Error::Streaming that wraps all errors during the reading of the http stream, and specifically looking for this during walking of the error chain.

Wdyt?

[konstin]

This exercises a realistic body-streaming protocol failure: the server advertises chunked transfer encoding but sends an invalid chunk size.

That sounds like something that either retryable_on_request_failure should cover, or should be fatal (here: next URL), it's the same problem for other types of requests. Is the problem that retryable_on_request_failure doesn't see the error type due to e.g. #[error(transparent)], or that we don't match that kind of error? It may just be that we consider this error fatal because it's not an error that happens transiently and goes away when retrying the URL.

[zsol]

The issue is that retryable_on_request_failure might consider some errors fatal (for good reasons), but they aren't fatal for the purposes of falling back to the next URL. I'm thinking of errors that point to a misconfiguration of the host we're downloading from; this is the reason I wanted to capture all network errors for Python downloading in

uv/crates/uv-python/src/downloads.rs

Lines 158 to 171 in 111b0f6

	fn should_try_next_url(&self) -> bool {
	match self {
	// There are two primary reasons to try an alternative URL:
	// - HTTP/DNS/TCP/etc errors due to a mirror being blocked at various layers
	// - HTTP 404s from the mirror, which may mean the next URL still works
	// So we catch all network-level errors here.
	Self::NetworkError(..)
	| Self::NetworkMiddlewareError(..)
	| Self::NetworkErrorWithRetries { .. } => true,
	// `Io` uses `#[error(transparent)]`, so `source()` delegates to the inner error's
	// own source rather than returning the `io::Error` itself. We must unwrap it
	// explicitly so that `retryable_on_request_failure` can inspect the io error kind.
	Self::Io(err) => retryable_on_request_failure(err).is_some(),
	_ => false,

The invalid chunk size case is arguably a sign of the host being broken, and I wouldn't have high hopes of a retry working, so I think it makes sense to mark it as fatal for retry purposes