feat(error): styled themed error page for AppError::IntoResponse (LC-220) #263

Merged
nrupard merged 2 commits from feat/lc-220-styled-error-page into main 2026-05-29 19:22:09 +02:00
Owner

Summary

User on staging: hitting a deleted-room URL like /room/9999 while logged in showed raw "Not Found" text on a blank white page in dark mode, no chrome, no theme, no back link. The router fallback for unmatched paths rendered the styled not_found.html, but every handler-returned AppError returned bare text.

AppError::IntoResponse now renders a themed standalone error page that extends base.html directly. The no-flash theme bootstrap + branding apply automatically; the user sees a card-style page in their chosen theme with a "Back to home" link.

What ships

  • server/templates/error.html — standalone page extending base.html, themed via tokens.
  • server/src/views/error_page.rsErrorPage view struct.
  • server/src/error.rsrender_styled(status, heading_key, message) helper; AppError::IntoResponse rewritten to use it for every variant except Redirect.
  • 9 new Fluent keys in server/locales/{en,es}/common.ftl (8 status headings + error-back-home).

Behavior changes

  • AppError::Internal(msg): previously surfaced "Internal Server Error" as the body. Now renders the styled page WITHOUT msg. The server-side detail still logs via tracing::error! but never reaches the response body. Closes a mild leak where a sqlx / askama error string could land in the response.
  • AppError::TooManyRequests: Retry-After header still emitted; body now styled.
  • AppError::Conflict / BadRequest / PayloadTooLarge: dynamic msg still surfaced as the page detail (server-curated, contextually useful).
  • AppError::Redirect: unchanged.

Scope carve-out

Sidebar chrome is deliberately NOT included. IntoResponse has no AppState access (no DB pool). The router-level handle_not_found keeps its sidebar-bearing rendering for the genuinely-unmatched-path 404 case; this commit covers handler-returned errors only. The user complaint was about BARE-TEXT-IN-DARK-MODE; that's fixed end-to-end. A follow-up could thread state through a tower middleware to add chrome on top of these pages.

Test plan

  • cargo check -p lets-chat-server clean (standalone + saas).
  • cargo clippy --workspace --all-targets --features standalone -- -D warnings clean.
  • cargo fmt --all no-op after fmt run.
  • Smoke test on routes_room_rbac (3 forbidden-path assertions) green.
  • Visual smoke on staging after deploy: /room/{deleted-id} shows themed page in dark mode (NOT bare-text-on-white). Internal error surfaces a card with "Server error" heading, NO sqlx detail text.

Pre-existing main red

cargo test --test lc77_webhook_render_fixture fails on bare main (4 fixture mismatches). Independent of LC-220; fixtures need FIXTURE_WRITE=1 regenerate via direct docker. Tracked separately.

