Introduce RUSTUP_DOWNLOAD_TIMEOUT to override the download timeout
#4440
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rami3l
reviewed
Aug 10, 2025
src/download/mod.rs
Outdated
| let timeout = Duration::from_secs(match process.var("RUSTUP_DOWNLOAD_TIMEOUT") { | ||
| Ok(s) => s.parse::<NonZeroU64>().context( | ||
| "invalid value in RUSTUP_DOWNLOAD_TIMEOUT -- must be a natural number greater than zero", | ||
| ).unwrap().get(), |
There was a problem hiding this comment.
Why are we using an .unwrap() here?
There was a problem hiding this comment.
To get the NonZeroU64 out of the Result; but after reading about good practices, I realized that using ? instead of .unwrap() is preferable.
Is this what you were referring to?
There was a problem hiding this comment.
FYI, IMO it's nicer to write NonZeroU64::from_str(s) compared to s.parse::<NonZeroU64>().
…ding download timeout As some (corporate) firewalls take too much time to verify the downloads, the timeout is reached and the user cannot complete its installation. Besides changing the timeout from 30 secs to 3 mins, a new environment variable is introduced to allow users to override the timeout value to their taste.
FranciscoTGouveia
force-pushed
the
download-timeout
branch
from
bf699dd to
d19ffb2
Compare
djc
reviewed
Aug 12, 2025
Comment on lines
+637
to
+654
| pub(super) fn native_tls_client(timeout: Duration) -> Result<&'static Client, DownloadError> { | ||
| if let Some(client) = CLIENT_NATIVE_TLS.get() { | ||
| return Ok(client); | ||
| } | ||
|
|
||
| // woah, an unwrap?! | ||
| // It's OK. This is the same as what is happening in curl. | ||
| // | ||
| // The curl::Easy::new() internally assert!s that the initialized | ||
| // Easy is not null. Inside reqwest, the errors here would be from | ||
| // the TLS library returning a null pointer as well. | ||
| catcher().unwrap() | ||
| }); | ||
| let client = client_generic() | ||
| .read_timeout(timeout) | ||
| .user_agent(super::REQWEST_DEFAULT_TLS_USER_AGENT) | ||
| .build() | ||
| .map_err(DownloadError::Reqwest)?; | ||
|
|
||
| let _ = CLIENT_NATIVE_TLS.set(client); | ||
|
|
||
| Ok(CLIENT_NATIVE_TLS.get().unwrap()) | ||
| } | ||
|
|
||
| #[cfg(feature = "reqwest-native-tls")] | ||
| static CLIENT_NATIVE_TLS: OnceLock<Client> = OnceLock::new(); |
There was a problem hiding this comment.
The change from LazyLock to OnceLock seems unrelated and doesn't look like an improvement to me.
rami3l
reviewed
Aug 12, 2025
| @@ -66,6 +66,9 @@ | |||
|
|
|||
| - `RUSTUP_TERM_WIDTH` (default: none). Allows to override the terminal width for progress bars. | |||
|
|
|||
| - `RUSTUP_DOWNLOAD_TIMEOUT` (default: 180). Allows to override the default | |||
There was a problem hiding this comment.
Nit: You probably need to mark this variable as unstable.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #4439.
Besides increasing the default timeout from 30 seconds to 3 minutes, a new environment variable (
RUSTUP_DOWNLOAD_TIMEOUT) is introduced to allow users to override this value.