Classify weather-fetch failures with a typed exception (#44)

Rate-limit handling crossed the climate→app boundary as prose: climate
raised RuntimeError(cooldown text), and app._weather_fetch_error re-parsed
it by keyword ('429'/'rate-limited'/'daily'/'tomorrow') while also calling
climate's private _is_rate_limit/_rate_limit_reason — and the user-facing
daily-quota copy existed verbatim in both modules.

climate now raises WeatherUnavailable (a RuntimeError subclass carrying
the user-facing text and a daily flag): from _load_history when both
sources fail with no stale cache, and from the forecast fetch on a 429.
limit_message() is the single home of the rate-limit copy; is_rate_limit
is public for the one remaining raw-429 fallback. app maps the typed
error to 503 by isinstance — no message parsing, no private imports.

The burst-limit copy is unified on 'The weather service is rate-limited…'
(the archive path previously said 'weather archive' internally but the
API always rewrote it; user-visible text is unchanged).

Tests: daily-quota 503 carries the 'tomorrow' copy, an unclassified raw
429 still maps to a clean 503, and a genuine fault stays a 502 with the
raw error.

app.py

@@ -29,21 +29,15 @@ BASE = "/" + os.environ.get("THERMOGRAPH_BASE", "/thermograph").strip("/")
 
 
 def _weather_fetch_error(e) -> HTTPException:
-    """A clean, retryable message for a rate limit; the raw error otherwise."""
-    low = str(e).lower()
-    reason = (climate._rate_limit_reason(e) or "").lower()
-    blob = low + " " + reason
-    if climate._is_rate_limit(e) or "429" in low or "rate-limited" in low or "limit" in reason:
-        if "daily" in blob or "tomorrow" in blob:   # daily quota exhausted — resets tomorrow
-            return HTTPException(
-                status_code=503,
-                detail="Open-Meteo's daily request limit is exhausted — new locations will work again "
-                       "tomorrow. Places you've already viewed still work.",
-            )
-        return HTTPException(
-            status_code=503,
-            detail="The weather service is rate-limited right now — please try again in a minute.",
-        )
+    """A clean, retryable 503 when weather data is temporarily unavailable; the
+    raw error as a 502 otherwise. The fetch layer classifies its own failures
+    (climate.WeatherUnavailable carries the user-facing text) so no message
+    parsing happens here; the is_rate_limit fallback covers a raw upstream 429
+    from a fetcher that didn't classify it."""
+    if isinstance(e, climate.WeatherUnavailable):
+        return HTTPException(status_code=503, detail=str(e))
+    if climate.is_rate_limit(e):
+        return HTTPException(status_code=503, detail=climate.limit_message(False))
     return HTTPException(status_code=502, detail=f"weather data fetch failed: {e}")
 
 

climate.py

@@ -74,7 +74,27 @@ _archive_cooldown_until = 0.0
 _archive_limit_daily = False   # is the current cooldown a daily-quota exhaustion?
 
 
-def _is_rate_limit(e) -> bool:
+class WeatherUnavailable(RuntimeError):
+    """Weather data can't be fetched right now (rate limit, upstream outage with
+    no cached fallback). Carries user-facing text; ``daily`` marks a daily-quota
+    exhaustion (resets after UTC midnight) rather than a transient burst limit.
+    The API layer maps this to a retryable 503 — it never needs to parse the
+    message, and new upstream sources classify their own failures here."""
+
+    def __init__(self, message: str, daily: bool = False):
+        super().__init__(message)
+        self.daily = daily
+
+
+def limit_message(daily: bool) -> str:
+    """The user-facing rate-limit copy, in one place."""
+    if daily:
+        return ("Open-Meteo's daily request limit is exhausted — new locations will work again "
+                "tomorrow. Places you've already viewed still work.")
+    return "The weather service is rate-limited right now — please try again in a minute."
+
+
+def is_rate_limit(e) -> bool:
     return getattr(getattr(e, "response", None), "status_code", None) == 429
 
 
@@ -107,13 +127,6 @@ def _note_rate_limit(e) -> None:
         _seconds_to_utc_reset() if _archive_limit_daily else ARCHIVE_COOLDOWN)
 
 
-def _cooldown_message() -> str:
-    if _archive_limit_daily:
-        return ("Open-Meteo's daily request limit is exhausted — new locations will work again "
-                "tomorrow. Places you've already viewed still work.")
-    return "The weather archive is rate-limited right now — please try again in a minute."
-
-
 def _cell_lock(cell_id: str) -> threading.Lock:
     with _LOCKS_GUARD:
         lk = _CELL_LOCKS.get(cell_id)