## Summary User on staging: hitting a deleted-room URL like `/room/9999` while logged in showed raw "Not Found" text on a blank white page in dark mode, no chrome, no theme, no back link. The router fallback for unmatched paths rendered the styled `not_found.html`, but every handler-returned `AppError` returned bare text. `AppError::IntoResponse` now renders a themed standalone error page that extends `base.html` directly. The no-flash theme bootstrap + branding apply automatically; the user sees a card-style page in their chosen theme with a "Back to home" link. ## What ships - `server/templates/error.html` — standalone page extending `base.html`, themed via tokens. - `server/src/views/error_page.rs` — `ErrorPage` view struct. - `server/src/error.rs` — `render_styled(status, heading_key, message)` helper; `AppError::IntoResponse` rewritten to use it for every variant except `Redirect`. - 9 new Fluent keys in `server/locales/{en,es}/common.ftl` (8 status headings + `error-back-home`). ## Behavior changes - **`AppError::Internal(msg)`**: previously surfaced "Internal Server Error" as the body. Now renders the styled page WITHOUT `msg`. The server-side detail still logs via `tracing::error!` but never reaches the response body. Closes a mild leak where a sqlx / askama error string could land in the response. - **`AppError::TooManyRequests`**: `Retry-After` header still emitted; body now styled. - **`AppError::Conflict / BadRequest / PayloadTooLarge`**: dynamic `msg` still surfaced as the page detail (server-curated, contextually useful). - **`AppError::Redirect`**: unchanged. ## Scope carve-out Sidebar chrome is deliberately NOT included. `IntoResponse` has no `AppState` access (no DB pool). The router-level `handle_not_found` keeps its sidebar-bearing rendering for the genuinely-unmatched-path 404 case; this commit covers handler-returned errors only. The user complaint was about BARE-TEXT-IN-DARK-MODE; that's fixed end-to-end. A follow-up could thread state through a tower middleware to add chrome on top of these pages. ## Test plan - [x] `cargo check -p lets-chat-server` clean (standalone + saas). - [x] `cargo clippy --workspace --all-targets --features standalone -- -D warnings` clean. - [x] `cargo fmt --all` no-op after fmt run. - [x] Smoke test on `routes_room_rbac` (3 forbidden-path assertions) green. - [ ] Visual smoke on staging after deploy: `/room/{deleted-id}` shows themed page in dark mode (NOT bare-text-on-white). Internal error surfaces a card with "Server error" heading, NO sqlx detail text. ## Pre-existing main red `cargo test --test lc77_webhook_render_fixture` fails on bare main (4 fixture mismatches). Independent of LC-220; fixtures need `FIXTURE_WRITE=1` regenerate via direct docker. Tracked separately.
feat(error): styled themed error page for AppError::IntoResponse (LC-220)
All checks were successful
check-secrets / TruffleHog (push) Successful in 4s
check-secrets / Nosey parker (push) Successful in 5s
check-secrets / Kingfisher (push) Successful in 6s
check-secrets / Kingfisher (pull_request) Successful in 5s
check-secrets / Nosey parker (pull_request) Successful in 6s
check-secrets / TruffleHog (pull_request) Successful in 6s
Check / clippy + fmt + tests (pull_request) Successful in 2m2s
ac538f4228
User on staging: hitting a deleted-room URL like `/room/9999` while logged in showed raw "Not Found" text on a blank white page in dark mode, no chrome, no theme, no back link. Same for every other variant of `AppError::IntoResponse`. The router fallback `handle_not_found` rendered the styled `not_found.html` for unmatched paths, but every handler-returned `AppError` returned bare text.

Replace the bare-text branches with a themed standalone error page that extends `base.html` directly. The no-flash theme bootstrap and operator branding apply automatically; the user sees a card-style page in their chosen theme with a large "Back to home" link.

New surfaces:
- `server/templates/error.html`: extends `base.html`, renders status + heading + optional message + back link. Uses `bg-surface text-content` so it themes under all four palettes (light / dark / hc-light / hc-dark).
- `server/src/views/error_page.rs`: `ErrorPage` view struct with status, status_heading, message, back_url, back_label, asset_version.
- `server/src/error.rs`: `render_styled(status, heading_key, message)` helper renders the page via Askama. `AppError::IntoResponse` now branches per variant on which Fluent key to use and which `msg` (if any) to surface as the body detail.

Localized headings + back label added to `server/locales/{en,es}/common.ftl`:
`error-status-{not-found,forbidden,unauthorized,conflict,bad-request,payload-too-large,too-many-requests,internal}` plus `error-back-home`. Fluent's `CURRENT_LOCALE` task-local is still in scope when `into_response` runs (set by the `resolve_locale` middleware that wraps the whole handler).

