Stop recording exceptions for gateway client errors

changelog.d/962.changed.md

@@ -0,0 +1,3 @@
+The gateway no longer records an exception, which `policyengine-observability` writes at ERROR severity, for a request it answers with a 400 or 404 client error: a `ValueError` from route resolution on `POST /simulate/economy/comparison` or `POST /simulate/economy/budget-window`, such as the unknown country sent by the authentication test that each deployment runs, an unknown job ID on `GET /jobs/{job_id}`, an unknown batch ID on `GET /budget-window-jobs/{batch_job_id}`, or an unknown kind on `GET /versions/{kind}`. Status codes and response bodies are unchanged, and each request's `request.completed` record still reports the response with outcome `client_error`. Server errors, and the 404 for a job whose gateway metadata exists but whose Modal call is missing or whose output has expired, still record their exceptions.
+
+On both economy submission routes, a `ValueError` or `HuggingFaceDatasetReferenceError` without an SPM error code, raised while describing the route's bundle or resolving its SPM settings, now returns 500 instead of 400. Only bad data in the deployed bundle manifest can raise it. The gateway logs the exception with its traceback and records it as a handled server error, and the response has `status: "failed"` and the redacted error `Simulation failed (correlation_id=...)` instead of the exception text. A coded SPM error still returns 400 with structured `errors`. Both routes declare the 500 in the gateway's OpenAPI document and in the entrypoint's, so the generated client's two submission functions handle it as a declared status.

projects/policyengine-simulation-entry/src/policyengine_simulation_entry/app.py

@@ -329,6 +329,9 @@ def _register_simulation_routes(
                 "description": "Invalid request or SPM selection",
                 "model": SimulationErrorResponse,
             },
+            500: {
+                "description": "Server failure (redacted error with a correlation ID)"
+            },
         },
         dependencies=[Depends(submitter)],
     )
@@ -368,6 +371,9 @@ async def submit_comparison(
                 "description": "Invalid request or SPM selection",
                 "model": SimulationErrorResponse,
             },
+            500: {
+                "description": "Server failure (redacted error with a correlation ID)"
+            },
         },
         dependencies=[Depends(submitter)],
     )

projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/generate_openapi.py

@@ -40,6 +40,9 @@ def create_openapi_app() -> FastAPI:
         responses={
             200: {"description": "Job submitted successfully"},
             400: {"description": "Invalid request (unknown country/version)"},
+            500: {
+                "description": "Server failure (redacted error with a correlation ID)"
+            },
         },
     )
     async def submit_simulation(request: SimulationRequest) -> JobSubmitResponse:
@@ -56,6 +59,9 @@ async def submit_simulation(request: SimulationRequest) -> JobSubmitResponse:
         responses={
             200: {"description": "Budget-window batch submitted successfully"},
             400: {"description": "Invalid request (unknown country/version/year)"},
+            500: {
+                "description": "Server failure (redacted error with a correlation ID)"
+            },
         },
     )
     async def submit_budget_window_batch(

projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/job_creation_routes.py

@@ -35,6 +35,7 @@
     resolve_spm_selection,
     spm_error_detail,
 )
