fix(backend): 400 instead of 500 on an unparseable peildatum (RB-16)

BIO-019: GET /stamdata/{table}?peildatum= called DateOnly.Parse directly, which throws FormatException on anything unparseable — an unhandled 500 (leaking exception detail in Development) instead of the 400-with-problem-details every other bad-input check in this file returns. §3c named backend/Stamdata's 71.7% branch coverage (BL-005) as the weak spot this bug lived in. Switched to DateOnly.TryParse; an unparseable value now returns Results.Problem(detail: ..., statusCode: 400), matching the shape the upload/change-request endpoints already use. Endpoint doc gained .ProducesProblem(400), so the OpenAPI doc + generated client were regenerated and committed in this same diff (RB-09's note records a prior incident where a response-shape change shipped without this and the drift went unnoticed). No FE change needed: libs/beheer's stamdata adapter already funnels every call through runSubmit, which folds any thrown ApiException (now including this 400) into a generic Result error — ADR-0001's "the FE renders the decision" already covers "the server rejected this input". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-27 16:53:29 +02:00
parent a93218e8ac
commit 2627799284
5 changed files with 121 additions and 1 deletions
@@ -212,12 +212,24 @@ api.MapGet("/stamdata/{table}", (string table, string? peildatum, HttpContext ct
 {
  var t = StamdataCatalog.Find(table);
  if (t is null) return Results.NotFound();
-  var rows = peildatum is { Length: > 0 } p ? t.RowsOn(DateOnly.Parse(p)) : t.Rows();
+  DateOnly? peildatumWaarde = null;
+  // RB-16/BIO-019: DateOnly.Parse threw FormatException on unparseable input, surfacing as
+  // an unhandled 500 (and, in Development, an exception detail leaked to the caller) — an
+  // admin-gated but still user-supplied string needs the same 400 path every other bad-input
+  // check in this file uses, not a crash.
+  if (peildatum is { Length: > 0 } p)
+  {
+    if (!DateOnly.TryParse(p, out var parsed))
+      return Results.Problem(detail: $"Ongeldige peildatum '{p}'.", statusCode: StatusCodes.Status400BadRequest);
+    peildatumWaarde = parsed;
+  }
+  var rows = peildatumWaarde is { } d ? t.RowsOn(d) : t.Rows();
  return Results.Ok(new StamdataTableDto(t.Id, t.Label, t.Columns.Select(ToColumnDto).ToList(), t.Temporal, rows));
 }))
 .Gate("StamdataAdmin")
 .WithName("stamdataTable")
 .Produces<StamdataTableDto>()
+.ProducesProblem(StatusCodes.Status400BadRequest)
 .ProducesProblem(StatusCodes.Status403Forbidden)
 .Produces(StatusCodes.Status404NotFound);

@@ -194,6 +194,16 @@
              }
            }
          },
