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..21612c38c4 100644 --- a/crates/vp_shared/src/env_vars.rs +++ b/crates/vp_shared/src/env_vars.rs @@ -95,7 +95,15 @@ 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 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 f41a08dfc7..fd95e56186 100644 --- a/crates/vp_shared/src/http.rs +++ b/crates/vp_shared/src/http.rs @@ -66,6 +66,54 @@ 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`. +const DEFAULT_DOWNLOAD_TIMEOUT: Duration = Duration::from_mins(10); + +/// 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. +const MAX_DOWNLOAD_TIMEOUT: Duration = Duration::from_hours(24); + +/// Per-request timeout for large file downloads. +/// +/// 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. 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) 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 && secs <= MAX_DOWNLOAD_TIMEOUT.as_secs() => { + Duration::from_secs(secs) + } + _ => { + output::warn(&vt_str::format!( + "ignoring invalid {}={value:?}: expected a number of seconds between 1 and {}", + env_vars::VP_DOWNLOAD_TIMEOUT, + MAX_DOWNLOAD_TIMEOUT.as_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 +351,56 @@ 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); + } + 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, "1800"); + } + assert_eq!(download_timeout(), Duration::from_mins(30)); + unsafe { + 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, "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, 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); + } + } + #[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..d281b6356e 100644 --- a/docs/guide/installer-env-vars.md +++ b/docs/guide/installer-env-vars.md @@ -100,6 +100,17 @@ 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` + +- **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 + # Allow up to 30 minutes per download on a slow connection + VP_DOWNLOAD_TIMEOUT=1800 vp env install 22 + ``` + ### `VP_SHELL` - **Purpose**: Specify the current shell