fix(restapi): a reader missing on the server is a 501, not the caller's fault (#1913)

A deployment installed without the `bufr` extra answered a well-formed
request for a BUFR network with `400 Bad Request` and a body reading
`pip install wetterdienst[bufr]` -- an instruction for a machine the caller
does not administer, about a request they phrased correctly. `interpolate`
and `summarize` were worse: their blanket handler called it a 404, which
reads as "no such network".

The missing half is a property of the deployment, so 501 is the honest
answer: this server does not implement the networks published as BUFR. The
body says that and nothing the caller cannot act on; the install line moves
to the server log, where whoever runs the instance reads it.

This is the other half of #1910, which gave the CLI a handler for
`BufrReaderMissingError` and left the REST API on the blanket one. The MCP
server rides on the same app, so it answers the same way now.

The values endpoint gained a third `except` and tripped `C901`, so its
collection moves into `_values`, beside the `_geo_values` the other two
endpoints already share. Same reason as that one: which failure earns which
status is a decision of its own.

The three new tests fail against the old handlers with 400, 404 and 404.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: gutzbenj

CHANGELOG.md

@@ -31,6 +31,12 @@ Types of changes:
 
 ### Fixed
 
+- REST API: a BUFR reader missing on the server answers 501 rather than 400. The blanket handler
+  read every failure as the caller's, so a deployment installed without the `bufr` extra told the
+  client to `pip install wetterdienst[bufr]` on a machine they do not administer, for a request
+  that was perfectly well formed -- and `interpolate` and `summarize` called the same thing a 404,
+  which reads as "no such network". The install line moves to the server log, where whoever runs
+  the instance can act on it
 - CI: the test and coverage workflows watch `examples/**`. `tests/examples` runs those files, so a
   change to one is a change both suites cover -- and a pull request touching only an example ran
   neither, while the coverage workflow's header said it takes the same inputs as the test matrix

src/wetterdienst/ui/restapi.py

@@ -14,7 +14,12 @@
 from pydantic import ValidationError
 
 from wetterdienst import Author, Info, Settings, Wetterdienst, __version__
-from wetterdienst.exceptions import ApiNotFoundError, NoStationsWithHeightError, StartDateEndDateError
+from wetterdienst.exceptions import (
+    ApiNotFoundError,
+    BufrReaderMissingError,
+    NoStationsWithHeightError,
+    StartDateEndDateError,
+)
 
 # needed at runtime: FastAPI resolves this annotation to build the query parameter's enum
 from wetterdienst.metadata.unit_type import UnitType  # noqa: TC001
@@ -56,7 +61,7 @@
     from collections.abc import Callable
 
     from wetterdienst.model.request import TimeseriesRequest
-    from wetterdienst.model.result import InterpolatedValuesResult, SummarizedValuesResult
+    from wetterdienst.model.result import InterpolatedValuesResult, SummarizedValuesResult, ValuesResult
 
 info = Info()
 
@@ -84,6 +89,28 @@
 }
 
 
+def _reader_missing_on_the_server(e: BufrReaderMissingError, what: str) -> HTTPException:
+    """Report a reader this deployment does not have as the server's lack, not the caller's error.
+
+    The blanket handlers answered it with a 400 carrying `pip install wetterdienst[bufr]` -- an
+    instruction for a machine the caller does not administer, about a request that was perfectly
+    well formed. Over HTTP the missing half is a property of the deployment, and 501 is what says
+    so: this server does not implement the networks published as BUFR. The install line is not
+    lost, it moves to where someone can act on it -- the server log, carried there by the message
+    itself rather than by a traceback, since a dependency that was never installed has no incident
+    to show.
+    """
+    log.error(f"Failed to {what}, this deployment cannot decode BUFR: {e}")
+    return HTTPException(
+        status_code=501,
+        detail=(
+            "This server cannot decode BUFR, which the requested network is published as. The "
+            "request was valid; the deployment is missing the eccodes and pdbufr readers that "
+            "read it. Ask whoever runs this instance to install them."
+        ),
+    )
+
+
 @app.get("/")
 def index() -> HTMLResponse:
     """Provide index page."""
@@ -540,21 +567,7 @@ def values(
         ts_drop_nulls=request.drop_nulls,
     )
 