+from policyengine_simulation_observability.errors import log_and_redact_exception
 from policyengine_simulation_observability.identifiers import (
     OBSERVABILITY_ID_HEADER,
     resolve_observability_id,
@@ -226,7 +227,6 @@ async def submit_simulation(
             )
         _require_route_serves_breakdown(request, route)
     except ValueError as e:
-        runtime.record_exception(e, handled=True, status_code=400)
         raise HTTPException(status_code=400, detail=str(e)) from e
 
     runtime.set_context(
@@ -259,8 +259,20 @@ async def submit_simulation(
                 errors=[detail.model_dump()],
                 headers=observability_headers(observability_id),
             )
-        runtime.record_exception(exc, handled=True, status_code=400)
-        raise HTTPException(status_code=400, detail=str(exc)) from exc
+        # No request input reaches this point: the country and version are
+        # already checked, and every request-driven SPM error has a code. The
+        # error comes from the deployed bundle manifest, so it is a server
+        # failure, and the caller gets a redacted message.
+        redacted_error = log_and_redact_exception(
+            exc,
+            runtime=runtime,
+            scope="simulation_submission",
+            context={"resolved_app_name": route.app_name},
+        )
+        return failed_job_response(
+            error=redacted_error,
+            headers=observability_headers(observability_id),
+        )
 
     if request.spm is not None:
         payload["spm"] = request.spm.model_dump(mode="json")
@@ -336,7 +348,6 @@ async def submit_budget_window_batch(
                 request.policyengine_version,
             )
     except ValueError as e:
-        runtime.record_exception(e, handled=True, status_code=400)
         raise HTTPException(status_code=400, detail=str(e)) from e
 
     runtime.set_context(
@@ -360,8 +371,20 @@ async def submit_budget_window_batch(
                 errors=[detail.model_dump()],
                 headers=observability_headers(observability_id),
             )
-        runtime.record_exception(exc, handled=True, status_code=400)
-        raise HTTPException(status_code=400, detail=str(exc)) from exc
+        # No request input reaches this point: the country and version are
+        # already checked, and every request-driven SPM error has a code. The
+        # error comes from the deployed bundle manifest, so it is a server
+        # failure, and the caller gets a redacted message.
+        redacted_error = log_and_redact_exception(
+            exc,
+            runtime=runtime,
+            scope="budget_window_submission",
+            context={"resolved_app_name": route.app_name},
+        )
+        return failed_job_response(
+            error=redacted_error,
+            headers=observability_headers(observability_id),
+        )
     with runtime.span(BUDGET_WINDOW_STAGES.name(Stage.REQUEST_PARSE)):
         payload = _build_budget_window_parent_payload(
             request,

projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/job_status_routes.py

@@ -46,7 +46,6 @@
     logger,
     observability_headers,
     observability_runtime,
-    record_not_found,
     router,
 )
 
@@ -122,7 +121,6 @@ async def get_job_status(
     with runtime.span(ANNUAL_IMPACT_STAGES.name(Stage.MODAL_JOB_METADATA_READ)):
         job_metadata = job_metadata_store().get(job_id)
     if job_metadata is None:
-        record_not_found(runtime, f"Job not found: {job_id}")
         raise HTTPException(status_code=404, detail=f"Job not found: {job_id}")
     observability_id = bind_observability_id(
         request,
@@ -226,7 +224,6 @@ async def get_budget_window_job_status(
     with runtime.span(BUDGET_WINDOW_STAGES.name(Stage.BUDGET_WINDOW_STATE_LOAD)):
         seed_state = get_batch_job_seed(batch_job_id)
     if seed_state is None:
-        record_not_found(runtime, f"Budget-window job not found: {batch_job_id}")
         raise HTTPException(
             status_code=404, detail=f"Budget-window job not found: {batch_job_id}"
         )

projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/routes_common.py

@@ -25,11 +25,6 @@ def observability_runtime(request: Request) -> ObservabilityRuntime:
     return runtime
 
 
-def record_not_found(runtime: ObservabilityRuntime, message: str) -> None:
-    """Record a handled not-found lookup (status 404) on the request's telemetry."""
-    runtime.record_exception(LookupError(message), handled=True, status_code=404)
-
-
 def bind_observability_id(
     http_request: Request,
     runtime: ObservabilityRuntime,

projects/policyengine-simulation-gateway/src/policyengine_simulation_gateway/version_routes.py

@@ -29,7 +29,6 @@
 from policyengine_simulation_gateway.routes_common import (
     logger,
     observability_runtime,
-    record_not_found,
     router,
 )
 
@@ -92,7 +91,6 @@ async def get_country_versions(kind: str, request: Request) -> VersionMap:
     runtime = observability_runtime(request)
     kind_lower = kind.lower()
     if kind_lower not in SUPPORTED_ROUTE_KINDS:
-        record_not_found(runtime, f"Unknown version kind: {kind}")
         raise HTTPException(status_code=404, detail=f"Unknown version kind: {kind}")
 
     with runtime.span(ANNUAL_IMPACT_STAGES.name(Stage.ROUTE_RESOLUTION)):

projects/policyengine-simulation-gateway/tests/golden/openapi.json

@@ -35,6 +35,9 @@
           "400": {
             "description": "Invalid request (unknown country/version)"
           },
+          "500": {
+            "description": "Server failure (redacted error with a correlation ID)"
+          },
           "422": {
             "description": "Validation Error",
             "content": {
@@ -77,6 +80,9 @@
           "400": {
             "description": "Invalid request (unknown country/version/year)"
           },
+          "500": {
+            "description": "Server failure (redacted error with a correlation ID)"
+          },
           "422": {
             "description": "Validation Error",
             "content": {

projects/policyengine-simulation-gateway/tests/test_budget_window_endpoints.py

@@ -135,7 +135,7 @@ def test__given_budget_window_submission__then_returns_parent_batch_job_id(
         }
 
     @pytest.mark.usefixtures("mock_modal")
-    def test__given_unknown_budget_window_job__then_records_handled_404(
+    def test__given_unknown_budget_window_job__then_404_without_exception(
         self, client: TestClient, monkeypatch
     ):
         recorded_errors = []
@@ -151,13 +151,7 @@ def test__given_unknown_budget_window_job__then_records_handled_404(
         assert response.json()["detail"] == (
             "Budget-window job not found: missing-batch"
         )
-        assert str(recorded_errors[0][0]) == (
-            "Budget-window job not found: missing-batch"
-        )
-        assert recorded_errors[0][1] == {
-            "handled": True,
-            "status_code": 404,
-        }
+        assert recorded_errors == []
 
     def test__given_parent_lookup_failure__then_degraded_poll_is_not_an_error(
         self, mock_modal, client: TestClient, monkeypatch

projects/policyengine-simulation-gateway/tests/test_endpoints.py

@@ -226,7 +226,7 @@ def test__given_active_state__then_policyengine_versions_endpoint_works(
         assert response.json()["latest"] == "4.10.0"
 
     @pytest.mark.usefixtures("mock_modal")
-    def test__given_unknown_version_kind__then_records_handled_404(
+    def test__given_unknown_version_kind__then_404_without_exception(
         self, client: TestClient, monkeypatch
     ):
         recorded_errors = []
@@ -240,11 +240,7 @@ def test__given_unknown_version_kind__then_records_handled_404(
 
         assert response.status_code == 404
         assert response.json()["detail"] == "Unknown version kind: fr"
-        assert str(recorded_errors[0][0]) == "Unknown version kind: fr"
-        assert recorded_errors[0][1] == {
-            "handled": True,
-            "status_code": 404,
-        }
+        assert recorded_errors == []
 
     def test__given_no_active_state__then_versions_fall_back_to_old_dicts(
         self, mock_modal, client: TestClient

projects/policyengine-simulation-gateway/tests/test_exception_records.py

@@ -0,0 +1,158 @@
+"""Client errors the gateway answers with 4xx record no exception; 5xx still do.
+
+``ObservabilityRuntime.record_exception`` always writes an ERROR record. A
+request the gateway answers with a 4xx already gets a ``request.completed``
+record whose outcome is ``client_error``, so an exception record for it adds
+only a false alarm to the daily error report. A server failure (5xx) keeps
+its exception record.
+
+An uncoded error while describing the route's bundle comes from the deployed
+bundle manifest, not from the request, so it is a server failure.
+"""
+
+import re
+from typing import NamedTuple
+
+import pytest
+from fastapi.testclient import TestClient
+from policyengine_simulation_contract.hf_dataset import HuggingFaceDatasetReferenceError
+from policyengine_simulation_gateway import job_creation_routes
+from policyengine_simulation_observability.identifiers import OBSERVABILITY_ID_HEADER
+
+# The country the deploy pipeline's authentication test sends, expecting 400.
+AUTHENTICATION_PROBE_COUNTRY = "__authentication_probe__"
+
+
+class Submission(NamedTuple):
+    """A submission endpoint and the request fields it needs besides country."""
+
+    path: str
+    request_fields: dict[str, object]
+
+
+SUBMISSIONS = [
+    pytest.param(
+        Submission(
+            path="/simulate/economy/comparison",
+            request_fields={"scope": "macro", "reform": {}},
+        ),
+        id="comparison",
+    ),
+    pytest.param(
+        Submission(
+            path="/simulate/economy/budget-window",
+            request_fields={
+                "region": "us",
+                "scope": "macro",
+                "reform": {},
+                "start_year": "2026",
+                "window_size": 2,
+            },
+        ),
+        id="budget_window",
+    ),
+]
+
+RecordedException = tuple[BaseException, dict[str, object]]
+
+
+@pytest.fixture
+def recorded_exceptions(
+    client: TestClient, monkeypatch: pytest.MonkeyPatch
+) -> list[RecordedException]:
+    """Replace the runtime's ``record_exception`` with one that keeps each call."""
+    recorded: list[RecordedException] = []
+
+    def record_exception(error: BaseException, **keyword_arguments: object) -> None:
+        recorded.append((error, keyword_arguments))
+
+    monkeypatch.setattr(
+        client.app.state.policyengine_observability,
+        "record_exception",
+        record_exception,
+    )
+    return recorded
+
+
+@pytest.mark.parametrize("submission", SUBMISSIONS)
+def test__given_unknown_country__then_returns_400_without_exception_record(
+    mock_modal,
+    client: TestClient,
+    recorded_exceptions: list[RecordedException],
+    submission: Submission,
+):
+    response = client.post(
+        submission.path,
+        json={"country": AUTHENTICATION_PROBE_COUNTRY, **submission.request_fields},
+    )
+
+    assert response.status_code == 400
+    assert response.json() == {
+        "detail": f"Unknown country: {AUTHENTICATION_PROBE_COUNTRY}"
+    }
+    assert recorded_exceptions == []
+    assert mock_modal["func"].calls == []
+
+
+@pytest.mark.parametrize("submission", SUBMISSIONS)
+def test__given_bundle_lookup_error__then_records_it_and_returns_redacted_500(
+    mock_modal,
+    client: TestClient,
+    recorded_exceptions: list[RecordedException],
+    monkeypatch: pytest.MonkeyPatch,
+    submission: Submission,
+):
+    malformed_dataset_reference = "hf://policyengine/internal-repo/missing.h5"
+    manifest_error = HuggingFaceDatasetReferenceError(
+        f"Malformed dataset reference in bundle manifest: {malformed_dataset_reference}"
+    )
+
+    def reject_bundle_lookup(*_arguments: object) -> None:
+        raise manifest_error
+
+    monkeypatch.setattr(
+        job_creation_routes, "build_policyengine_bundle", reject_bundle_lookup
+    )
+
+    response = client.post(
+        submission.path, json={"country": "us", **submission.request_fields}
+    )
+
+    assert response.status_code == 500
+    body = response.json()
+    assert body["status"] == "failed"
+    assert body["result"] is None
+    assert "errors" not in body
+    assert re.fullmatch(
+        r"Simulation failed \(correlation_id=[0-9a-f]{32}\)", body["error"]
+    )
+    assert malformed_dataset_reference not in response.text
+    assert OBSERVABILITY_ID_HEADER in response.headers
+    assert recorded_exceptions == [
+        (manifest_error, {"handled": True, "status_code": 500}),
+    ]
+    assert mock_modal["func"].calls == []
+
+
+def test__given_modal_lookup_failure__then_job_status_records_server_exception(
+    mock_modal,
+    client: TestClient,
+    recorded_exceptions: list[RecordedException],
+):
+    submit_response = client.post(
+        "/simulate/economy/comparison",
+        json={"country": "us", "scope": "macro", "reform": {}},
+    )
+    job_id = submit_response.json()["job_id"]
+    lookup_failure = RuntimeError("Modal control plane unavailable")
+    mock_modal["function_call"].from_id_errors[job_id] = lookup_failure
+    client_receiving_server_errors = TestClient(
+        client.app, raise_server_exceptions=False
+    )
+
+    response = client_receiving_server_errors.get(f"/jobs/{job_id}")
+
+    assert response.status_code == 500
+    assert recorded_exceptions == [
+        (lookup_failure, {"handled": False, "status_code": 500}),
+    ]