From 1e38f203f0e98a9ea8ff300ade403efbe7304cac Mon Sep 17 00:00:00 2001 From: Emil Lerch Date: Mon, 26 Oct 2026 11:47:17 -0700 Subject: [PATCH] understand server 404s --- src/app.rs | 61 ++++++++++++++++++++++++++++++++++++++++++++------- src/config.rs | 37 +++++++++++-------------------- 2 files changed, 66 insertions(+), 32 deletions(-) diff --git a/src/app.rs b/src/app.rs index 882e54f..0e91e0f 100644 --- a/src/app.rs +++ b/src/app.rs @@ -547,7 +547,7 @@ async fn fetch_weather(units: Option) -> Result<(String, String), String> .await .map_err(|e| e.to_string())?; - let text = response.text().await.map_err(|e| e.to_string())?; + let text = response_text(response).await?; let text = text.trim(); // Detect captive portal: the weather API returns plain text, not HTML. @@ -555,14 +555,36 @@ async fn fetch_weather(units: Option) -> Result<(String, String), String> return Err("Captive portal detected: received HTML instead of weather data".to_string()); } - // Format: "location|icon|temp" + parse_panel_weather(text) +} + +/// Parse the panel's `location|icon|temp` response into +/// `(location, "icon|temp")`. +/// +/// Anything else is an error rather than something to display: showing an +/// unexpected body verbatim in the panel would also skip the retry path. +fn parse_panel_weather(text: &str) -> Result<(String, String), String> { let parts: Vec<&str> = text.splitn(3, '|').collect(); - if parts.len() == 3 { - let location = parts[0].to_string(); - let weather = format!("{}|{}", parts[1], parts[2]); - Ok((location, weather)) + if let [location, icon, temp] = parts.as_slice() { + Ok(((*location).to_string(), format!("{icon}|{temp}"))) } else { - Ok((String::new(), text.to_string())) + Err(format!("Unexpected weather response format: {text:?}")) + } +} + +/// Read the response body, failing on non-success HTTP status. +/// +/// wttr.in reports failures such as "Location not found (location query +/// resolution failure)" with a 404 and a plain-text body. Without this check +/// that body would be treated as weather data: shown in the panel or cached +/// as the popup forecast, and never retried. +async fn response_text(response: reqwest::Response) -> Result { + let status = response.status(); + let text = response.text().await.map_err(|e| e.to_string())?; + if status.is_success() { + Ok(text) + } else { + Err(format!("HTTP {status}: {}", text.trim())) } } @@ -593,7 +615,7 @@ async fn fetch_full_weather(units: Option) -> Result { .await .map_err(|e| e.to_string())?; - let text = response.text().await.map_err(|e| e.to_string())?; + let text = response_text(response).await?; let text = text.trim(); if looks_like_captive_portal(text) { @@ -677,3 +699,26 @@ fn pad_to_forecast_height(text: &str) -> String { } padded } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parse_panel_weather_splits_fields() { + assert_eq!( + parse_panel_weather("San Francisco, California, United States|☀️|+59.9°F"), + Ok(( + "San Francisco, California, United States".to_string(), + "☀️|+59.9°F".to_string() + )) + ); + } + + #[test] + fn parse_panel_weather_rejects_error_body() { + let err = parse_panel_weather("Location not found (location query resolution failure)") + .unwrap_err(); + assert!(err.contains("Location not found"), "{err}"); + } +} diff --git a/src/config.rs b/src/config.rs index c9bd90b..38a5932 100644 --- a/src/config.rs +++ b/src/config.rs @@ -46,31 +46,21 @@ const USCS_LOCALES: &[&str] = &["en_US", "en_LR", "en_BS", "en_BZ", "en_KY", "en /// client's geolocation. #[must_use] pub fn resolve_units() -> Option { - if let Some(u) = units_from_env() { - return u; - } - units_from_locale() + units_from_env().or_else(units_from_locale) } /// Parse `COSMIC_WEATHER_UNITS`. /// -/// Returns: -/// - `Some(Some(units))` — explicit override. -/// - `Some(None)` — explicit `auto` or empty; caller should still return -/// `None` overall (skip locale detection? No — we want `auto` to mean -/// "fall through to locale", so this returns `None` to signal fall-through). -/// - `None` — env var unset; fall through. -/// -/// To keep the control flow simple we collapse all fall-through cases to -/// a single `None` return and only return `Some(Some(_))` for an explicit -/// unit choice. -fn units_from_env() -> Option> { +/// Returns `Some(units)` only for an explicit unit choice. An unset, empty, +/// `auto`, or unrecognized value returns `None` so the caller falls through +/// to locale detection. +fn units_from_env() -> Option { let raw = std::env::var("COSMIC_WEATHER_UNITS").ok()?; let trimmed = raw.trim(); if trimmed.is_empty() || trimmed.eq_ignore_ascii_case("auto") { return None; } - let parsed = match trimmed { + match trimmed { "u" | "U" => Some(Units::Uscs), "m" => Some(Units::Metric), "M" => Some(Units::MetricMs), @@ -79,26 +69,25 @@ fn units_from_env() -> Option> { "COSMIC_WEATHER_UNITS={other:?} is not recognized; \ expected one of: u, m, M, auto" ); - return None; + None } - }; - Some(parsed) + } } /// Inspect measurement-related locale env vars and map to `Units`. fn units_from_locale() -> Option { for var in ["LC_MEASUREMENT", "LC_ALL", "LANG"] { - if let Ok(val) = std::env::var(var) { - if let Some(units) = classify_locale(&val) { - return Some(units); - } + if let Ok(val) = std::env::var(var) + && let Some(units) = classify_locale(&val) + { + return Some(units); } } None } /// Strip codeset (`.UTF-8`) and modifier (`@euro`) and check the -/// language_TERRITORY prefix against the USCS list. +/// `language_TERRITORY` prefix against the USCS list. fn classify_locale(value: &str) -> Option { let trimmed = value.trim(); if trimmed.is_empty() || trimmed == "C" || trimmed == "POSIX" {