From 5137bf87a19480a134cf8dd801c303396ace8729 Mon Sep 17 00:00:00 2001 From: sax3l Date: Mon, 2 Nov 2026 06:58:15 +0100 Subject: [PATCH] fix: distinguish failed sources from empty results --- app/api/screen/route.ts | 43 ++++++++------- app/api/vehicles/route.ts | 46 ++++++++-------- services/api/orchestrator/main.py | 13 ++--- services/api/sources/connectors.py | 15 ++++-- .../api/tests/test_source_failure_honesty.py | 53 +++++++++++++++++++ tests/api/source-failure-honesty.test.ts | 42 +++++++++++++++ 6 files changed, 156 insertions(+), 56 deletions(-) create mode 100644 services/api/tests/test_source_failure_honesty.py create mode 100644 tests/api/source-failure-honesty.test.ts diff --git a/app/api/screen/route.ts b/app/api/screen/route.ts index 3dac028..a8bba5b 100644 --- a/app/api/screen/route.ts +++ b/app/api/screen/route.ts @@ -1,27 +1,30 @@ import { NextRequest, NextResponse } from "next/server"; -import { engineFetch } from "@/lib/engine"; +import { engineFetch, hasEngine } from "@/lib/engine"; -// GET /api/screen?q=Name — OpenSanctions screening (sanctions/PEP). VERIFY -// commercial terms before production volume. Defers to engine when running. +// An unavailable or malformed screening source is never evidence of no match. export async function GET(req: NextRequest) { - const q = req.nextUrl.searchParams.get("q") ?? ""; - if (!q) return NextResponse.json({ matches: [] }); - - const eng = await engineFetch(`/screen?q=${encodeURIComponent(q)}`); - if (eng && eng.ok) return NextResponse.json(await eng.json()); - + const q = req.nextUrl.searchParams.get("q")?.trim() ?? ""; + if (!q) return NextResponse.json({ error: { code: "bad_request", message: "q krävs" } }, { status: 400 }); + const unavailable = () => NextResponse.json({ error: { code: "source_unavailable", message: "Screeningkällan är inte tillgänglig" } }, { status: 503 }); try { + if (hasEngine()) { + const eng = await engineFetch(`/screen?q=${encodeURIComponent(q)}`); + if (!eng?.ok) return unavailable(); + const data: unknown = await eng.json(); + if (!data || typeof data !== "object" || (!Array.isArray((data as { matches?: unknown }).matches) || (data as { source_status?: unknown }).source_status !== "ok")) return unavailable(); + return NextResponse.json(data); + } const r = await fetch(`https://api.opensanctions.org/search/default?q=${encodeURIComponent(q)}&limit=5`, { - headers: { "User-Agent": "0pt/0.1 (+https://0pt.app; SIAX)" }, - cache: "no-store", + headers: { "User-Agent": "0pt/0.1 (+https://0pt.app; SIAX)" }, cache: "no-store", }); - if (!r.ok) return NextResponse.json({ matches: [], source: "opensanctions" }); - const data = (await r.json()) as { results?: { caption?: string; score?: number; schema?: string }[] }; - return NextResponse.json({ - source: "opensanctions", - matches: (data.results ?? []).map((x) => ({ name: x.caption ?? null, score: x.score ?? null, schema: x.schema ?? null })), - }); - } catch { - return NextResponse.json({ matches: [], source: "opensanctions" }); - } + if (!r.ok) return unavailable(); + const data: unknown = await r.json(); + if (!data || typeof data !== "object" || !Array.isArray((data as { results?: unknown }).results)) return unavailable(); + const results = (data as { results: unknown[] }).results; + if (results.some((item) => !item || typeof item !== "object" || Array.isArray(item))) return unavailable(); + return NextResponse.json({ source: "opensanctions", matches: results.map((item) => { + const x = item as { caption?: unknown; score?: unknown; schema?: unknown }; + return { name: x.caption ?? null, score: x.score ?? null, schema: x.schema ?? null }; + }) }); + } catch { return unavailable(); } } diff --git a/app/api/vehicles/route.ts b/app/api/vehicles/route.ts index be3d78e..0670bca 100644 --- a/app/api/vehicles/route.ts +++ b/app/api/vehicles/route.ts @@ -1,34 +1,30 @@ import { NextRequest, NextResponse } from "next/server"; -import { engineFetch } from "@/lib/engine"; +import { engineFetch, hasEngine } from "@/lib/engine"; -// GET /api/vehicles?plate=XX-99-YY — RDW (NL) open vehicle data (technical/fleet -// only, no owner PII). Defers to the engine when running; else calls RDW directly. +// A missing record is distinct from an unavailable or malformed source. export async function GET(req: NextRequest) { const plate = (req.nextUrl.searchParams.get("plate") ?? "").toUpperCase().replace(/[-\s]/g, "").replace(/[^A-Z0-9]/g, ""); if (!plate) return NextResponse.json({ error: { code: "bad_request", message: "plate krävs" } }, { status: 400 }); - - const eng = await engineFetch(`/vehicles/${plate}`); - if (eng && eng.ok) return NextResponse.json(await eng.json()); - + const unavailable = () => NextResponse.json({ error: { code: "source_unavailable", message: "Fordonskällan är inte tillgänglig" } }, { status: 503 }); try { + if (hasEngine()) { + const eng = await engineFetch(`/vehicles/${plate}`); + if (!eng?.ok) return unavailable(); + const data: unknown = await eng.json(); + if (!data || typeof data !== "object" || Array.isArray(data) || (data as { source_status?: unknown }).source_status !== "ok") return unavailable(); + return NextResponse.json(data); + } const r = await fetch(`https://opendata.rdw.nl/resource/m9d7-ebf2.json?kenteken=${encodeURIComponent(plate)}`, { - headers: { "User-Agent": "0pt/0.1 (+https://0pt.app; SIAX)" }, - cache: "no-store", + headers: { "User-Agent": "0pt/0.1 (+https://0pt.app; SIAX)" }, cache: "no-store", }); - if (!r.ok) return NextResponse.json({ vehicle: null }, { status: 200 }); - const arr = (await r.json()) as Record[]; - if (!arr.length) return NextResponse.json({ vehicle: null }); - const d = arr[0]; - return NextResponse.json({ - source: "rdw", - vehicle: { - reg_plate: plate, market: "NL", - make: d.merk ?? null, model: d.handelsbenaming ?? null, - year: (d.datum_eerste_toelating ?? "").slice(0, 4) || null, - }, - provenance: { level: 3, source: "rdw" }, - }); - } catch { - return NextResponse.json({ vehicle: null, error: "fetch_failed" }, { status: 200 }); - } + if (!r.ok) return unavailable(); + const data: unknown = await r.json(); + if (!Array.isArray(data) || data.some((item) => !item || typeof item !== "object" || Array.isArray(item))) return unavailable(); + if (!data.length) return NextResponse.json({ vehicle: null }); + const d = data[0] as Record; + if (d.datum_eerste_toelating != null && typeof d.datum_eerste_toelating !== "string") return unavailable(); + return NextResponse.json({ source: "rdw", vehicle: { reg_plate: plate, market: "NL", make: d.merk ?? null, + model: d.handelsbenaming ?? null, year: (d.datum_eerste_toelating ?? "").slice(0, 4) || null }, + provenance: { level: 3, source: "rdw" } }); + } catch { return unavailable(); } } diff --git a/services/api/orchestrator/main.py b/services/api/orchestrator/main.py index fe59485..547939e 100644 --- a/services/api/orchestrator/main.py +++ b/services/api/orchestrator/main.py @@ -169,17 +169,18 @@ async def sources_for(market: str): async def vehicle(plate: str): try: v = await rdw_vehicle(plate) - except Exception: - v = None - return v or {"vehicle": None} + except Exception as exc: + raise HTTPException(status_code=503, detail="vehicle source unavailable") from exc + return {**(v or {"vehicle": None}), "source_status": "ok"} @app.get("/screen") async def screen(q: str): try: - return await opensanctions_screen(q) - except Exception: - return {"matches": [], "source": "opensanctions"} + result = await opensanctions_screen(q) + return {**result, "source_status": "ok"} + except Exception as exc: + raise HTTPException(status_code=503, detail="screening source unavailable") from exc @app.post("/estimates") diff --git a/services/api/sources/connectors.py b/services/api/sources/connectors.py index b7bf5ea..65b1aa9 100644 --- a/services/api/sources/connectors.py +++ b/services/api/sources/connectors.py @@ -268,9 +268,10 @@ async def rdw_vehicle(kenteken: str) -> dict[str, Any] | None: url = f"https://opendata.rdw.nl/resource/m9d7-ebf2.json?kenteken={plate}" async with _client() as c: r = await c.get(url) - if r.status_code != 200: - return None + r.raise_for_status() arr = r.json() + if not isinstance(arr, list) or any(not isinstance(item, dict) for item in arr): + raise ValueError("invalid RDW response") if not arr: return None d = arr[0] @@ -296,9 +297,13 @@ async def opensanctions_screen(name: str) -> dict[str, Any]: url = "https://api.opensanctions.org/search/default" async with _client() as c: r = await c.get(url, params={"q": name, "limit": 5}) - if r.status_code != 200: - return {"matches": [], "source": "opensanctions"} - results = r.json().get("results", []) + r.raise_for_status() + payload = r.json() + if not isinstance(payload, dict) or not isinstance(payload.get("results"), list) or any( + not isinstance(item, dict) for item in payload["results"] + ): + raise ValueError("invalid OpenSanctions response") + results = payload["results"] return { "source": "opensanctions", "matches": [{"name": x.get("caption"), "score": x.get("score"), "schema": x.get("schema")} for x in results], diff --git a/services/api/tests/test_source_failure_honesty.py b/services/api/tests/test_source_failure_honesty.py new file mode 100644 index 0000000..3fc0db6 --- /dev/null +++ b/services/api/tests/test_source_failure_honesty.py @@ -0,0 +1,53 @@ +"""A failed or malformed source must never become a verified empty result.""" +import httpx +import pytest +import respx +from fastapi.testclient import TestClient +from orchestrator import main +from sources.connectors import opensanctions_screen, rdw_vehicle + + +@pytest.mark.asyncio +@pytest.mark.parametrize("status", [403, 429, 500]) +async def test_source_http_failure_raises(status): + with respx.mock: + respx.get("https://api.opensanctions.org/search/default").respond(status) + respx.get("https://opendata.rdw.nl/resource/m9d7-ebf2.json").respond(status) + with pytest.raises(httpx.HTTPStatusError): + await opensanctions_screen("example") + with pytest.raises(httpx.HTTPStatusError): + await rdw_vehicle("AB123") + + +@pytest.mark.asyncio +async def test_malformed_and_valid_empty_are_distinct(): + with respx.mock: + screen = respx.get("https://api.opensanctions.org/search/default").respond(200, json={}) + vehicle = respx.get("https://opendata.rdw.nl/resource/m9d7-ebf2.json").respond(200, json={}) + with pytest.raises(ValueError): + await opensanctions_screen("example") + with pytest.raises(ValueError): + await rdw_vehicle("AB123") + screen.respond(200, json={"results": []}) + vehicle.respond(200, json=[]) + assert (await opensanctions_screen("example"))["matches"] == [] + assert await rdw_vehicle("AB123") is None + + +def test_routes_return_503_on_source_failure_and_mark_verified_empty(monkeypatch): + async def failed(*args): + raise RuntimeError("fixture failure") + async def empty_screen(*args): + return {"source": "opensanctions", "matches": []} + async def empty_vehicle(*args): + return None + monkeypatch.setattr(main, "_INTERNAL_TOKEN", "") + monkeypatch.setattr(main, "opensanctions_screen", failed) + monkeypatch.setattr(main, "rdw_vehicle", failed) + client = TestClient(main.app) + assert client.get('/screen?q=example').status_code == 503 + assert client.get('/vehicles/AB123').status_code == 503 + monkeypatch.setattr(main, "opensanctions_screen", empty_screen) + monkeypatch.setattr(main, "rdw_vehicle", empty_vehicle) + assert client.get('/screen?q=example').json() == {"source": "opensanctions", "source_status": "ok", "matches": []} + assert client.get('/vehicles/AB123').json() == {"vehicle": None, "source_status": "ok"} diff --git a/tests/api/source-failure-honesty.test.ts b/tests/api/source-failure-honesty.test.ts new file mode 100644 index 0000000..a253d34 --- /dev/null +++ b/tests/api/source-failure-honesty.test.ts @@ -0,0 +1,42 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { NextRequest } from 'next/server'; +import { GET as screen } from '@/app/api/screen/route'; +import { GET as vehicles } from '@/app/api/vehicles/route'; + +const originalEngine = process.env.ENGINE_URL; +afterEach(() => { if (originalEngine === undefined) delete process.env.ENGINE_URL; else process.env.ENGINE_URL = originalEngine; vi.unstubAllGlobals(); }); +const screening = () => screen(new NextRequest('http://localhost/api/screen?q=example')); +const vehicle = () => vehicles(new NextRequest('http://localhost/api/vehicles?plate=AB123')); + +describe('source failure cannot mean absence', () => { + it.each([screening, vehicle])('returns 503 for source failures and malformed responses', async (call) => { + delete process.env.ENGINE_URL; + for (const response of [new Response('down', { status: 503 }), Response.json({}), new Response('{')]) { + vi.stubGlobal('fetch', vi.fn(async () => response)); + const result = await call(); + expect(result.status).toBe(503); + expect(await result.json()).toEqual({ error: expect.objectContaining({ code: 'source_unavailable' }) }); + } + }); + it('preserves an actual empty screening result', async () => { + delete process.env.ENGINE_URL; + vi.stubGlobal('fetch', vi.fn(async () => Response.json({ results: [] }))); + expect(await (await screening()).json()).toEqual({ source: 'opensanctions', matches: [] }); + }); + it('preserves an actual missing vehicle', async () => { + delete process.env.ENGINE_URL; + vi.stubGlobal('fetch', vi.fn(async () => Response.json([]))); + expect(await (await vehicle()).json()).toEqual({ vehicle: null }); + }); + it.each([screening, vehicle])('does not trust an old engine which can fabricate empty results', async (call) => { + process.env.ENGINE_URL = 'https://engine.test'; + vi.stubGlobal('fetch', vi.fn(async () => Response.json({ matches: [], vehicle: null }))); + expect((await call()).status).toBe(503); + expect(fetch).toHaveBeenCalledOnce(); + }); + it('accepts verified engine screening results', async () => { + process.env.ENGINE_URL = 'https://engine.test'; + vi.stubGlobal('fetch', vi.fn(async () => Response.json({ source: 'opensanctions', source_status: 'ok', matches: [] }))); + expect((await screening()).status).toBe(200); + }); +}); -- 2.54.0