From 0b1a710ffa4b59ca7d335738d47df8c22925a7f6 Mon Sep 17 00:00:00 2001 From: Jan-Peter von Hunnius Date: Tue, 29 Sep 2026 09:25:47 +0200 Subject: [PATCH] Retry transient 400s with body logging; garage reserve default 27 --- README.md | 4 ++-- src/commands/mod.rs | 7 ++++-- src/http.rs | 55 ++++++++++++++++++++++++++++++++++++--------- tests/http_tests.rs | 51 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 103 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 0496117..44ecf90 100644 --- a/README.md +++ b/README.md @@ -119,7 +119,7 @@ Generates `trash.csv` — owned cars that are not best on any track, have RQ < 8 trash.csv — Owned cars safe to remove or use as fusion material ``` -The band thresholds derive from a single base value (`--trash-base-count`, default 4); each higher band gets half of the previous one. The tool prints the effective thresholds and a garage-space projection (`user.garageSlots`/`garageSlotsUsed` from the export). When the projected free slots after clean-up fall below `--garage-min-free` (default 20), the base count is raised automatically — and printed — until the target is reached or no more cars qualify: +The band thresholds derive from a single base value (`--trash-base-count`, default 4); each higher band gets half of the previous one. The tool prints the effective thresholds and a garage-space projection (`user.garageSlots`/`garageSlotsUsed` from the export). When the projected free slots after clean-up fall below `--garage-min-free` (default 27), the base count is raised automatically — and printed — until the target is reached or no more cars qualify: ```bash cargo run --bin records -- trash --trash-base-count 5 --garage-min-free 30 @@ -249,7 +249,7 @@ The analyzer supports multiple track conditions: | `--min-count N` | `10` | Minimum number of tracks a car must be best on to qualify as "very best" | | `--max-wishlist M` | `300` | Maximum number of cars on the ideal wishlist | | `--trash-base-count N` | `4` | Maximum improve track count for RQ 10-19 to count as trash; RQ 20-29/30-39 get half/a quarter. `0` disables the criteria | -| `--garage-min-free N` | `20` | Minimum projected free garage slots; the trash base count is raised automatically when the projection falls short | +| `--garage-min-free N` | `27` | Minimum projected free garage slots; the trash base count is raised automatically when the projection falls short | ## 📅 Daily Workflow diff --git a/src/commands/mod.rs b/src/commands/mod.rs index fd6f635..92dfafb 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -32,7 +32,10 @@ use crate::output::{ const DEFAULT_TRASH_BASE_COUNT: usize = 4; /// Default minimum projected free garage slots for trash threshold escalation. -const DEFAULT_GARAGE_MIN_FREE: usize = 20; +/// +/// Sized to keep roughly a dozen trash cars parked in the garage after +/// clean-up plus 15 free slots for new purchases. +const DEFAULT_GARAGE_MIN_FREE: usize = 27; /// CLI parameters for the improve-based trash criteria. #[derive(clap::Args)] @@ -42,7 +45,7 @@ pub struct TrashParams { #[arg(long, default_value_t = DEFAULT_TRASH_BASE_COUNT)] pub trash_base_count: usize, /// Minimum projected free garage slots; the base count is raised - /// automatically when the projection falls short (default: 20) + /// automatically when the projection falls short (default: 27) #[arg(long, default_value_t = DEFAULT_GARAGE_MIN_FREE)] pub garage_min_free: usize, } diff --git a/src/http.rs b/src/http.rs index aca3a3b..22f838a 100644 --- a/src/http.rs +++ b/src/http.rs @@ -14,6 +14,31 @@ fn retry_delay(attempt: usize) -> std::time::Duration { std::time::Duration::from_millis(500 * (attempt as u64 + 1)) } +/// Maximum number of characters from an error-response body to include in a log line. +const MAX_LOGGED_BODY_CHARS: usize = 200; + +/// Consumes the response and formats its body as a single-line log suffix. +/// +/// Returns an empty string when the body is empty or unreadable, so log lines +/// stay clean. The body of an error response often names the actual server-side +/// complaint (rate limiting, malformed request), which status codes alone do not. +async fn log_body_suffix(response: reqwest::Response) -> String { + let body = match response.text().await { + Ok(body) => body, + Err(_) => return String::new(), + }; + let mut body = body.replace(['\n', '\r'], " "); + if body.chars().count() > MAX_LOGGED_BODY_CHARS { + body = body.chars().take(MAX_LOGGED_BODY_CHARS).collect::() + "…"; + } + let body = body.trim(); + if body.is_empty() { + String::new() + } else { + format!(" body: {body}") + } +} + /// Downloads a file from a URL and saves it locally. /// /// Returns `Ok(())` on success, or an error describing what went wrong. @@ -21,13 +46,13 @@ fn retry_delay(attempt: usize) -> std::time::Duration { /// Writes to a temporary file first, then renames on success, so a failed /// download never deletes the existing file. /// -/// Retries up to `MAX_FETCH_RETRIES` times on transient failures (server errors -/// or connection errors). Permanent conditions fail immediately: client HTTP -/// errors (4xx) and empty bodies ([`AppError::EmptyResponse`] — the records API -/// reports "no data submitted yet" as a 200 with an empty body, which no amount -/// of retrying can change). +/// Retries up to `MAX_FETCH_RETRIES` times on transient failures (server errors, +/// HTTP 400, or connection errors). Permanent conditions fail immediately: other +/// client HTTP errors (4xx) and empty bodies ([`AppError::EmptyResponse`] — the +/// records API reports "no data submitted yet" as a 200 with an empty body, +/// which no amount of retrying can change). pub async fn get_file(file_url: &str, local_file_path: &str) -> Result<(), AppError> { - /// Maximum number of retries on transient failures (5xx, connection errors). + /// Maximum number of retries on transient failures (5xx, 400, connection errors). const MAX_FETCH_RETRIES: usize = 3; // Parse the URL once so malformed URLs are caught before any network I/O. @@ -45,17 +70,27 @@ pub async fn get_file(file_url: &str, local_file_path: &str) -> Result<(), AppEr Ok(response) => { let status = response.status(); if !status.is_success() { - // Only retry on server errors (5xx); client errors (4xx) are permanent. - if status.is_server_error() && attempt < MAX_FETCH_RETRIES { + // Server errors and 400s are retried: the records API + // occasionally answers with a transient 400 under load. + // Other client errors (4xx) are permanent. + let retryable = status.is_server_error() || status.as_u16() == 400; + if retryable && attempt < MAX_FETCH_RETRIES { + let body_suffix = log_body_suffix(response).await; log::warn!( - "HTTP {} from {file_url} (attempt {}/{MAX_FETCH_RETRIES}), retrying...", - status, + "HTTP {status} from {file_url} (attempt {}/{MAX_FETCH_RETRIES}), retrying...{body_suffix}", attempt + 1, ); tokio::time::sleep(retry_delay(attempt)).await; attempt += 1; continue; } + if retryable { + let body_suffix = log_body_suffix(response).await; + log::warn!("HTTP {status} from {file_url}, giving up.{body_suffix}"); + return Err(AppError::Network(format!( + "HTTP {status} from {file_url}{body_suffix}" + ))); + } return Err(AppError::Network(format!("HTTP {status} from {file_url}"))); } match response.text().await { diff --git a/tests/http_tests.rs b/tests/http_tests.rs index 34c6b9f..2008b47 100644 --- a/tests/http_tests.rs +++ b/tests/http_tests.rs @@ -69,6 +69,57 @@ async fn test_get_file_server_error_retries() { m_ok.assert(); } +#[tokio::test] +async fn test_get_file_bad_request_retries_then_recovers() { + let mut server = Server::new_async().await; + // The records API occasionally serves transient 400s — return 400 three + // times, then 200. Three requests must hit the 400 mock. + let m_fail = server + .mock("GET", "/data.json") + .with_status(400) + .with_body("upstream overloaded") + .expect(3) + .create(); + let m_ok = mock_success(&mut server, "recovered"); + + let dir = tempfile::tempdir().unwrap(); + let out_path = dir.path().join("output.txt"); + let url = format!("{}/data.json", server.url()); + + get_file(&url, out_path.to_str().unwrap()).await.unwrap(); + + assert_eq!(fs::read_to_string(&out_path).unwrap(), "recovered"); + m_fail.assert(); + m_ok.assert(); +} + +#[tokio::test] +async fn test_get_file_persistent_bad_request_fails_after_retries() { + let mut server = Server::new_async().await; + // 400 every time — should retry 3 times then fail with the body in the message + let m = server + .mock("GET", "/data.json") + .with_status(400) + .with_body("upstream overloaded") + .expect(4) // 1 initial + 3 retries + .create(); + + let dir = tempfile::tempdir().unwrap(); + let out_path = dir.path().join("output.txt"); + let url = format!("{}/data.json", server.url()); + + let result = get_file(&url, out_path.to_str().unwrap()).await; + assert!(result.is_err(), "Persistent 400 should fail after retries"); + let err_msg = format!("{}", result.unwrap_err()); + assert!(err_msg.contains("400"), "Error should mention HTTP 400: {err_msg}"); + assert!( + err_msg.contains("upstream overloaded"), + "Error should carry the response body: {err_msg}" + ); + assert!(!out_path.exists(), "No file should be written on persistent failure"); + m.assert(); +} + #[tokio::test] async fn test_get_file_client_error_fails_immediately() { let mut server = Server::new_async().await;