Refactor code structure for improved readability and maintainability
Playwright Tests / test (push) Has been cancelled
Playwright Tests / test (push) Has been cancelled
This commit is contained in:
@@ -0,0 +1,72 @@
|
||||
---
|
||||
status: Accepted
|
||||
date: 2026-07-08
|
||||
applyTo:
|
||||
- "Fuchs/Controllers/**"
|
||||
- "Fuchs/Services/**"
|
||||
- "Fuchs/Notifications/**"
|
||||
supersededBy: ""
|
||||
---
|
||||
|
||||
# 0003 — Any exception that interrupts a user action notifies the user (not just the log)
|
||||
|
||||
## Context
|
||||
[0001](0001-domain-events-and-notification-triggers.md) requires the *modeled*
|
||||
failure paths (invoice/reminder/banking create, send, import) to publish a
|
||||
`*IssueAsync`/`*Failed` event. But an action can also fail through an
|
||||
**unexpected/unmodeled** exception — a bug, a transient dependency error, an
|
||||
edge case nobody wrote a specific failure event for. Those were only landing in
|
||||
the log (`_logger.LogError` + an HTTP 500), so the user saw the action stop with
|
||||
no explanation and no notification. The user asked that *whenever* an exception
|
||||
interrupts a process they initiated, they be told via the notification system.
|
||||
|
||||
## Decision
|
||||
Every exception that **interrupts a user-initiated action** must surface to the
|
||||
user through `IEventService`, in addition to being logged. Concretely:
|
||||
|
||||
- **Catch-all safety net at the dispatcher.** `IntranetController.Do`'s
|
||||
top-level `catch` publishes a generic `UserIssueAsync("Aktion fehlgeschlagen",
|
||||
…)` for any `Do_Process_*` action that throws without having already published
|
||||
its own (more specific) issue event. It is guarded by
|
||||
`UserIdent.IsAuthenticated` — pre-auth flows (login/logout, anonymous GETs)
|
||||
have no session to notify and the HTTP status already conveys the failure.
|
||||
- **Handlers with their own `catch` must notify locally.** A handler that
|
||||
swallows its exception (returns a 500/error result instead of rethrowing)
|
||||
never reaches the `Do` net, so it must call the matching issue event itself —
|
||||
e.g. `HandleInvoiceGet` calls `InvoiceIssueAsync` before returning 500. Prefer
|
||||
the domain-specific method (`InvoiceIssueAsync`/`ReminderIssueAsync`/
|
||||
`BankingImportIssueAsync`); fall back to `UserIssueAsync` when none fits.
|
||||
- **Message stays user-readable and broadcast-safe.** Per
|
||||
[0002](0002-gui-notification-delivery-signalr.md) notifications are broadcast
|
||||
to every logged-in session, so the German `Message`/`Context` must never carry
|
||||
the raw exception text or anything sensitive — diagnostics go to the log; the
|
||||
user gets a plain "could not be completed" message.
|
||||
|
||||
This deliberately **excludes** operations that do not interrupt a discrete user
|
||||
action: background/best-effort work (blob archiving, startup self-tests,
|
||||
per-entry parse skips) stays log-only, and auto-refreshing read views (dashboard
|
||||
widgets, report reloads) return their error status without a toast, because
|
||||
notifying on every poll cycle would spam the user rather than inform them.
|
||||
|
||||
## Consequences
|
||||
- New `catch` blocks on a request-handling path must be classified: does the
|
||||
exception interrupt a user action? If yes → publish an issue event (specific
|
||||
if one exists, else `UserIssueAsync`). If it is background/best-effort or an
|
||||
auto-poll read → log only, and say so in a comment.
|
||||
- The `Do` net is a backstop, not a replacement for specific events: modeled
|
||||
failures should still publish their contextful `*IssueAsync` at the point of
|
||||
failure so the message names the invoice/reminder/file involved.
|
||||
- Because the net only fires on *unhandled* exceptions (handled flows return
|
||||
rather than rethrow), it does not double-notify the flows that already report
|
||||
their own failures.
|
||||
|
||||
## Alternatives considered
|
||||
- **Rely solely on 0001's per-flow issue events**: rejected — it leaves every
|
||||
unmodeled/unexpected exception silent, which is exactly the gap the user
|
||||
reported.
|
||||
- **Notify on every failing read/poll too (widgets, reports)**: rejected —
|
||||
auto-refresh would turn a transient backend hiccup into a stream of toasts;
|
||||
those paths surface failure via HTTP status instead.
|
||||
- **Surface the raw exception message to the GUI**: rejected for the same
|
||||
reason as 0001 — not user-understandable, leaks internals, and (per 0002) is
|
||||
visible to every logged-in session.
|
||||
Reference in New Issue
Block a user