feat: Enhance error logging for PDF preview failures and SQL timeouts

- Implemented a new method `ShapeTableNames` to gather entity and navigation table names, ensuring no blanks or duplicates are included. - Updated `FdsMfrClient` to handle cases where schema queries return no tables, logging appropriate warnings and preventing misleading errors. - Introduced `PdfPreviewAsync` in `IntranetController` to manage PDF preview rendering, capturing detailed logs for failures and notifying users with readable error messages. - Added comprehensive unit tests for error logging and table name shaping to ensure robustness and correctness. - Documented decision to persist user action failures to the `fds__admin_logdebug` table, enhancing support diagnostics.
2026-09-16 16:35:04 +02:00
parent 38e0d55b09
commit fce83c528f
19 changed files with 609 additions and 56 deletions
@@ -0,0 +1,64 @@
+---
+status: Accepted
+date: 2026-09-16
+applyTo:
+  - "Fuchs/Controllers/**"
+  - "Fuchs/code/FuchsIntranet.cs"
+  - "Fuchs/js/intranet/**"
+supersededBy: ""
+---
+
+# 0023 — Failures of user actions are persisted to `fds__admin_logdebug`, with the SQL and a readable description
+
+## Context
+Support diagnoses production problems from a query over the `fds__admin_logdebug` table
+(`EXECUTE [dbo].[fds__admin_logdebug]` via `Fuchs_intranet.debug_log`) — not from the
+web server's `logs/ErrorLog.txt` ([0003](0003-structured-diagnostic-logging.md)), which is
+rarely reachable. Reviewing the 2026-09-16 production extract of that table showed three gaps:
+
+- Every "SQL Error in getSQLDataSet" row (twelve execution timeouts in one morning) had
+  `Data = NULL`. `FIS_SQLOptions.OnError` logged its constructor *context* (the fn/id/code,
+  usually `null`) instead of the *data* OCORE hands over (`new { sql = … }`), so nobody could
+  tell which statement timed out.
+- Unhandled exceptions from `IntranetController.Do` went to `ILogger` only. A customer's
+  failing "PDF-Vorschau" left no trace in the table at all.
+- The browser's generic failure alert reads its description from the `x-ocms-desc` response
+  header, which the backend never set — so users always saw an empty
+  "Der Server hat einen Fehler gemeldet:".
+
+## Decision
+- **The DB log row must carry what is needed to attribute the failure.** `FIS_SQLOptions`
+  merges the OCORE payload (the SQL statement) with the caller context into the `Data`
+  column (`{ sql, context }`). New `OnError` hooks must never drop the statement.
+- **Every failure that interrupts a user action is written to `fds__admin_logdebug` too**,
+  in addition to `ILogger` and the notification required by
+  [0003](0003-interrupting-exceptions-notify-the-user.md): the `Do` catch-all writes
+  `Unhandled exception in Do {fn}/{id}` with route, user and the form *field names* (never
+  values — they can carry tokens/PII); the PDF-preview endpoints go through a shared guard
+  (`IntranetController.PdfPreviewAsync`) that logs `PDF preview failed (endpoint)` with
+  ref/user/duration, counts `fuchs.pdf.previews{outcome}` and toasts the user.
+- **A 500 carries its message in the `x-ocms-desc` header** (`ServerError(msg)` →
+  `EncodeResponseDescription`: percent-encoded ASCII, spaces as `%20` because the client
+  maps `+` to space before `decodeURIComponent`). Handlers that return a 500 should pass a
+  user-readable German message; the frontend shows it verbatim.
+- **Preview calls in the frontend never fail silently.** They use a 120 s timeout
+  (`$inv.pdfPreviewTimeout`), one shared error handler (`$inv.pdfPreviewFailed`) and a
+  `complete` handler that unfreezes the editor and reports a timeout — `postXT` returns early
+  on a timeout without calling `error`, so `complete` is the only place it is observable.
+
+## Consequences
+- Support can attribute a SQL timeout to its statement straight from the table; a failing
+  preview is visible as one row with endpoint, reference id, user and duration.
+- `debug_log` is a synchronous DB round-trip and is only used on error paths; it must not be
+  added to hot/success paths (see [0003](0003-structured-diagnostic-logging.md) on the cost
+  of DB logging).
+- `x-ocms-desc` is user-facing: messages must be German, short, and free of secrets. The
+  innermost exception text is acceptable for an internal intranet audience.
+
+## Alternatives considered
+- **Enable `FuchsLoggerProvider.DatabaseLoggingEnabled`** so every `ILogger` error lands in
+  the DB. Rejected: it would write every `LogError` (including expected/handled ones and
+  background-service noise) with a DB round-trip each, and the provider has no access to the
+  request context (user, route) that makes a row actionable.
+- **Return the message only in the JSON body.** The generic client failure path never parses
+  the body of a non-2xx response; the header is the existing contract in `oci_basic.js`.