From bc82508b2dfcc7106d5ec449afc2487e6511624a Mon Sep 17 00:00:00 2001 From: Tarik Ermis Date: Sun, 9 Aug 2026 09:42:31 +0200 Subject: [PATCH 1/3] feat(cli): make download timeout configurable and more forgiving Node.js runtime and package-manager tarball downloads shared the HTTP client's hardcoded 2-minute per-request timeout with small metadata fetches. On slow or flaky connections that budget aborts an otherwise healthy transfer, and there was no way to override it. Give large file downloads their own per-request timeout: a 10-minute default, overridable via the VP_DOWNLOAD_TIMEOUT_SECS environment variable. The shared client's 2-minute default stays as-is so a stuck metadata fetch still fails fast. Closes #2370 --- crates/vp_js_runtime/src/download.rs | 7 ++- crates/vp_pm_cli/src/request.rs | 7 ++- crates/vp_shared/src/env_vars.rs | 9 +++- crates/vp_shared/src/http.rs | 77 ++++++++++++++++++++++++++++ crates/vp_shared/src/lib.rs | 2 +- docs/guide/installer-env-vars.md | 10 ++++ 6 files changed, 108 insertions(+), 4 deletions(-) diff --git a/crates/vp_js_runtime/src/download.rs b/crates/vp_js_runtime/src/download.rs index a6335129db..e0c72d728d 100644 --- a/crates/vp_js_runtime/src/download.rs +++ b/crates/vp_js_runtime/src/download.rs @@ -62,8 +62,13 @@ pub async fn download_file( // Make the request *and* the body stream a single retried unit, so a // truncated download (bytes written != advertised Content-Length) triggers // a re-download instead of surfacing as a corrupt archive later. + // + // Runtime archives are tens of megabytes, so the request gets the longer, + // configurable download budget instead of the shared client's 2-minute + // default — a slow-but-steady transfer must be allowed to finish. + let timeout = vp_shared::download_timeout(); let result = (|| async { - let response = client.get(url).send().await?.error_for_status()?; + let response = client.get(url).timeout(timeout).send().await?.error_for_status()?; // Advertised length, used both for the progress bar and the // truncation check below. diff --git a/crates/vp_pm_cli/src/request.rs b/crates/vp_pm_cli/src/request.rs index 33d5773864..05f82605b1 100644 --- a/crates/vp_pm_cli/src/request.rs +++ b/crates/vp_pm_cli/src/request.rs @@ -190,8 +190,13 @@ impl HttpClient { // the request inline (instead of calling `self.get`) avoids a double // retry layer. A truncated download (bytes written != advertised // Content-Length) returns an error so the retry re-downloads. + // + // Tarballs are large, so the request gets the longer, configurable + // download budget instead of the shared client's 2-minute default — + // a slow-but-steady transfer must be allowed to finish. + let timeout = vp_shared::download_timeout(); let result = (|| async { - let response = client.get(url).send().await?.error_for_status()?; + let response = client.get(url).timeout(timeout).send().await?.error_for_status()?; if let Some(ref pb) = progress { pb.set_position(0); if let Some(size) = response.content_length() { diff --git a/crates/vp_shared/src/env_vars.rs b/crates/vp_shared/src/env_vars.rs index 0588b56322..1a93e16544 100644 --- a/crates/vp_shared/src/env_vars.rs +++ b/crates/vp_shared/src/env_vars.rs @@ -95,7 +95,14 @@ pub const VP_CLI_BIN: &str = "VP_CLI_BIN"; /// Global CLI version, passed from Rust binary to JS for --version display. pub const VP_GLOBAL_VERSION: &str = "VP_GLOBAL_VERSION"; -// ── HTTP client TLS / CA configuration ────────────────────────────────── +// ── HTTP client configuration ─────────────────────────────────────────── + +/// Override the per-request timeout (in seconds) for large file downloads +/// (Node.js runtimes, package-manager tarballs). +/// +/// Must be a positive integer; an invalid value warns and is ignored. +/// Default: 600 (10 minutes). +pub const VP_DOWNLOAD_TIMEOUT_SECS: &str = "VP_DOWNLOAD_TIMEOUT_SECS"; /// Path to a PEM bundle of extra CA certificates to trust for HTTPS. /// diff --git a/crates/vp_shared/src/http.rs b/crates/vp_shared/src/http.rs index f41a08dfc7..ab03e620c3 100644 --- a/crates/vp_shared/src/http.rs +++ b/crates/vp_shared/src/http.rs @@ -66,6 +66,42 @@ const REQUEST_TIMEOUT: Duration = Duration::from_mins(2); /// retries (multiple minutes). const CONNECT_TIMEOUT: Duration = Duration::from_secs(30); +/// Default per-request timeout for large file downloads (Node.js runtimes, +/// package-manager tarballs). Those archives are tens of megabytes, so on +/// slow or flaky connections the shared [`REQUEST_TIMEOUT`] aborts an +/// otherwise healthy transfer; this budget is far more forgiving while still +/// bounding a stuck stream. Overridable via `VP_DOWNLOAD_TIMEOUT_SECS`. +const DEFAULT_DOWNLOAD_TIMEOUT: Duration = Duration::from_mins(10); + +/// Per-request timeout for large file downloads. +/// +/// Returns [`DEFAULT_DOWNLOAD_TIMEOUT`] unless `VP_DOWNLOAD_TIMEOUT_SECS` is +/// set to a positive integer number of seconds. A set-but-invalid value +/// (non-numeric, zero, negative) warns and falls back to the default. +/// +/// Call sites apply this per request rather than raising the shared client's +/// [`REQUEST_TIMEOUT`], which stays short so a single stuck metadata fetch +/// cannot hang a build. +#[must_use] +pub fn download_timeout() -> Duration { + let Some(value) = std::env::var_os(env_vars::VP_DOWNLOAD_TIMEOUT_SECS) else { + return DEFAULT_DOWNLOAD_TIMEOUT; + }; + if os_str_is_blank(&value) { + return DEFAULT_DOWNLOAD_TIMEOUT; + } + match value.to_str().and_then(|s| s.trim().parse::().ok()) { + Some(secs) if secs > 0 => Duration::from_secs(secs), + _ => { + output::warn(&vt_str::format!( + "ignoring invalid {}={value:?}: expected a positive integer number of seconds", + env_vars::VP_DOWNLOAD_TIMEOUT_SECS + )); + DEFAULT_DOWNLOAD_TIMEOUT + } + } +} + /// Get the process-wide `reqwest::Client`. /// /// The client is built on first call and reused thereafter. See module docs @@ -303,6 +339,47 @@ mod tests { assert!(message.contains(env_vars::NODE_EXTRA_CA_CERTS), "{message}"); } + #[test] + #[serial_test::serial(env)] + fn download_timeout_defaults_to_ten_minutes() { + // SAFETY: tests are run serially within this module for env vars. + unsafe { + std::env::remove_var(env_vars::VP_DOWNLOAD_TIMEOUT_SECS); + } + assert_eq!(download_timeout(), DEFAULT_DOWNLOAD_TIMEOUT); + assert_eq!(download_timeout(), Duration::from_mins(10)); + // The download budget must stay more forgiving than the shared + // per-request default, or slow tarball downloads keep timing out. + assert!(DEFAULT_DOWNLOAD_TIMEOUT > REQUEST_TIMEOUT); + } + + #[test] + #[serial_test::serial(env)] + fn download_timeout_honors_override_and_rejects_invalid_values() { + // SAFETY: tests are run serially within this module for env vars. + unsafe { + std::env::set_var(env_vars::VP_DOWNLOAD_TIMEOUT_SECS, "1800"); + } + assert_eq!(download_timeout(), Duration::from_mins(30)); + unsafe { + std::env::set_var(env_vars::VP_DOWNLOAD_TIMEOUT_SECS, " 120 "); + } + assert_eq!(download_timeout(), Duration::from_secs(120)); + for invalid in ["0", "-1", "abc", "1.5", "", " "] { + unsafe { + std::env::set_var(env_vars::VP_DOWNLOAD_TIMEOUT_SECS, invalid); + } + assert_eq!( + download_timeout(), + DEFAULT_DOWNLOAD_TIMEOUT, + "should fall back to the default: {invalid:?}" + ); + } + unsafe { + std::env::remove_var(env_vars::VP_DOWNLOAD_TIMEOUT_SECS); + } + } + #[test] fn os_str_is_blank_matches_whitespace_only() { assert!(os_str_is_blank(&OsString::from(""))); diff --git a/crates/vp_shared/src/lib.rs b/crates/vp_shared/src/lib.rs index bcac140c23..ed64ebf714 100644 --- a/crates/vp_shared/src/lib.rs +++ b/crates/vp_shared/src/lib.rs @@ -27,7 +27,7 @@ mod tracing; pub use env_config::{EnvConfig, TestEnvGuard}; pub use error::format_error_chain; pub use home::{VP_BINARY_NAME, get_vp_home}; -pub use http::{HttpClientError, shared_http_client}; +pub use http::{HttpClientError, download_timeout, shared_http_client}; pub use interactivity::{ is_ci_environment, is_interactive_terminal, is_stderr_terminal, is_stdin_terminal, is_stdout_terminal, diff --git a/docs/guide/installer-env-vars.md b/docs/guide/installer-env-vars.md index b4b087116b..39e10e1c30 100644 --- a/docs/guide/installer-env-vars.md +++ b/docs/guide/installer-env-vars.md @@ -100,6 +100,16 @@ These variables configure the installed Vite+ CLI. `VP_HOME` (above) also applie - **Default**: None (verification enabled) - **Details**: [Node.js Signature Verification](/guide/env#node-js-signature-verification) +### `VP_DOWNLOAD_TIMEOUT_SECS` + +- **Purpose**: Per-request timeout, in seconds, for large downloads such as Node.js runtimes and package-manager tarballs +- **Default**: `600` (10 minutes) +- **Example**: + ```bash + # Allow up to 30 minutes per download on a slow connection + VP_DOWNLOAD_TIMEOUT_SECS=1800 vp env install 22 + ``` + ### `VP_SHELL` - **Purpose**: Specify the current shell From f4632e6ffff7050a208eed9ceba49ded01adbddc Mon Sep 17 00:00:00 2001 From: Tarik Ermis Date: Sun, 9 Aug 2026 11:18:09 +0200 Subject: [PATCH 2/3] fix(cli): clamp VP_DOWNLOAD_TIMEOUT_SECS to a sane upper bound An extreme but parseable value (e.g. 18446744073709551615) was accepted and the resulting Duration overflowed reqwest's request-deadline computation (Instant + Duration panics; the release profile aborts), so a malformed timeout crashed every download instead of producing the documented warn-and-fallback. Reject values above 86400 (24 hours) with the same warn-and-fallback as other invalid values, and cover the boundary and overflow cases in the unit test. Refs #2370 --- crates/vp_shared/src/env_vars.rs | 4 ++-- crates/vp_shared/src/http.rs | 31 +++++++++++++++++++++++++------ docs/guide/installer-env-vars.md | 1 + 3 files changed, 28 insertions(+), 8 deletions(-) diff --git a/crates/vp_shared/src/env_vars.rs b/crates/vp_shared/src/env_vars.rs index 1a93e16544..282e2ca889 100644 --- a/crates/vp_shared/src/env_vars.rs +++ b/crates/vp_shared/src/env_vars.rs @@ -100,8 +100,8 @@ pub const VP_GLOBAL_VERSION: &str = "VP_GLOBAL_VERSION"; /// Override the per-request timeout (in seconds) for large file downloads /// (Node.js runtimes, package-manager tarballs). /// -/// Must be a positive integer; an invalid value warns and is ignored. -/// Default: 600 (10 minutes). +/// Must be a positive integer no larger than 86400 (24 hours); an invalid +/// value warns and is ignored. Default: 600 (10 minutes). pub const VP_DOWNLOAD_TIMEOUT_SECS: &str = "VP_DOWNLOAD_TIMEOUT_SECS"; /// Path to a PEM bundle of extra CA certificates to trust for HTTPS. diff --git a/crates/vp_shared/src/http.rs b/crates/vp_shared/src/http.rs index ab03e620c3..1df8bb6006 100644 --- a/crates/vp_shared/src/http.rs +++ b/crates/vp_shared/src/http.rs @@ -73,11 +73,18 @@ const CONNECT_TIMEOUT: Duration = Duration::from_secs(30); /// bounding a stuck stream. Overridable via `VP_DOWNLOAD_TIMEOUT_SECS`. const DEFAULT_DOWNLOAD_TIMEOUT: Duration = Duration::from_mins(10); +/// Largest accepted `VP_DOWNLOAD_TIMEOUT_SECS` value. Longer budgets are +/// absurd for a download — and extreme values (e.g. `u64::MAX`) overflow when +/// reqwest computes the request deadline: `Instant + Duration` panics, which +/// the release profile turns into an abort. +const MAX_DOWNLOAD_TIMEOUT: Duration = Duration::from_hours(24); + /// Per-request timeout for large file downloads. /// /// Returns [`DEFAULT_DOWNLOAD_TIMEOUT`] unless `VP_DOWNLOAD_TIMEOUT_SECS` is -/// set to a positive integer number of seconds. A set-but-invalid value -/// (non-numeric, zero, negative) warns and falls back to the default. +/// set to a positive integer number of seconds no larger than +/// [`MAX_DOWNLOAD_TIMEOUT`]. A set-but-invalid value (non-numeric, zero, +/// negative, above the maximum) warns and falls back to the default. /// /// Call sites apply this per request rather than raising the shared client's /// [`REQUEST_TIMEOUT`], which stays short so a single stuck metadata fetch @@ -91,11 +98,14 @@ pub fn download_timeout() -> Duration { return DEFAULT_DOWNLOAD_TIMEOUT; } match value.to_str().and_then(|s| s.trim().parse::().ok()) { - Some(secs) if secs > 0 => Duration::from_secs(secs), + Some(secs) if secs > 0 && secs <= MAX_DOWNLOAD_TIMEOUT.as_secs() => { + Duration::from_secs(secs) + } _ => { output::warn(&vt_str::format!( - "ignoring invalid {}={value:?}: expected a positive integer number of seconds", - env_vars::VP_DOWNLOAD_TIMEOUT_SECS + "ignoring invalid {}={value:?}: expected a number of seconds between 1 and {}", + env_vars::VP_DOWNLOAD_TIMEOUT_SECS, + MAX_DOWNLOAD_TIMEOUT.as_secs() )); DEFAULT_DOWNLOAD_TIMEOUT } @@ -365,7 +375,16 @@ mod tests { std::env::set_var(env_vars::VP_DOWNLOAD_TIMEOUT_SECS, " 120 "); } assert_eq!(download_timeout(), Duration::from_secs(120)); - for invalid in ["0", "-1", "abc", "1.5", "", " "] { + // Boundary: the maximum itself is accepted... + unsafe { + std::env::set_var(env_vars::VP_DOWNLOAD_TIMEOUT_SECS, "86400"); + } + assert_eq!(download_timeout(), MAX_DOWNLOAD_TIMEOUT); + // ...anything beyond it is not. Regression for extreme parseable + // values (u64::MAX) overflowing the request-deadline computation + // (`Instant + Duration` panics; the release profile aborts), which + // crashed every download instead of falling back with a warning. + for invalid in ["0", "-1", "abc", "1.5", "", " ", "86401", "18446744073709551615"] { unsafe { std::env::set_var(env_vars::VP_DOWNLOAD_TIMEOUT_SECS, invalid); } diff --git a/docs/guide/installer-env-vars.md b/docs/guide/installer-env-vars.md index 39e10e1c30..c3755d47e6 100644 --- a/docs/guide/installer-env-vars.md +++ b/docs/guide/installer-env-vars.md @@ -103,6 +103,7 @@ These variables configure the installed Vite+ CLI. `VP_HOME` (above) also applie ### `VP_DOWNLOAD_TIMEOUT_SECS` - **Purpose**: Per-request timeout, in seconds, for large downloads such as Node.js runtimes and package-manager tarballs +- **Values**: Positive integer, at most `86400` (24 hours); invalid values are ignored with a warning - **Default**: `600` (10 minutes) - **Example**: ```bash From 6d3b34c9fcd553318d260635c7048a2c26199bde Mon Sep 17 00:00:00 2001 From: Tarik Ermis Date: Mon, 10 Aug 2026 19:53:12 +0200 Subject: [PATCH 3/3] refactor(cli): rename download timeout env var to VP_DOWNLOAD_TIMEOUT Keep the unit out of the name so the value can later accept duration formats like 10m; plain integers remain seconds. Bounds (1..=86400), warning text, and warn-fallback behavior are unchanged. --- crates/vp_shared/src/env_vars.rs | 7 ++++--- crates/vp_shared/src/http.rs | 26 ++++++++++++++------------ docs/guide/installer-env-vars.md | 4 ++-- 3 files changed, 20 insertions(+), 17 deletions(-) diff --git a/crates/vp_shared/src/env_vars.rs b/crates/vp_shared/src/env_vars.rs index 282e2ca889..21612c38c4 100644 --- a/crates/vp_shared/src/env_vars.rs +++ b/crates/vp_shared/src/env_vars.rs @@ -100,9 +100,10 @@ pub const VP_GLOBAL_VERSION: &str = "VP_GLOBAL_VERSION"; /// Override the per-request timeout (in seconds) for large file downloads /// (Node.js runtimes, package-manager tarballs). /// -/// Must be a positive integer no larger than 86400 (24 hours); an invalid -/// value warns and is ignored. Default: 600 (10 minutes). -pub const VP_DOWNLOAD_TIMEOUT_SECS: &str = "VP_DOWNLOAD_TIMEOUT_SECS"; +/// Must be a positive integer number of seconds no larger than 86400 +/// (24 hours); an invalid value warns and is ignored. Default: 600 +/// (10 minutes). Duration formats like `10m` may be supported later. +pub const VP_DOWNLOAD_TIMEOUT: &str = "VP_DOWNLOAD_TIMEOUT"; /// Path to a PEM bundle of extra CA certificates to trust for HTTPS. /// diff --git a/crates/vp_shared/src/http.rs b/crates/vp_shared/src/http.rs index 1df8bb6006..fd95e56186 100644 --- a/crates/vp_shared/src/http.rs +++ b/crates/vp_shared/src/http.rs @@ -70,10 +70,10 @@ const CONNECT_TIMEOUT: Duration = Duration::from_secs(30); /// package-manager tarballs). Those archives are tens of megabytes, so on /// slow or flaky connections the shared [`REQUEST_TIMEOUT`] aborts an /// otherwise healthy transfer; this budget is far more forgiving while still -/// bounding a stuck stream. Overridable via `VP_DOWNLOAD_TIMEOUT_SECS`. +/// bounding a stuck stream. Overridable via `VP_DOWNLOAD_TIMEOUT`. const DEFAULT_DOWNLOAD_TIMEOUT: Duration = Duration::from_mins(10); -/// Largest accepted `VP_DOWNLOAD_TIMEOUT_SECS` value. Longer budgets are +/// Largest accepted `VP_DOWNLOAD_TIMEOUT` value. Longer budgets are /// absurd for a download — and extreme values (e.g. `u64::MAX`) overflow when /// reqwest computes the request deadline: `Instant + Duration` panics, which /// the release profile turns into an abort. @@ -81,17 +81,19 @@ const MAX_DOWNLOAD_TIMEOUT: Duration = Duration::from_hours(24); /// Per-request timeout for large file downloads. /// -/// Returns [`DEFAULT_DOWNLOAD_TIMEOUT`] unless `VP_DOWNLOAD_TIMEOUT_SECS` is +/// Returns [`DEFAULT_DOWNLOAD_TIMEOUT`] unless `VP_DOWNLOAD_TIMEOUT` is /// set to a positive integer number of seconds no larger than /// [`MAX_DOWNLOAD_TIMEOUT`]. A set-but-invalid value (non-numeric, zero, -/// negative, above the maximum) warns and falls back to the default. +/// negative, above the maximum) warns and falls back to the default. The +/// value is a plain number of seconds today; duration formats like `10m` +/// may be supported later. /// /// Call sites apply this per request rather than raising the shared client's /// [`REQUEST_TIMEOUT`], which stays short so a single stuck metadata fetch /// cannot hang a build. #[must_use] pub fn download_timeout() -> Duration { - let Some(value) = std::env::var_os(env_vars::VP_DOWNLOAD_TIMEOUT_SECS) else { + let Some(value) = std::env::var_os(env_vars::VP_DOWNLOAD_TIMEOUT) else { return DEFAULT_DOWNLOAD_TIMEOUT; }; if os_str_is_blank(&value) { @@ -104,7 +106,7 @@ pub fn download_timeout() -> Duration { _ => { output::warn(&vt_str::format!( "ignoring invalid {}={value:?}: expected a number of seconds between 1 and {}", - env_vars::VP_DOWNLOAD_TIMEOUT_SECS, + env_vars::VP_DOWNLOAD_TIMEOUT, MAX_DOWNLOAD_TIMEOUT.as_secs() )); DEFAULT_DOWNLOAD_TIMEOUT @@ -354,7 +356,7 @@ mod tests { fn download_timeout_defaults_to_ten_minutes() { // SAFETY: tests are run serially within this module for env vars. unsafe { - std::env::remove_var(env_vars::VP_DOWNLOAD_TIMEOUT_SECS); + std::env::remove_var(env_vars::VP_DOWNLOAD_TIMEOUT); } assert_eq!(download_timeout(), DEFAULT_DOWNLOAD_TIMEOUT); assert_eq!(download_timeout(), Duration::from_mins(10)); @@ -368,16 +370,16 @@ mod tests { fn download_timeout_honors_override_and_rejects_invalid_values() { // SAFETY: tests are run serially within this module for env vars. unsafe { - std::env::set_var(env_vars::VP_DOWNLOAD_TIMEOUT_SECS, "1800"); + std::env::set_var(env_vars::VP_DOWNLOAD_TIMEOUT, "1800"); } assert_eq!(download_timeout(), Duration::from_mins(30)); unsafe { - std::env::set_var(env_vars::VP_DOWNLOAD_TIMEOUT_SECS, " 120 "); + std::env::set_var(env_vars::VP_DOWNLOAD_TIMEOUT, " 120 "); } assert_eq!(download_timeout(), Duration::from_secs(120)); // Boundary: the maximum itself is accepted... unsafe { - std::env::set_var(env_vars::VP_DOWNLOAD_TIMEOUT_SECS, "86400"); + std::env::set_var(env_vars::VP_DOWNLOAD_TIMEOUT, "86400"); } assert_eq!(download_timeout(), MAX_DOWNLOAD_TIMEOUT); // ...anything beyond it is not. Regression for extreme parseable @@ -386,7 +388,7 @@ mod tests { // crashed every download instead of falling back with a warning. for invalid in ["0", "-1", "abc", "1.5", "", " ", "86401", "18446744073709551615"] { unsafe { - std::env::set_var(env_vars::VP_DOWNLOAD_TIMEOUT_SECS, invalid); + std::env::set_var(env_vars::VP_DOWNLOAD_TIMEOUT, invalid); } assert_eq!( download_timeout(), @@ -395,7 +397,7 @@ mod tests { ); } unsafe { - std::env::remove_var(env_vars::VP_DOWNLOAD_TIMEOUT_SECS); + std::env::remove_var(env_vars::VP_DOWNLOAD_TIMEOUT); } } diff --git a/docs/guide/installer-env-vars.md b/docs/guide/installer-env-vars.md index c3755d47e6..d281b6356e 100644 --- a/docs/guide/installer-env-vars.md +++ b/docs/guide/installer-env-vars.md @@ -100,7 +100,7 @@ These variables configure the installed Vite+ CLI. `VP_HOME` (above) also applie - **Default**: None (verification enabled) - **Details**: [Node.js Signature Verification](/guide/env#node-js-signature-verification) -### `VP_DOWNLOAD_TIMEOUT_SECS` +### `VP_DOWNLOAD_TIMEOUT` - **Purpose**: Per-request timeout, in seconds, for large downloads such as Node.js runtimes and package-manager tarballs - **Values**: Positive integer, at most `86400` (24 hours); invalid values are ignored with a warning @@ -108,7 +108,7 @@ These variables configure the installed Vite+ CLI. `VP_HOME` (above) also applie - **Example**: ```bash # Allow up to 30 minutes per download on a slow connection - VP_DOWNLOAD_TIMEOUT_SECS=1800 vp env install 22 + VP_DOWNLOAD_TIMEOUT=1800 vp env install 22 ``` ### `VP_SHELL`