[djc]

Fixes #5176.

(With help from Claude.)

[rami3l]

@iliana @Skgland Would you mind giving our test build a try and see if rustup can better handle your network connection? You can download the corresponding one for your host at https://github.com/rust-lang/rustup/actions/runs/41038394164 🙏

[rami3l]

LGTM, although I'd love if this can be tested before being merged 🙏

PS: Also changed the patch's title to make it clear that this is resuming server-interrupted downloads (instead of user-interrupted downloads which we already support).

View changes since this review

[djc]

LGTM, although I'd love if this can be tested before being merged 🙏

Testing it isn't really trivial -- the dist test fixtures are using file:// URLs and so far we don't have a HTTP server for them AIUI. We'd also need a much larger file.

[rami3l]

LGTM, although I'd love if this can be tested before being merged 🙏

Testing it isn't really trivial -- the dist test fixtures are using file:// URLs and so far we don't have a HTTP server for them AIUI. We'd also need a much larger file.

I totally understand that, and that is why I think some manual testing could be helpful here. 🙏

[Skgland]

@iliana @Skgland Would you mind giving our test build a try and see if rustup can better handle your network connection? You can download the corresponding one for your host at https://github.com/rust-lang/rustup/actions/runs/41038394164 🙏

I won't be able to try it before sunday.

[iliana]

With this change on my system I am now able to get toolchains to install, although I did hit:

info: checking for self-update (current version: 1.30.0)
info: downloading self-update (new version: 1.29.2)
error: could not download file from 'https://static.rust-lang.org/rustup/archive/1.29.2/x86_64-unknown-linux-gnu/rustup-init' to '/home/iliana/.rustup/self-update/rustup-init': error decoding response body: request or response body error: error reading a body from connection: connection reset

The behavior is now tolerable but still annoying compared to the curl backend due to what seem like sleeps/exponential backoff between retries. curl might be experiencing the same connection resets but retrying without any backoff, I'm not entirely sure.

[rami3l]

With this change on my system I am now able to get toolchains to install, although I did hit:

info: checking for self-update (current version: 1.30.0)
info: downloading self-update (new version: 1.29.2)
error: could not download file from 'https://static.rust-lang.org/rustup/archive/1.29.2/x86_64-unknown-linux-gnu/rustup-init' to '/home/iliana/.rustup/self-update/rustup-init': error decoding response body: request or response body error: error reading a body from connection: connection reset

The behavior is now tolerable but still annoying compared to the curl backend due to what seem like sleeps/exponential backoff between retries. curl might be experiencing the same connection resets but retrying without any backoff, I'm not entirely sure.

@iliana As I explained above, rustup's curl backend doesn't expose this behavior because it's too old to support http2; we are trying to improve http2 support here so there is no parity to be reached WRT the curl backend. Removing http2 support or defaulting to http1.1 when using the reqwest backend is NOT an option here for us.

[iliana]

That makes sense! Sorry I missed that explanation.

[djc]

The behavior is now tolerable but still annoying compared to the curl backend due to what seem like sleeps/exponential backoff between retries. curl might be experiencing the same connection resets but retrying without any backoff, I'm not entirely sure.

How do you sleeps/exponential backoffs? I don't think we have those? If there is something we could maybe avoid it for this particular failure mode.

[djc]

@iliana As I explained above, rustup's curl backend doesn't expose this behavior because it's too old to support http2; we are trying to improve http2 support here so there is no parity to be reached WRT the libcurl backend. Removing http2 support or defaulting to http1.1 when using the reqwest backend is NOT an option here for us.

But, maybe there is a way to fall back to HTTP/1 in failure cases? Not sure how easy reqwest makes that sort of thing.

[iliana]

How do you sleeps/exponential backoffs? I don't think we have those?

Here's what I see: https://asciinema.org/a/7eDycB7rXUMn1WLX

This may simply be the amount of time it takes to start the connection + request again?

[djc]

How do you sleeps/exponential backoffs? I don't think we have those?

Here's what I see: https://asciinema.org/a/7eDycB7rXUMn1WLX

This may simply be the amount of time it takes to start the connection + request again?

Yeah, the delay in retrying is a little surprising. Also, I guess we should retry the rustup self-update download, as well?

Still, seems like this PR provides a decent improvement, in that your toolchain component downloads can now actually complete.

[Skgland]

Appears to be working better.
Though the fix doesn't appear to be effective for automatic/implicit self update?
While manual/explicit self update appears to work.

https://asciinema.org/a/d45nBt3okPDBAqb6

[rami3l]

@Skgland Thanks for the feedback!

Though the fix doesn't appear to be effective for automatic/implicit self update?

As far as I know this logic is not properly applied to self update; will investigate.