@@ -345,7 +358,7 @@ def _topup_tail(cell: dict, df: pd.DataFrame, path: str) -> pd.DataFrame:
             recent = _fetch_history_range(
                 cell, (cached_max + datetime.timedelta(days=1)).isoformat(), expected.isoformat())
         except Exception as e:  # noqa: BLE001 - keep the cached record on failure
-            if _is_rate_limit(e):
+            if is_rate_limit(e):
                 _note_rate_limit(e)
             return df
         merged = (pd.concat([df, recent.drop(columns=["doy"], errors="ignore")])
@@ -419,7 +432,7 @@ def _load_history(cell: dict) -> tuple[pd.DataFrame, dict]:
                 df = _fetch_history(cell)
                 source = "open-meteo"
             except Exception as e:  # noqa: BLE001
-                if _is_rate_limit(e):
+                if is_rate_limit(e):
                     _note_rate_limit(e)
         if df is None:
             try:
@@ -430,7 +443,8 @@ def _load_history(cell: dict) -> tuple[pd.DataFrame, dict]:
         if df is None:
             if stale is not None:
                 return _serve_stale()
-            raise RuntimeError(_cooldown_message())
+            raise WeatherUnavailable(limit_message(_archive_limit_daily),
+                                     daily=_archive_limit_daily)
 
         os.makedirs(CACHE_DIR, exist_ok=True)
         # Store the raw record; percentiles are derived at request time.
@@ -490,7 +504,15 @@ def _load_recent_forecast(cell: dict) -> pd.DataFrame:
         "past_days": RECENT_PAST_DAYS,
         "forecast_days": FORECAST_DAYS,
     }
-    r = _request(FORECAST_URL, params, 60, phase="recent_forecast_fetch")
+    try:
+        r = _request(FORECAST_URL, params, 60, phase="recent_forecast_fetch")
+    except Exception as e:  # noqa: BLE001
+        # Classify a forecast-API rate limit here so callers get the typed error
+        # (the archive path classifies its own inside _load_history).
+        if is_rate_limit(e):
+            daily = "daily" in _rate_limit_reason(e).lower()
+            raise WeatherUnavailable(limit_message(daily), daily=daily) from e
+        raise
     df = _to_frame(r.json()["daily"])
     os.makedirs(CACHE_DIR, exist_ok=True)
     df.drop(columns=["doy"]).to_parquet(path, compression="zstd", index=False)

tests/test_api.py

@@ -76,7 +76,7 @@ def test_day_detail_and_ladders(client, history):
 @pytest.mark.parametrize("path", ["grade", "calendar", "day", "forecast", "cell"])
 def test_every_data_route_maps_rate_limit_to_503(client, monkeypatch, path):
     def rate_limited(cell):
-        raise RuntimeError("Open-Meteo request failed (429): rate-limited")
+        raise climate.WeatherUnavailable(climate.limit_message(False))
     monkeypatch.setattr(climate, "get_history", rate_limited)
     # A distinct spot per route: a store row cached by an earlier test would
     # otherwise be checked only after the (failing) history fetch anyway.
@@ -85,6 +85,32 @@ def test_every_data_route_maps_rate_limit_to_503(client, monkeypatch, path):
     assert "rate-limited" in r.json()["detail"]
 
 
+def test_upstream_failure_classification(client, monkeypatch):
+    q = {"lat": 52.52, "lon": 13.4}
+
+    def daily_quota(cell):
+        raise climate.WeatherUnavailable(climate.limit_message(True), daily=True)
+    monkeypatch.setattr(climate, "get_history", daily_quota)
+    r = client.get("/thermograph/api/v2/grade", params=q)
+    assert r.status_code == 503 and "tomorrow" in r.json()["detail"]
+
+    # A raw upstream 429 that no fetcher classified still maps to a clean 503.
+    def raw_429(cell):
+        e = RuntimeError("upstream said no")
+        e.response = type("R", (), {"status_code": 429})()
+        raise e
+    monkeypatch.setattr(climate, "get_history", raw_429)
+    r = client.get("/thermograph/api/v2/grade", params=q)
+    assert r.status_code == 503 and "rate-limited" in r.json()["detail"]
+
+    # Anything else is a genuine upstream fault: 502 with the raw error.
+    def boom(cell):
+        raise RuntimeError("parquet cache corrupted")
+    monkeypatch.setattr(climate, "get_history", boom)
+    r = client.get("/thermograph/api/v2/grade", params=q)
+    assert r.status_code == 502 and "parquet cache corrupted" in r.json()["detail"]
+
+
 def test_cell_prefetch_never_fetches_upstream(client, monkeypatch):
     def boom(cell):
         raise AssertionError("prefetch=1 must never fetch weather upstream")