diff --git a/cli/src/install.rs b/cli/src/install.rs index 7ad3651..5410a1d 100644 --- a/cli/src/install.rs +++ b/cli/src/install.rs @@ -183,9 +183,12 @@ fn platform_key() -> &'static str { } async fn fetch_download_url() -> Result<(String, String), String> { - let resp = reqwest::get(LAST_KNOWN_GOOD_URL) + let client = http_client()?; + let resp = client + .get(LAST_KNOWN_GOOD_URL) + .send() .await - .map_err(|e| format!("Failed to fetch version info: {}", e))?; + .map_err(|e| format!("Failed to fetch version info: {}", format_reqwest_error(&e)))?; let body: serde_json::Value = resp .json() @@ -223,44 +226,110 @@ async fn fetch_download_url() -> Result<(String, String), String> { Ok((version, url)) } +fn format_reqwest_error(e: &reqwest::Error) -> String { + let mut msg = e.to_string(); + let mut source = std::error::Error::source(e); + while let Some(cause) = source { + msg.push_str(&format!(": {}", cause)); + source = std::error::Error::source(cause); + } + msg +} + +fn http_client() -> Result { + reqwest::Client::builder() + .user_agent(format!("agent-browser/{}", env!("CARGO_PKG_VERSION"))) + .timeout(std::time::Duration::from_secs(120)) + .connect_timeout(std::time::Duration::from_secs(30)) + .build() + .map_err(|e| format!("Failed to create HTTP client: {}", format_reqwest_error(&e))) +} + async fn download_bytes(url: &str) -> Result, String> { - let resp = reqwest::get(url) - .await - .map_err(|e| format!("Download failed: {}", e))?; + let client = http_client()?; + let max_retries = 3; + let mut last_err = String::new(); - let total = resp.content_length(); - let mut bytes = Vec::new(); - let mut stream = resp; - let mut downloaded: u64 = 0; - let mut last_pct: u64 = 0; + for attempt in 0..max_retries { + if attempt > 0 { + eprintln!( + " Retrying download (attempt {}/{})", + attempt + 1, + max_retries + ); + tokio::time::sleep(std::time::Duration::from_secs(1 << attempt)).await; + } - loop { - let chunk = stream - .chunk() - .await - .map_err(|e| format!("Download error: {}", e))?; - match chunk { - Some(data) => { - downloaded += data.len() as u64; - bytes.extend_from_slice(&data); + let resp = match client.get(url).send().await { + Ok(r) => r, + Err(e) => { + last_err = format!("Download failed: {}", format_reqwest_error(&e)); + if e.is_connect() || e.is_timeout() { + continue; + } + return Err(last_err); + } + }; - if let Some(total) = total { - let pct = (downloaded * 100) / total; - if pct >= last_pct + 5 { - last_pct = pct; - let mb = downloaded as f64 / 1_048_576.0; - let total_mb = total as f64 / 1_048_576.0; - eprint!("\r {:.0}/{:.0} MB ({pct}%)", mb, total_mb); - let _ = io::stderr().flush(); + let status = resp.status(); + if !status.is_success() { + last_err = format!( + "Download failed: server returned HTTP {} for {}", + status, url + ); + if status.is_server_error() { + continue; + } + return Err(last_err); + } + + let total = resp.content_length(); + let mut bytes = Vec::new(); + let mut stream = resp; + let mut downloaded: u64 = 0; + let mut last_pct: u64 = 0; + + let mut chunk_err = None; + loop { + let chunk = stream + .chunk() + .await + .map_err(|e| format!("Download error: {}", format_reqwest_error(&e))); + match chunk { + Ok(Some(data)) => { + downloaded += data.len() as u64; + bytes.extend_from_slice(&data); + + if let Some(total) = total { + let pct = (downloaded * 100) / total; + if pct >= last_pct + 5 { + last_pct = pct; + let mb = downloaded as f64 / 1_048_576.0; + let total_mb = total as f64 / 1_048_576.0; + eprint!("\r {:.0}/{:.0} MB ({pct}%)", mb, total_mb); + let _ = io::stderr().flush(); + } } } + Ok(None) => break, + Err(e) => { + chunk_err = Some(e); + break; + } } - None => break, } + + eprintln!(); + + if let Some(e) = chunk_err { + last_err = e; + continue; + } + + return Ok(bytes); } - eprintln!(); - Ok(bytes) + Err(last_err) } fn extract_zip(bytes: Vec, dest: &Path) -> Result<(), String> { @@ -780,6 +849,193 @@ pub fn run_dashboard_install() { } } +#[cfg(test)] +mod tests { + use super::*; + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + use tokio::net::TcpListener; + + fn http_response(status: u16, reason: &str, body: &[u8]) -> Vec { + let header = format!( + "HTTP/1.1 {} {}\r\nContent-Length: {}\r\nConnection: close\r\n\r\n", + status, + reason, + body.len() + ); + let mut resp = header.into_bytes(); + resp.extend_from_slice(body); + resp + } + + async fn accept_once(listener: &TcpListener, response: &[u8]) { + let (mut s, _) = listener.accept().await.unwrap(); + let mut buf = [0u8; 4096]; + let _ = s.read(&mut buf).await; + s.write_all(response).await.unwrap(); + } + + async fn accept_with_ua_check(listener: &TcpListener, response: &[u8]) -> String { + let (mut s, _) = listener.accept().await.unwrap(); + let mut buf = [0u8; 4096]; + let n = s.read(&mut buf).await.unwrap(); + let request = String::from_utf8_lossy(&buf[..n]).to_string(); + s.write_all(response).await.unwrap(); + request + } + + #[tokio::test] + async fn download_bytes_returns_body_on_200() { + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let port = listener.local_addr().unwrap().port(); + let body = b"fake-zip-content"; + let resp = http_response(200, "OK", body); + + let server = tokio::spawn(async move { + accept_once(&listener, &resp).await; + }); + + let url = format!("http://127.0.0.1:{}/test.zip", port); + let result = download_bytes(&url).await; + assert!(result.is_ok()); + assert_eq!(result.unwrap(), body); + server.await.unwrap(); + } + + #[tokio::test] + async fn download_bytes_returns_error_on_404() { + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let port = listener.local_addr().unwrap().port(); + let resp = http_response(404, "Not Found", b"not found"); + + let server = tokio::spawn(async move { + accept_once(&listener, &resp).await; + }); + + let url = format!("http://127.0.0.1:{}/test.zip", port); + let result = download_bytes(&url).await; + assert!(result.is_err()); + let err = result.unwrap_err(); + assert!( + err.contains("HTTP 404"), + "expected HTTP 404 in error, got: {}", + err + ); + server.await.unwrap(); + } + + #[tokio::test] + async fn download_bytes_retries_on_500() { + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let port = listener.local_addr().unwrap().port(); + + let server = tokio::spawn(async move { + // First two attempts: 500 + let r500 = http_response(500, "Internal Server Error", b"error"); + accept_once(&listener, &r500).await; + accept_once(&listener, &r500).await; + // Third attempt: 200 + let r200 = http_response(200, "OK", b"ok-data"); + accept_once(&listener, &r200).await; + }); + + let url = format!("http://127.0.0.1:{}/test.zip", port); + let result = download_bytes(&url).await; + assert!( + result.is_ok(), + "expected success after retries: {:?}", + result + ); + assert_eq!(result.unwrap(), b"ok-data"); + server.await.unwrap(); + } + + #[tokio::test] + async fn download_bytes_gives_up_after_max_retries() { + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let port = listener.local_addr().unwrap().port(); + + let server = tokio::spawn(async move { + let r500 = http_response(500, "Internal Server Error", b"error"); + // All 3 attempts get 500 + accept_once(&listener, &r500).await; + accept_once(&listener, &r500).await; + accept_once(&listener, &r500).await; + }); + + let url = format!("http://127.0.0.1:{}/test.zip", port); + let result = download_bytes(&url).await; + assert!(result.is_err()); + let err = result.unwrap_err(); + assert!( + err.contains("HTTP 500"), + "expected HTTP 500 in error, got: {}", + err + ); + server.await.unwrap(); + } + + #[tokio::test] + async fn download_bytes_does_not_retry_on_403() { + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let port = listener.local_addr().unwrap().port(); + let resp = http_response(403, "Forbidden", b"forbidden"); + + let server = tokio::spawn(async move { + // Only one request should arrive (no retries for 4xx) + accept_once(&listener, &resp).await; + }); + + let url = format!("http://127.0.0.1:{}/test.zip", port); + let result = download_bytes(&url).await; + assert!(result.is_err()); + assert!(result.unwrap_err().contains("HTTP 403")); + server.await.unwrap(); + } + + #[tokio::test] + async fn http_client_sends_user_agent() { + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let port = listener.local_addr().unwrap().port(); + let resp = http_response(200, "OK", b"ok"); + + let server = tokio::spawn(async move { + let req = accept_with_ua_check(&listener, &resp).await; + req + }); + + let client = http_client().unwrap(); + let url = format!("http://127.0.0.1:{}/test", port); + let _ = client.get(&url).send().await; + let request_text = server.await.unwrap(); + let expected_ua = format!("agent-browser/{}", env!("CARGO_PKG_VERSION")); + assert!( + request_text.contains(&expected_ua), + "expected User-Agent '{}' in request:\n{}", + expected_ua, + request_text + ); + } + + #[test] + fn download_bytes_connection_refused_includes_details() { + // Use a port that nothing is listening on + let rt = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .unwrap(); + let result = rt.block_on(download_bytes("http://127.0.0.1:1/test.zip")); + assert!(result.is_err()); + let err = result.unwrap_err(); + // The new code should include the root cause (connection refused) + // not just the vague "error sending request for url" + assert!( + err.contains("Connection refused") || err.contains("connection refused"), + "expected 'connection refused' in error, got: {}", + err + ); + } +} + fn extract_dashboard_zip(bytes: Vec, dest: &Path) -> Result<(), String> { fs::create_dir_all(dest).map_err(|e| format!("Failed to create directory: {}", e))?;