refactor(server): consolidate handler error boilerplate into crate::error (#83) #104

Merged

icub3d merged 2 commits from feature/83-apierror into main

2026-07-09 04:15:45 +00:00

icub3d commented

2026-07-09 04:12:34 +00:00

Owner

Summary

Kills the error-handling duplication called out in #83. Every resource module had its own copy of struct ErrorBody, type ApiResult<T>, and bad_request/not_found/forbidden/conflict/internal/storage_err.

New crate::error:

ApiError { status, message } — IntoResponse renders the same { "error": msg } body as before; From<StorageError> applies the canonical mapping (NotFound→404, Conflict→409, else→500 — exactly what every storage_err did), so handlers can ? storage calls directly.
pub type ApiResult<T> = Result<Json<T>, ApiError>.
Free functions with the same names and behavior are re-exported, so call sites — bad_request("x"), .map_err(internal), .map_err(storage_err), .ok_or_else(|| not_found(..)) — are textually unchanged.

Migrated all 10 handler modules. Module-specific statuses (422 unprocessable, 410 gone, avatar errors) keep a tiny local helper that now returns ApiError.

Net −313 lines. No status code or response body changes — verified by the full existing suite.

Verification

cargo test -p server — 130 passed.
cargo clippy -p server --all-targets -- -D warnings — clean.
Reviewed every inline→helper conversion (channels 403s, invites 500, threads identity-map removal) to confirm status codes are preserved.

Deliberately out of scope (follow-up)

The ChannelMember extractor (second bullet of #83): it only fits routes with a :channel_id path segment, whereas reactions/threads derive the channel from a message/thread id — so it deserves its own focused change rather than being forced here.
The extractor-rejection types (AuthRejection, PermissionRejection, gateway handler) are a separate pattern and left as-is.

Part of #83

## Summary Kills the error-handling duplication called out in #83. Every resource module had its own copy of `struct ErrorBody`, `type ApiResult<T>`, and `bad_request`/`not_found`/`forbidden`/`conflict`/`internal`/`storage_err`. New `crate::error`: - **`ApiError { status, message }`** — `IntoResponse` renders the same `{ "error": msg }` body as before; **`From<StorageError>`** applies the canonical mapping (NotFound→404, Conflict→409, else→500 — exactly what every `storage_err` did), so handlers can `?` storage calls directly. - **`pub type ApiResult<T> = Result<Json<T>, ApiError>`.** - Free functions with the **same names and behavior** are re-exported, so call sites — `bad_request("x")`, `.map_err(internal)`, `.map_err(storage_err)`, `.ok_or_else(|| not_found(..))` — are textually unchanged. Migrated all 10 handler modules. Module-specific statuses (422 unprocessable, 410 gone, avatar errors) keep a tiny local helper that now returns `ApiError`. **Net −313 lines. No status code or response body changes** — verified by the full existing suite. ## Verification - `cargo test -p server` — **130 passed**. - `cargo clippy -p server --all-targets -- -D warnings` — clean. - Reviewed every inline→helper conversion (channels 403s, invites 500, threads identity-map removal) to confirm status codes are preserved. ## Deliberately out of scope (follow-up) - The **`ChannelMember` extractor** (second bullet of #83): it only fits routes with a `:channel_id` path segment, whereas reactions/threads derive the channel from a message/thread id — so it deserves its own focused change rather than being forced here. - The extractor-*rejection* types (`AuthRejection`, `PermissionRejection`, gateway handler) are a separate pattern and left as-is. Part of #83

icub3d added 1 commit

2026-07-09 04:12:34 +00:00

refactor(server): consolidate handler error boilerplate into crate::error

Security Audit / Gitleaks (Secrets) (pull_request) Successful in 5s

Details

Security Audit / Trivy (SCA & IaC) (pull_request) Failing after 18s

Details

Security Audit / PNPM Audit (JS SCA) (pull_request) Failing after 30s

Details

CI / Client — test & lint (pull_request) Successful in 40s

Details

Security Audit / Semgrep (SAST) (pull_request) Failing after 1m6s

Details

CI / Server — test & lint (pull_request) Successful in 2m52s

Details

Security Audit / Cargo Audit (Rust SCA) (pull_request) Has been cancelled

Details

5d21017a28

Every resource module redeclared an identical `struct ErrorBody`, an
`ApiResult<T>` alias, and the same `bad_request`/`not_found`/`forbidden`/
`conflict`/`internal`/`storage_err` constructors. Promote them once to
`crate::error`:

- `ApiError { status, message }` implements `IntoResponse` (same
  `{ "error": msg }` body as before) and `From<StorageError>` (NotFound→404,
  Conflict→409, else→500 — the old `storage_err` mapping), so handlers can use
  `?` on storage calls directly.
- `pub type ApiResult<T> = Result<Json<T>, ApiError>`.
- Free functions with the old names/behavior are kept, so call sites
  (`bad_request("x")`, `.map_err(internal)`, `.map_err(storage_err)`) are
  unchanged.

Migrated all 10 handler modules (attachments, auth, channels, invites,
membership, messages, profiles, reactions, roles, threads) to the shared type.
Module-specific statuses (422 unprocessable, 410 gone, avatar errors) keep a
small local helper now returning `ApiError`. Net −313 lines. No response
status/body changes.

Not included here: the `ChannelMember` extractor (the second bullet of #83) —
it only fits routes with a `:channel_id` path segment, while reactions/threads
derive the channel indirectly, so it warrants its own focused change. The
extractor-rejection types (`AuthRejection`, `PermissionRejection`) are left as
their own concern.

Part of #83

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

icub3d added the

area:api

type:refactor

labels

2026-07-09 04:12:40 +00:00

icub3d added 1 commit

2026-07-09 04:15:34 +00:00

Merge branch 'main' into feature/83-apierror

Security Audit / Gitleaks (Secrets) (pull_request) Successful in 14s

Details

Security Audit / PNPM Audit (JS SCA) (pull_request) Failing after 36s

Details