understand server 404s

src/app.rs

@@ -547,7 +547,7 @@ async fn fetch_weather(units: Option<Units>) -> 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<Units>) -> 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<String, String> {
+    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<Units>) -> Result<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();
 
     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}");
+    }
+}

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<Units> {
-    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<Option<Units>> {
+/// 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<Units> {
     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<Option<Units>> {
                 "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<Units> {
     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<Units> {
     let trimmed = value.trim();
     if trimmed.is_empty() || trimmed == "C" || trimmed == "POSIX" {