From 369e65089d3744c4168211243081f8a637b7891c Mon Sep 17 00:00:00 2001 From: Kim Morrison Date: Wed, 11 Feb 2026 00:08:50 +0000 Subject: [PATCH 1/4] feat: resume interrupted downloads via HTTP Range headers Port download resumption from rustup. When a download fails due to a network error, elan now automatically retries once, resuming from where it left off using HTTP Range headers. Also adds a stall timeout (30s at <10 bytes/sec) so downloads no longer hang indefinitely on unreliable connections. Closes https://github.com/leanprover/elan/issues/XXX Co-Authored-By: Claude Opus 4.6 --- src/download/src/lib.rs | 102 +++++++++++++++++++++++++------ src/elan-cli/download_tracker.rs | 13 ++++ src/elan-utils/src/utils.rs | 42 +++++++++++-- 3 files changed, 131 insertions(+), 26 deletions(-) diff --git a/src/download/src/lib.rs b/src/download/src/lib.rs index 972c463..42d15c1 100644 --- a/src/download/src/lib.rs +++ b/src/download/src/lib.rs @@ -14,6 +14,8 @@ pub enum Backend { #[derive(Debug, Copy, Clone)] pub enum Event<'a> { + /// Resuming a partial download. + ResumingPartialDownload, /// Received the Content-Length of the to-be downloaded data. DownloadContentLengthReceived(u64), /// Received some data. @@ -23,10 +25,11 @@ pub enum Event<'a> { fn download_with_backend( backend: Backend, url: &Url, + resume_from: u64, callback: &dyn Fn(Event<'_>) -> Result<()>, ) -> Result<()> { match backend { - Backend::Curl => curl::download(url, callback), + Backend::Curl => curl::download(url, resume_from, callback), } } @@ -34,22 +37,54 @@ pub fn download_to_path_with_backend( backend: Backend, url: &Url, path: &Path, + resume_from_partial: bool, callback: Option<&dyn Fn(Event<'_>) -> Result<()>>, ) -> Result<()> { use std::cell::RefCell; use std::fs::OpenOptions; - use std::io::Write; + use std::io::{Seek, SeekFrom, Write}; || -> Result<()> { - let file = OpenOptions::new() - .write(true) - .create(true) - .open(path) - .chain_err(|| "error creating file for download")?; + let (file, resume_from) = if resume_from_partial { + let possible_partial = OpenOptions::new().read(true).open(path); + + let downloaded_so_far = if let Ok(partial) = possible_partial { + if let Some(cb) = callback { + cb(Event::ResumingPartialDownload)?; + } + let file_info = partial + .metadata() + .chain_err(|| "error reading partial download metadata")?; + file_info.len() + } else { + 0 + }; + + let mut file = OpenOptions::new() + .write(true) + .create(true) + .truncate(false) + .open(path) + .chain_err(|| "error opening file for download")?; + + file.seek(SeekFrom::End(0)) + .chain_err(|| "error seeking in partial download")?; + + (file, downloaded_so_far) + } else { + let file = OpenOptions::new() + .write(true) + .create(true) + .truncate(true) + .open(path) + .chain_err(|| "error creating file for download")?; + + (file, 0) + }; let file = RefCell::new(file); - download_with_backend(backend, url, &|event| { + download_with_backend(backend, url, resume_from, &|event| { if let Event::DownloadDataReceived(data) = event { file.borrow_mut() .write_all(data) @@ -84,7 +119,11 @@ pub mod curl { thread_local!(pub static EASY: RefCell = RefCell::new(Easy::new())); - pub fn download(url: &Url, callback: &dyn Fn(Event<'_>) -> Result<()>) -> Result<()> { + pub fn download( + url: &Url, + resume_from: u64, + callback: &dyn Fn(Event<'_>) -> Result<()>, + ) -> Result<()> { // Fetch either a cached libcurl handle (which will preserve open // connections) or create a new one if it isn't listed. // @@ -98,11 +137,30 @@ pub mod curl { .follow_location(true) .chain_err(|| "failed to set follow redirects")?; + if resume_from > 0 { + handle + .resume_from(resume_from) + .chain_err(|| "failed to set resume from")?; + } else { + // An error here indicates that the range header isn't supported + // by underlying curl, so there's nothing to "clear" — safe to + // ignore this error. + let _ = handle.resume_from(0); + } + // Take at most 30s to connect handle .connect_timeout(Duration::new(30, 0)) .chain_err(|| "failed to set connect timeout")?; + // Abort if the speed is below 10 bytes/sec for 30 seconds + handle + .low_speed_limit(10) + .chain_err(|| "failed to set low speed limit")?; + handle + .low_speed_time(Duration::new(30, 0)) + .chain_err(|| "failed to set low speed time")?; + { let cberr = RefCell::new(None); let mut transfer = handle.transfer(); @@ -120,20 +178,24 @@ pub mod curl { }) .chain_err(|| "failed to set write")?; - // Listen for headers and parse out a `Content-Length` if it comes - // so we know how much we're downloading. + // Listen for headers and parse out a `Content-Length` + // (case-insensitive) if it comes so we know how much we're + // downloading. transfer .header_function(|header| { if let Ok(data) = str::from_utf8(header) { - let prefix = "Content-Length: "; - if data.starts_with(prefix) { - if let Ok(s) = data[prefix.len()..].trim().parse::() { - let msg = Event::DownloadContentLengthReceived(s); - match callback(msg) { - Ok(()) => (), - Err(e) => { - *cberr.borrow_mut() = Some(e); - return false; + let prefix = "content-length: "; + if let Some((dp, ds)) = data.split_at_checked(prefix.len()) { + if dp.eq_ignore_ascii_case(prefix) { + if let Ok(s) = ds.trim().parse::() { + let msg = + Event::DownloadContentLengthReceived(s + resume_from); + match callback(msg) { + Ok(()) => (), + Err(e) => { + *cberr.borrow_mut() = Some(e); + return false; + } } } } diff --git a/src/elan-cli/download_tracker.rs b/src/elan-cli/download_tracker.rs index bdd0b38..a8244f9 100644 --- a/src/elan-cli/download_tracker.rs +++ b/src/elan-cli/download_tracker.rs @@ -69,6 +69,10 @@ impl DownloadTracker { self.download_finished(); true } + Notification::Install(In::Utils(Un::ResumingPartialDownload)) => { + self.resuming_partial_download(); + true + } _ => false, } } @@ -104,6 +108,15 @@ impl DownloadTracker { } } } + /// Notifies self that a partial download is being resumed. + /// Resets speed tracking but preserves total_downloaded since the + /// content-length from the server will reflect the full file size. + pub fn resuming_partial_download(&mut self) { + self.downloaded_this_sec = 0; + self.downloaded_last_few_secs.clear(); + self.seconds_elapsed = 0; + self.last_sec = None; + } /// Notifies self that the download has finished. pub fn download_finished(&mut self) { if self.displayed_charcount.is_some() { diff --git a/src/elan-utils/src/utils.rs b/src/elan-utils/src/utils.rs index 9b14b83..55be181 100644 --- a/src/elan-utils/src/utils.rs +++ b/src/elan-utils/src/utils.rs @@ -126,17 +126,40 @@ pub fn download_file( url: &Url, path: &Path, notify_handler: &dyn Fn(Notification<'_>), +) -> Result<()> { + download_file_with_resume(url, path, false, notify_handler) +} + +pub fn download_file_with_resume( + url: &Url, + path: &Path, + resume_from_partial: bool, + notify_handler: &dyn Fn(Notification<'_>), ) -> Result<()> { use download::ErrorKind as DEK; - match download_file_(url, path, notify_handler) { + match download_file_(url, path, resume_from_partial, notify_handler) { Ok(_) => Ok(()), Err(e) => { - println!("{:?}", e); let is_client_error = match e.kind() { + // Specifically treat the bad partial range error as not our + // fault in case it was something odd which happened. + &ErrorKind::Download(DEK::HttpStatus(416)) => false, &ErrorKind::Download(DEK::HttpStatus(400..=499)) => true, &ErrorKind::Download(DEK::FileNotFound) => true, _ => false, }; + if !is_client_error && !resume_from_partial { + // Retry once with resume from partial + match download_file_(url, path, true, notify_handler) { + Ok(_) => return Ok(()), + Err(retry_err) => { + return Err(retry_err).chain_err(|| ErrorKind::DownloadingFile { + url: url.clone(), + path: path.to_path_buf(), + }); + } + } + } Err(e).chain_err(|| { if is_client_error { ErrorKind::DownloadNotExists { @@ -154,16 +177,23 @@ pub fn download_file( } } -fn download_file_(url: &Url, path: &Path, notify_handler: &dyn Fn(Notification<'_>)) -> Result<()> { +fn download_file_( + url: &Url, + path: &Path, + resume_from_partial: bool, + notify_handler: &dyn Fn(Notification<'_>), +) -> Result<()> { use download::download_to_path_with_backend; use download::{Backend, Event}; notify_handler(Notification::DownloadingFile(url, path)); - // This callback will write the download to disk and optionally - // hash the contents, then forward the notification up the stack + // This callback will forward the notification up the stack let callback: &dyn Fn(Event<'_>) -> download::Result<()> = &|msg| { match msg { + Event::ResumingPartialDownload => { + notify_handler(Notification::ResumingPartialDownload); + } Event::DownloadContentLengthReceived(len) => { notify_handler(Notification::DownloadContentLengthReceived(len)); } @@ -179,7 +209,7 @@ fn download_file_(url: &Url, path: &Path, notify_handler: &dyn Fn(Notification<' let (backend, notification) = (Backend::Curl, Notification::UsingCurl); notify_handler(notification); - download_to_path_with_backend(backend, url, path, Some(callback))?; + download_to_path_with_backend(backend, url, path, resume_from_partial, Some(callback))?; notify_handler(Notification::DownloadFinished); From 6a6b37716dfc2895dc039cda9c6a212a46c52f1e Mon Sep 17 00:00:00 2001 From: Kim Morrison Date: Wed, 11 Feb 2026 00:12:10 +0000 Subject: [PATCH 2/4] fix: handle servers that ignore Range headers and 416 errors Address review feedback: - Detect when server returns 200 instead of 206 (ignoring Range header) and fall back to a fresh download to avoid data corruption - On HTTP 416 (Range Not Satisfiable), fall back to fresh download instead of permanently failing - Only treat read-open errors as "no file" for NotFound; propagate other errors (permissions, I/O) instead of silently appending Co-Authored-By: Claude Opus 4.6 --- src/download/src/errors.rs | 3 +++ src/download/src/lib.rs | 35 +++++++++++++++++++++++------------ src/elan-utils/src/utils.rs | 26 ++++++++++++++++++++++---- 3 files changed, 48 insertions(+), 16 deletions(-) diff --git a/src/download/src/errors.rs b/src/download/src/errors.rs index 14f337f..5695cc1 100644 --- a/src/download/src/errors.rs +++ b/src/download/src/errors.rs @@ -20,5 +20,8 @@ error_chain! { description("download backend unavailable") display("download backend '{}' unavailable", be) } + ResumeNotSupported { + description("server does not support resuming downloads") + } } } diff --git a/src/download/src/lib.rs b/src/download/src/lib.rs index 42d15c1..cae45b9 100644 --- a/src/download/src/lib.rs +++ b/src/download/src/lib.rs @@ -48,27 +48,31 @@ pub fn download_to_path_with_backend( let (file, resume_from) = if resume_from_partial { let possible_partial = OpenOptions::new().read(true).open(path); - let downloaded_so_far = if let Ok(partial) = possible_partial { - if let Some(cb) = callback { - cb(Event::ResumingPartialDownload)?; + let downloaded_so_far = match possible_partial { + Ok(partial) => { + if let Some(cb) = callback { + cb(Event::ResumingPartialDownload)?; + } + let file_info = partial + .metadata() + .chain_err(|| "error reading partial download metadata")?; + file_info.len() } - let file_info = partial - .metadata() - .chain_err(|| "error reading partial download metadata")?; - file_info.len() - } else { - 0 + Err(e) if e.kind() == std::io::ErrorKind::NotFound => 0, + Err(e) => return Err(e).chain_err(|| "error opening partial download"), }; let mut file = OpenOptions::new() .write(true) .create(true) - .truncate(false) + .truncate(downloaded_so_far == 0) .open(path) .chain_err(|| "error opening file for download")?; - file.seek(SeekFrom::End(0)) - .chain_err(|| "error seeking in partial download")?; + if downloaded_so_far > 0 { + file.seek(SeekFrom::End(0)) + .chain_err(|| "error seeking in partial download")?; + } (file, downloaded_so_far) } else { @@ -235,6 +239,13 @@ pub mod curl { } }; + // If we asked for a range but got 200 instead of 206, the server + // ignored our Range header and sent the full file. Report this so + // the caller can truncate and use the full response. + if resume_from > 0 && code == 200 { + return Err(ErrorKind::ResumeNotSupported.into()); + } + Ok(()) }) } diff --git a/src/elan-utils/src/utils.rs b/src/elan-utils/src/utils.rs index 55be181..ff1d353 100644 --- a/src/elan-utils/src/utils.rs +++ b/src/elan-utils/src/utils.rs @@ -141,15 +141,32 @@ pub fn download_file_with_resume( Ok(_) => Ok(()), Err(e) => { let is_client_error = match e.kind() { - // Specifically treat the bad partial range error as not our - // fault in case it was something odd which happened. - &ErrorKind::Download(DEK::HttpStatus(416)) => false, &ErrorKind::Download(DEK::HttpStatus(400..=499)) => true, &ErrorKind::Download(DEK::FileNotFound) => true, _ => false, }; + + // On 416 (Range Not Satisfiable) or ResumeNotSupported (server + // returned 200 instead of 206), fall back to a fresh download. + let should_restart_fresh = matches!( + e.kind(), + &ErrorKind::Download(DEK::HttpStatus(416)) + | &ErrorKind::Download(DEK::ResumeNotSupported) + ); + if should_restart_fresh { + match download_file_(url, path, false, notify_handler) { + Ok(_) => return Ok(()), + Err(retry_err) => { + return Err(retry_err).chain_err(|| ErrorKind::DownloadingFile { + url: url.clone(), + path: path.to_path_buf(), + }); + } + } + } + + // On network errors, retry once with resume from the partial file. if !is_client_error && !resume_from_partial { - // Retry once with resume from partial match download_file_(url, path, true, notify_handler) { Ok(_) => return Ok(()), Err(retry_err) => { @@ -160,6 +177,7 @@ pub fn download_file_with_resume( } } } + Err(e).chain_err(|| { if is_client_error { ErrorKind::DownloadNotExists { From 5f1b48a78c1f55c03c92b4b696127ff8353d7e33 Mon Sep 17 00:00:00 2001 From: Kim Morrison Date: Wed, 11 Feb 2026 00:29:32 +0000 Subject: [PATCH 3/4] fix: revert strict read-open error handling back to rustup's behavior Any failure to open the partial file (not just NotFound) falls back to downloaded_so_far=0. This is safe because truncate(downloaded_so_far==0) ensures a fresh start, and avoids turning a recoverable situation (e.g. permission error on a temp file) into a hard failure. Co-Authored-By: Claude Opus 4.6 --- src/download/src/lib.rs | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/download/src/lib.rs b/src/download/src/lib.rs index cae45b9..04066d3 100644 --- a/src/download/src/lib.rs +++ b/src/download/src/lib.rs @@ -48,18 +48,16 @@ pub fn download_to_path_with_backend( let (file, resume_from) = if resume_from_partial { let possible_partial = OpenOptions::new().read(true).open(path); - let downloaded_so_far = match possible_partial { - Ok(partial) => { - if let Some(cb) = callback { - cb(Event::ResumingPartialDownload)?; - } - let file_info = partial - .metadata() - .chain_err(|| "error reading partial download metadata")?; - file_info.len() + let downloaded_so_far = if let Ok(partial) = possible_partial { + if let Some(cb) = callback { + cb(Event::ResumingPartialDownload)?; } - Err(e) if e.kind() == std::io::ErrorKind::NotFound => 0, - Err(e) => return Err(e).chain_err(|| "error opening partial download"), + let file_info = partial + .metadata() + .chain_err(|| "error reading partial download metadata")?; + file_info.len() + } else { + 0 }; let mut file = OpenOptions::new() From a68b73df88f5f97f27dfc841310fb41d74d41ca3 Mon Sep 17 00:00:00 2001 From: Sebastian Ullrich Date: Thu, 19 Feb 2026 08:33:56 +0000 Subject: [PATCH 4/4] add changelog --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ba93d8..51b18b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +# Unreleased + +- Resume interrupted downloads via HTTP Range headers. When a download fails due to a network error, + elan now automatically retries once, resuming from where it left off. Also adds a stall timeout + (30s at <10 bytes/sec) so downloads no longer hang indefinitely on unreliable connections. + # 4.1.2 - 2025-05-26 - Ignore errors when writing to `.elan/known-projects`, e.g. when it is read-only.