Behavior changes worth noting:
- `AppError::Internal(msg)`: previously surfaced "Internal Server Error" as the body. Now renders the styled page WITHOUT the `msg` (server-side detail logged via `tracing::error!` as before but never reaches the response body). This closes a mild leak where a sqlx / askama error string would land in the response body.
- `AppError::TooManyRequests(msg, retry_after)`: the `Retry-After` header is still emitted; the body now renders the styled page. Operators with well-behaved HTTP clients see no change in back-off behavior.
- `AppError::Conflict / BadRequest / PayloadTooLarge`: the dynamic `msg` IS still surfaced as the page detail, because those messages are server-curated and contextually useful to the user (e.g. "unknown quote_id", "burst exceeds 10000").

Sidebar chrome is deliberately NOT included. The IntoResponse path has no `AppState` access (no DB pool to load chrome from). The router-level `handle_not_found` keeps its sidebar-bearing rendering for the genuinely-unmatched-path 404 case; this commit covers handler-returned errors only. A follow-up could thread state through a tower middleware to add chrome on top of these pages, but the user complaint was the BARE-TEXT-IN-DARK-MODE failure, which this commit fixes end-to-end.

Verified: `cargo check -p lets-chat-server` clean (standalone + saas), `cargo clippy --workspace --all-targets --features standalone -- -D warnings` clean, `cargo fmt --all` no-op after fmt run, smoke test on `routes_room_rbac` (3 forbidden-path assertions) green.

#LC-220
fix(error): code-review fixes for LC-220 styled error page
All checks were successful
check-secrets / Nosey parker (push) Successful in 4s
check-secrets / Nosey parker (pull_request) Successful in 4s
check-secrets / Kingfisher (pull_request) Successful in 4s
check-secrets / Kingfisher (push) Successful in 5s
check-secrets / TruffleHog (pull_request) Successful in 5s
check-secrets / TruffleHog (push) Successful in 6s
Check / clippy + fmt + tests (pull_request) Successful in 2m0s
Create release / Create release from merged PR (pull_request) Has been skipped
2f8e4192ff
Five findings addressed in one follow-up commit so PR #263 ships clean.

1. `templates/error.html` heading color now splits 4xx (text-warning amber) and 5xx (text-danger red). Uniform red across all errors read as alarming on a 404 or 429; danger should be reserved for genuine server faults.

2. `templates/error.html` `<section role="alert">` added so assistive tech announces the page as an error region. URL change alone is not always announced; explicit `role="alert"` closes that gap.

3. `error.rs::render_styled` clamps the dynamic `message` to 200 characters before passing to the template. Audit of all `AppError::BadRequest(format!(...))` / `Conflict(format!(...))` / `PayloadTooLarge(format!(...))` callsites in `server/src/` found ~40 sites that interpolate user-controlled input (form fields, archive entry paths, sidebar reorder ids, status text, mute_mode form values, multipart parser errors that include attacker-controlled filename fragments, etc.). Askama auto-escapes HTML so XSS is prevented, but length-clamping limits same-session content reflection so an attacker who tricks a victim into visiting a long phishing-string URL cannot grow the rendered payload arbitrarily. 200 chars is enough for legitimate server-curated errors (the longest in-tree is the LC-94 "you are sending messages too quickly; retry in N seconds" message at ~70 chars).

4. `views/error_page.rs` `back_url` doc comment dropped the misleading "caller decides" claim. `AppError::IntoResponse` always passes `/`; the auth middleware redirects unauthed visitors to `/login` from there, so a single value covers both cases. Documenting accurate semantics; future extension can re-broaden the contract.

5. Consolidated the duplicate `error-back-home` / `not-found-back-home` Fluent keys onto the existing `not-found-back-home`. Both produced identical English ("Back to home") and Spanish ("Volver al inicio") values; one source of truth is cleaner. Catalogs now carry a one-line note explaining the share so the next translator knows.

Verified: `cargo check -p lets-chat-server` clean (standalone), `cargo clippy --workspace --all-targets --features standalone -- -D warnings` clean, `cargo fmt --all` no-op after fmt run.

#LC-220
nrupard deleted branch feat/lc-220-styled-error-page 2026-05-29 19:22:09 +02:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
psa-systems/lets-chat!263
No description provided.