-    try:
-        values_ = get_values(
-            api=api,
-            request=request,
-            settings=settings,
-        )
-    except StartDateEndDateError as e:
-        log.exception("Failed to get values.")
-        raise HTTPException(
-            status_code=400,
-            detail=str(e),
-        ) from e
-    except Exception as e:
-        log.exception("Failed to get values.")
-        raise HTTPException(status_code=400, detail=str(e)) from e
+    values_ = _values(api=api, request=request, settings=settings)
 
     # build kwargs dynamically
     kwargs: dict[str, Any] = {
@@ -609,6 +622,29 @@ def _geo_settings(
         raise HTTPException(status_code=400, detail=str(e)) from e
 
 
+def _values(
+    api: type[TimeseriesRequest],
+    request: ValuesRequest,
+    settings: Settings,
+) -> ValuesResult:
+    """Collect values, telling the caller which failures are theirs to fix.
+
+    The sibling of `_geo_values` for the plain values endpoint, and lifted out of the endpoint for
+    the same reason: which failure earns which status is a decision of its own, and the endpoint --
+    which also assembles a response format, a target and a set of kwargs -- is not where it belongs.
+    """
+    try:
+        return get_values(api=api, request=request, settings=settings)
+    except StartDateEndDateError as e:
+        log.exception("Failed to get values.")
+        raise HTTPException(status_code=400, detail=str(e)) from e
+    except BufrReaderMissingError as e:
+        raise _reader_missing_on_the_server(e, "get values") from e
+    except Exception as e:
+        log.exception("Failed to get values.")
+        raise HTTPException(status_code=400, detail=str(e)) from e
+
+
 def _geo_values(
     get: Callable[..., InterpolatedValuesResult | SummarizedValuesResult],
     api: type[TimeseriesRequest],
@@ -620,8 +656,9 @@ def _geo_values(
 
     Both endpoints answered every failure with a 404, which reads as "no such thing" for a request
     that was understood and simply cannot be served as phrased -- an elevation no station in reach
-    can be placed against, or a window that ends before it starts. Those are 400s, and the same two
-    in both places, so they are decided here rather than twice over.
+    can be placed against, or a window that ends before it starts. Those are 400s, and a reader
+    missing on the server is a 501; the same three in both places, so they are decided here rather
+    than twice over.
     """
     try:
         return get(api=api, request=request, settings=settings)
@@ -633,6 +670,8 @@ def _geo_values(
     except StartDateEndDateError as e:
         log.exception(f"Failed to {what}")
         raise HTTPException(status_code=400, detail=str(e)) from e
+    except BufrReaderMissingError as e:
+        raise _reader_missing_on_the_server(e, what) from e
     except Exception as e:
         log.exception(f"Failed to {what}")
         raise HTTPException(status_code=404, detail=str(e)) from e

tests/ui/test_restapi.py

@@ -3,6 +3,7 @@
 """Tests for the REST API."""
 
 import json
+import logging
 
 import pytest
 from dirty_equals import IsApprox, IsNumber, IsStr
@@ -2293,3 +2294,80 @@ def unanswerable(**_kwargs: object) -> None:
     )
     assert response.status_code == 400
     assert response.json()["detail"] == msg
+
+
+@pytest.mark.parametrize(
+    ("endpoint", "entry_point", "params"),
+    [
+        (
+            "/api/values",
+            "get_values",
+            {
+                "provider": "dwd",
+                "network": "road",
+                "parameters": "15_minutes/data/temperature_air_mean_2m",
+                "station": "A006",
+                "date": "2024-01-01/2024-01-02",
+            },
+        ),
+        (
+            "/api/interpolate",
+            "get_interpolate",
+            {
+                "provider": "dwd",
+                "network": "road",
+                "parameters": "15_minutes/data/temperature_air_mean_2m",
+                "station": "A006",
+                "date": "2024-01-01",
+            },
+        ),
+        (
+            "/api/summarize",
+            "get_summarize",
+            {
+                "provider": "dwd",
+                "network": "road",
+                "parameters": "15_minutes/data/temperature_air_mean_2m",
+                "station": "A006",
+                "date": "2024-01-01",
+            },
+        ),
+    ],
+)
+def test_a_reader_missing_on_the_server_is_a_501(
+    client: TestClient,
+    monkeypatch: pytest.MonkeyPatch,
+    caplog: pytest.LogCaptureFixture,
+    endpoint: str,
+    entry_point: str,
+    params: dict[str, str],
+) -> None:
+    """A reader the deployment lacks is the server's lack, and it says so with the right status.
+
+    The blanket handler turned it into a 400 -- the caller's fault -- carrying `pip install
+    wetterdienst[bufr]`, which is an instruction for a machine the caller does not administer. The
+    request was well formed; it is this instance that cannot serve it, which is a 501.
+
+    Raised from a stubbed getter rather than by asking DWD for a road station: what is under test
+    is the status and the body, and the real path downloads a station list on the way to the error.
+    """
+    from wetterdienst.exceptions import BufrReaderMissingError  # noqa: PLC0415
+
+    msg = (
+        "DWD road weather data is published as BUFR, which needs eccodes and pdbufr to read: "
+        "`pip install wetterdienst[bufr]` installs both."
+    )
+
+    def refuse(**_kwargs: object) -> None:
+        raise BufrReaderMissingError(msg)
+
+    monkeypatch.setattr(f"wetterdienst.ui.restapi.{entry_point}", refuse)
+    with caplog.at_level(logging.ERROR):
+        response = client.get(endpoint, params=params)
+
+    assert response.status_code == 501
+    detail = response.json()["detail"]
+    assert "eccodes and pdbufr" in detail
+    # the install line is for whoever runs the instance, and reaches them through the log
+    assert "pip install" not in detail
+    assert "pip install wetterdienst[bufr]" in caplog.text