diff --git a/app.py b/app.py index a389ead..4c4bab6 100644 --- a/app.py +++ b/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}") diff --git a/climate.py b/climate.py index da78da3..a992ef7 100644 --- a/climate.py +++ b/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) diff --git a/tests/test_api.py b/tests/test_api.py index 498e8f9..409af38 100644 --- a/tests/test_api.py +++ b/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")