feat: centralize API error localization

Replace the err instanceof ApiError ? err.message : t(fallback) anti- pattern -- which showed raw English backend text for the common case and only used the localized fallback for the rare network-failure case -- at all 13 call sites across 7 files. New frontend/src/api/errorMessages.ts (describeApiError) resolves a caught error to a localized {title, explanation, nextStep?, technical} by checking the 32 known AppError codes first, then known HTTP statuses (401/403/404/409/422/500), then a fully generic fallback. New ApiErrorNotice (PageChrome.tsx) renders title/explanation/nextStep with the raw text demoted to a "Technical details"/"Details techniques" disclosure -- never shown as the primary message. ApiError itself is split out of client.ts into a standalone api/apiError.ts with no import.meta.env dependency, so errorMessages.ts (and its tests) can be loaded outside a Vite/browser context.
2026-08-04 03:08:07 +02:00
parent 94cfb7bcbb
commit d17af1c52a
16 changed files with 940 additions and 82 deletions
@@ -0,0 +1,205 @@
+import { expect, test } from "@playwright/test";
+import fs from "node:fs";
+import path from "node:path";
+import { fileURLToPath } from "node:url";
+import { ApiError } from "../src/api/apiError";
+import { describeApiError, KNOWN_CODES } from "../src/api/errorMessages";
+
+// Pure Node-context checks for the central API-error-localization function (section 7 /
+// 11 of the Fleet Ops final localization brief). No browser needed: describeApiError()
+// only depends on a `t` function and a caught error, so it's tested here against the
+// real locale JSON with a minimal i18next-shaped `t` stub -- proving the known-code and
+// known-HTTP-status paths never leak raw backend English as the primary message, and
+// that the raw text is always still available via `.technical` for "Technical details".
+
+const __dirname = path.dirname(fileURLToPath(import.meta.url));
+const LOCALES_DIR = path.resolve(__dirname, "../src/i18n/locales");
+const LANGUAGES = ["nl-BE", "en-GB", "fr-BE"] as const;
+
+function loadNamespace(language: string, namespace: string): Record<string, unknown> {
+  const filePath = path.join(LOCALES_DIR, language, `${namespace}.json`);
+  return JSON.parse(fs.readFileSync(filePath, "utf-8"));
+}
+
+// Mirrors the (namespace, options.defaultValue) contract react-i18next's `t` exposes,
+// resolving "namespace:dotted.path" against the real locale files for the given language.
+function makeT(language: string): (key: string, options?: Record<string, unknown>) => string {
+  return (key: string, options?: Record<string, unknown>) => {
+    const [ns, ...rest] = key.includes(":") ? key.split(":") : ["errors", key];
+    const dottedPath = key.includes(":") ? rest.join(":") : rest.join("");
+    const data = loadNamespace(language, ns);
+    const value = dottedPath.split(".").reduce<unknown>((acc, part) => {
+      if (acc && typeof acc === "object") return (acc as Record<string, unknown>)[part];
+      return undefined;
+    }, data);
+    if (typeof value === "string") return value;
+    if (options && "defaultValue" in options) return String(options.defaultValue);
+    return key;
+  };
+}
+
+const KNOWN_HTTP_STATUSES = ["401", "403", "404", "409", "422", "500"];
+
+test("every known AppError code has a non-empty title+explanation in all 3 locales", () => {
+  for (const language of LANGUAGES) {
+    const codes = loadNamespace(language, "errors").codes as Record<string, { title?: string; explanation?: string }>;
+    for (const code of KNOWN_CODES) {
+      expect(codes[code], `${language}/errors.json is missing codes.${code}`).toBeTruthy();
+      expect(codes[code]?.title?.trim().length ?? 0, `${language}/errors.json:codes.${code}.title is empty`).toBeGreaterThan(0);
+      expect(
+        codes[code]?.explanation?.trim().length ?? 0,
+        `${language}/errors.json:codes.${code}.explanation is empty`,
+      ).toBeGreaterThan(0);
+    }
+  }
+});
+
+test("every known HTTP status fallback has a non-empty title+explanation in all 3 locales", () => {
+  for (const language of LANGUAGES) {
+    const http = loadNamespace(language, "errors").http as Record<string, { title?: string; explanation?: string }>;
+    for (const status of KNOWN_HTTP_STATUSES) {
+      expect(http[status], `${language}/errors.json is missing http.${status}`).toBeTruthy();
+      expect(http[status]?.title?.trim().length ?? 0, `${language}/errors.json:http.${status}.title is empty`).toBeGreaterThan(0);
+      expect(
+        http[status]?.explanation?.trim().length ?? 0,
+        `${language}/errors.json:http.${status}.explanation is empty`,
+      ).toBeGreaterThan(0);
+    }
+  }
+});
+
+test("a known AppError code resolves to its localized codes.* entry, never the raw backend message", () => {
+  for (const language of LANGUAGES) {
+    const t = makeT(language);
+    const raw = "IntegrityError: duplicate key value violates unique constraint";
+    const err = new ApiError(409, "VEHICLE_NOT_FOUND", raw, "corr-1");
+    const info = describeApiError(t, err);
+    const expected = loadNamespace(language, "errors").codes as Record<string, { title: string; explanation: string; nextStep?: string }>;
+    expect(info.title).toBe(expected.VEHICLE_NOT_FOUND.title);
+    expect(info.explanation).toBe(expected.VEHICLE_NOT_FOUND.explanation);
+    expect(info.title).not.toBe(raw);
+    expect(info.explanation).not.toBe(raw);
+    // The raw backend text must still be reachable, just demoted to `.technical`.
+    expect(info.technical).toBe(raw);
+  }
+});
+
+test("a code with nextStep populates it; a code without nextStep leaves it undefined", () => {
+  const t = makeT("nl-BE");
+  const withNextStep = describeApiError(t, new ApiError(422, "EMPTY_VALUE", "raw", "c1"));
+  expect(withNextStep.nextStep).toBeTruthy();
+
+  const withoutNextStep = describeApiError(t, new ApiError(404, "CUSTOMER_NOT_FOUND", "raw", "c2"));
+  expect(withoutNextStep.nextStep).toBeUndefined();
+});
+
+test("an unrecognized AppError code falls back to the matching known HTTP status, not raw text", () => {
+  for (const language of LANGUAGES) {
+    const t = makeT(language);
+    const raw = "Some brand-new backend code nobody localized yet";
+    const err = new ApiError(404, "SOME_FUTURE_CODE_NOT_YET_LOCALIZED", raw, "corr-2");
+    const info = describeApiError(t, err);
+    const expected404 = (loadNamespace(language, "errors").http as Record<string, { title: string; explanation: string }>)["404"];
+    expect(info.title).toBe(expected404.title);
+    expect(info.explanation).toBe(expected404.explanation);
+    expect(info.title).not.toBe(raw);
+    expect(info.technical).toBe(raw);
+  }
+});
+
+test("an unrecognized code and an unrecognized HTTP status fall back to the fully generic message", () => {
+  for (const language of LANGUAGES) {
+    const t = makeT(language);
+    const raw = "418 I'm a teapot (never mapped)";
+    const err = new ApiError(418, "418", raw, "corr-3");
+    const info = describeApiError(t, err);
+    const generic = loadNamespace(language, "errors").generic as { title: string; explanation: string };
+    expect(info.title).toBe(generic.title);
+    expect(info.explanation).toBe(generic.explanation);
+    expect(info.technical).toBe(raw);
+  }
+});
+
+test("a stringified HTTP status used as the AppError code (plain HTTPException path) resolves via the http map", () => {
+  // Mirrors app/main.py's plain-HTTPException handler, which sets code = str(status_code)
+  // (e.g. "401") rather than a semantic AppError code -- see backend/app/main.py.
+  const t = makeT("fr-BE");
+  const err = new ApiError(401, "401", "Not authenticated", "corr-4");
+  const info = describeApiError(t, err);
+  const expected401 = (loadNamespace("fr-BE", "errors").http as Record<string, { title: string }>)["401"];
+  expect(info.title).toBe(expected401.title);
+  expect(info.title).not.toBe("Not authenticated");
+});
+
+test("a non-ApiError (e.g. network failure before any response) uses the fallback key, never a raw JS error message as the primary text", () => {
+  const t = makeT("nl-BE");
+  const networkFailure = new TypeError("Failed to fetch");
+  // Real call sites (e.g. Automation.tsx) invoke t() with their own default namespace
+  // already scoped via useTranslation("integrations"); this stub's default namespace is
+  // "errors", so the fallback key is qualified explicitly here to match.
+  const info = describeApiError(t, networkFailure, "integrations:ledger.retryFailed");
+  const expectedFallback = loadNamespace("nl-BE", "integrations").ledger as Record<string, string>;
+  expect(info.explanation).toBe(expectedFallback.retryFailed);
+  expect(info.explanation).not.toBe("Failed to fetch");
+  expect(info.technical).toBe("Failed to fetch");
+});
+
+// --- Backend/frontend AppError code drift guard ---
+// KNOWN_CODES is a hand-maintained mirror of every `raise AppError("CODE", ...)` in the
+// backend (see app/core/errors.py::AppError and every raise site). If the backend adds a
+// new code and nobody updates KNOWN_CODES, it silently falls back to the generic-but-
+// still-localized HTTP/generic message rather than raw English -- not a broken build, but
+// a missed opportunity for a more specific message. This test surfaces that drift instead
+// of letting it go unnoticed indefinitely.
+const BACKEND_APP_DIR = path.resolve(__dirname, "../../backend/app");
+
+function collectPyFiles(dir: string): string[] {
+  const entries = fs.readdirSync(dir, { withFileTypes: true });
+  return entries.flatMap((entry) => {
+    const full = path.join(dir, entry.name);
+    if (entry.isDirectory()) return collectPyFiles(full);
+    return entry.name.endsWith(".py") ? [full] : [];
+  });
+}
+
+function collectBackendAppErrorCodes(): Set<string> {
+  const codes = new Set<string>();
+  for (const file of collectPyFiles(BACKEND_APP_DIR)) {
+    const source = fs.readFileSync(file, "utf-8");
+    const pattern = /AppError\(\s*"([A-Z_]+)"/g;
+    let match: RegExpExecArray | null;
+    while ((match = pattern.exec(source)) !== null) {
+      codes.add(match[1]);
+    }
+  }
+  return codes;
+}
+
+test("frontend KNOWN_CODES exactly matches every AppError code actually raised by the backend", () => {
+  const backendCodes = collectBackendAppErrorCodes();
+  const frontendCodes = KNOWN_CODES;
+
+  const missingFromFrontend = [...backendCodes].filter((c) => !frontendCodes.has(c)).sort();
+  const staleInFrontend = [...frontendCodes].filter((c) => !backendCodes.has(c)).sort();
+
+  expect(
+    missingFromFrontend,
+    `Backend raises AppError code(s) with no localized entry in errorMessages.ts KNOWN_CODES ` +
+      `(they'll fall back to a generic/HTTP-status message): ${missingFromFrontend.join(", ")}`,
+  ).toEqual([]);
+  expect(
+    staleInFrontend,
+    `errorMessages.ts KNOWN_CODES lists code(s) the backend never raises -- likely renamed or ` +
+      `removed on the backend side: ${staleInFrontend.join(", ")}`,
+  ).toEqual([]);
+});
+
+test("a non-ApiError with no fallbackKey uses the fully generic explanation", () => {
+  for (const language of LANGUAGES) {
+    const t = makeT(language);
+    const info = describeApiError(t, new TypeError("Failed to fetch"));
+    const generic = loadNamespace(language, "errors").generic as { title: string; explanation: string };
+    expect(info.title).toBe(generic.title);
+    expect(info.explanation).toBe(generic.explanation);
+  }
+});