+          "400": {
+            "description": "Bad Request",
+            "content": {
+              "application/problem+json": {
+                "schema": {
+                  "$ref": "#/components/schemas/ProblemDetails"
+                }
+              }
+            }
+          },
          "403": {
            "description": "Forbidden",
            "content": {
@@ -59,6 +59,16 @@ public class StamdataEndpointTests(TestWebApplicationFactory factory) : IClassFi
    Assert.Empty(table.Rows);
  }

+  /// RB-16/BIO-019: DateOnly.Parse used to throw FormatException on unparseable input,
+  /// surfacing as an unhandled 500 instead of the 400-with-problem-details every other
+  /// bad-input check in this endpoint file returns.
+  [Fact]
+  public async Task Unparseable_peildatum_is_400_not_500()
+  {
+    var res = await _client.SendAsync(Req(HttpMethod.Get, "/api/v1/stamdata/professions?peildatum=not-a-date", role: "admin"));
+    Assert.Equal(HttpStatusCode.BadRequest, res.StatusCode);
+  }
+
  [Fact]
  public async Task Unknown_table_is_404()
  {
@@ -0,0 +1,82 @@
+# RB-16 — `DateOnly.TryParse` on `?peildatum=`
+
+Status: **implemented** · 2026-08-27 · Source findings: `07-bio2-compliance.md` BIO-019 · `99-backlog.md` RB-16
+
+## What was wrong
+
+`Program.cs:215` (pre-change) —
+`var rows = peildatum is { Length: > 0 } p ? t.RowsOn(DateOnly.Parse(p)) : t.Rows();`.
+`DateOnly.Parse` throws `FormatException` on anything unparseable; there was no
+`TryParse`, no 400 path, and `.Produces` on the endpoint declared only 200/403/404 — so
+an unparseable `?peildatum=` value 500'd, and in Development the exception detail was
+returned to the caller. §3c's baseline named `backend/Stamdata` 96.8% line but **71.7%
+branch** (BL-005) — this was one of the unentered branches.
+
+## What changed
+
+| File                                                                   | Change                                                                                                                                                                                                                          |
+| ---------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
+| `Program.cs` — `GET /stamdata/{table}`                                 | `DateOnly.Parse` replaced with `DateOnly.TryParse`; an unparseable value now returns `Results.Problem(detail: …, statusCode: 400)` instead of throwing; endpoint doc gained `.ProducesProblem(StatusCodes.Status400BadRequest)` |
+| `tests/BigRegister.Tests/StamdataEndpointTests.cs`                     | **new** `Unparseable_peildatum_is_400_not_500`                                                                                                                                                                                  |
+| `backend/swagger.json`, `libs/shared/src/infrastructure/api-client.ts` | regenerated (`npm run gen:api`) — the new 400 response is now part of the documented contract                                                                                                                                   |
+
+## What the fix looks like
+
+```csharp
+DateOnly? peildatumWaarde = null;
+if (peildatum is { Length: > 0 } p)
+{
+  if (!DateOnly.TryParse(p, out var parsed))
+    return Results.Problem(detail: $"Ongeldige peildatum '{p}'.", statusCode: StatusCodes.Status400BadRequest);
+  peildatumWaarde = parsed;
+}
+var rows = peildatumWaarde is { } d ? t.RowsOn(d) : t.Rows();
+```
+
+Matches the shape every other bad-input check in this file already uses (e.g. the
+upload endpoint's `Results.Problem(detail: …, statusCode: 400)` for a malformed
+multipart request) — a `Results.Problem` with a Dutch detail message, not a bespoke
+response shape.
+
+## Judgement calls
+
+- **No FE change needed, and none made.** `libs/beheer/src/infrastructure/
+stamdata.adapter.ts`'s `load()` already routes every call through `runSubmit`
+  (`libs/shared/src/application/submit.ts`), which try/catches any thrown
+  `ApiException` — including the client's new 400 branch — into a generic `Result`
+  error string via `problemDetail`. There is no status-code-specific branching to
+  extend; ADR-0001's "the FE renders the decision, it does not recompute the rule"
+  already covers "the server rejected this input" as a case the generic error path
+  handles, same as the existing 403.
+- **Regenerated the API client and committed it in this ticket's diff**, rather than
+  leaving it to drift. RB-09's implementation note records a real prior incident where
+  a response-shape change (RB-08's 403 → `ProducesProblem`) landed without a
+  regeneration and the drift went unnoticed until the next ticket's `gen:api` run. This
+  ticket's `.ProducesProblem(400)` is exactly that same category of change, so
+  `npm run gen:api` was run immediately as part of implementing it, not deferred.
+- **The Dutch detail message follows the file's own convention** (`$"Ongeldige
+peildatum '{p}'."`) rather than English — every other `Results.Problem(detail: …)`
+  call in `Program.cs` (change-request rejection, upload validation, submit rejection)
+  is Dutch; this is server-internal wire text, not `$localize`-wrapped UI copy (the FE
+  never renders it verbatim — CLAUDE.md's `$localize` rule is about user-facing copy
+  the FE owns, not backend `ProblemDetails.detail` strings), so no locale entry was
+  needed.
+
+## Verification
+
+- **Reverted the fix only** (`DateOnly.Parse` restored, ternary un-nested, via Edit —
+  test left in place) and ran `StamdataEndpointTests`:
+  `Unparseable_peildatum_is_400_not_500` failed red (`Expected: BadRequest, Actual:
+InternalServerError` — confirming the endpoint really did 500, not some other status).
+  Restored the fix (via Edit) and re-ran: all 6 tests in the class green.
+- `dotnet build`: clean, 0 warnings.
+- `dotnet format BigRegister.slnx --verify-no-changes`: clean.
+- `dotnet test --filter "Category!=Integration"`: **260 passed, 0 failed** (259 + 1
+  new).
+- `npm run gen:api`: exit 0; `backend/swagger.json` gained the 400 response shape for
+  this one endpoint; `libs/shared/src/infrastructure/api-client.ts` gained the
+  matching `status === 400` branch. Both regenerated files committed alongside the
+  code change.
+- `npm test` (all four Vitest projects — ssp/behandelportal/shared/beheer): **445
+  passed, 0 failed**, confirming the regenerated client doesn't break any existing FE
+  consumer of `stamdataTable(...)`.
@@ -341,6 +341,12 @@ export class ApiClient {
            result200 = _responseText === "" ? null : JSON.parse(_responseText, this.jsonParseReviver) as StamdataTableDto;
            return result200;
            });
+        } else if (status === 400) {
+            return response.text().then((_responseText) => {
+            let result400: any = null;
+            result400 = _responseText === "" ? null : JSON.parse(_responseText, this.jsonParseReviver) as ProblemDetails;
+            return throwException("Bad Request", status, _responseText, _headers, result400);
+            });
        } else if (status === 403) {
            return response.text().then((_responseText) => {
            let result403: any = null;