Commit 20ab763
committed
Merge #852: refactor!: replace ServiceError god enum with domain-scoped error types (ADR-T-006)
19ad039 fix: improve error fidelity across From impls and messages (Peer Cat)
f51406b feat: add 188 crate-level tests for the domain error system (ADR-T-006 Phase 4) (Peer Cat)
f231c6c refactor!: replace ServiceError god enum with domain-scoped error types (ADR-T-006) (Peer Cat)
0816d65 refactor(errors): extract CategoryTagError from ServiceError (Peer Cat)
a55fc2c refactor: standardise error derives on thiserror (ADR-T-006 Phase 0) (Peer Cat)
Pull request description:
### Summary
Replace the monolithic 41-variant `ServiceError` enum with four domain-scoped error types — `AuthError`, `UserError`, `TorrentError`, and `CategoryTagError` — each with co-located HTTP status-code mapping. A thin `ApiError` wrapper unifies them at the handler boundary. A 188-test crate-level suite locks down the new error contracts.
Full rationale, migration plan, and testing acceptance criteria are documented in ADR-T-006.
### Motivation
`ServiceError` had grown into a god enum where every service function could theoretically return any of 41 variants. Nine `From` impls silently erased original error causes into `InternalServerError` via `eprintln!`, and a monolithic match block mapped variants to HTTP status codes far from where the errors were defined. This made it impossible to know at the type level which errors a given service call could actually produce.
### What changed
**Phase 0 — Prerequisite cleanup** (`a55fc2c`)
- Standardised all error derives from `derive_more` and hand-written impls to `thiserror`
- Replaced all `eprintln!` in `From` impls with structured `tracing::error!` calls
- Implemented the `Error` trait on cache error types that previously only derived `Debug`
- Removed the unused `anyhow` dependency
**Phase 1 — Extract domain errors** (`430a06d`, `3b715a0`)
- Introduced `AuthError` (13 variants: JWT, password, authorization)
- Introduced `UserError` (21 variants: registration, profile, banning)
- Introduced `TorrentError` (~24 variants: upload, listing, tracker interaction)
- Retained `CategoryTagError` (extracted first as a proving ground)
- Each domain error has a `status_code()` method and typed `From` impls for lower-level errors
**Phase 2 — `ApiError` wrapper** (`3b715a0`)
- `ApiError` wraps all four domain errors via `#[from]` and implements `IntoResponse`
- Handlers spanning multiple domains use `ApiError` as their error type
**Phase 3 — Cleanup** (`3b715a0`)
- Removed `ServiceError` enum and `ServiceResult` type alias
- Removed `http_status_code_for_service_error()` and `map_database_error_to_service_error()` free functions
- Removed `IntoResponse for database::Error`
**Phase 4 — Crate-level test suite** (`f51406b`)
- 188 tests in errors covering ADR-T-006 §1–§4:
- `auth_error.rs` — 29 tests (status codes, display, `From` impls)
- `user_error.rs` — 54 tests
- `torrent_error.rs` — 73 tests
- `category_tag_error.rs` — 24 tests
- `api_error.rs` — 8 tests (`status_code`/`Display` delegation)
- Added Testing Acceptance Guide to ADR-T-006 (§1–§7) documenting requirements for future error variants
- Fixed four "nota" → "not a" typos in torrent request error messages
### Breaking changes
All public service function signatures now return `Result<T, DomainError>` instead of `Result<T, ServiceError>`.
### What did NOT change
- The HTTP response shape (`{"error": "..."}`) is unchanged
- No new crate dependencies (switched from `derive_more::Error` to `thiserror`, removed `anyhow`)
- No behavioural changes — all existing tests pass
### Stats
- 40 files changed, +2699 / −480 lines
- Primary file: `errors.rs` (+426 / −223)
- New: `006-error-system-refactor.md` (630 lines)
- New: errors (1,168 lines across 5 test modules)
### Testing
- `cargo test --workspace --all-targets --all-features` ✅
- `cargo test --workspace --all-features --doc` ✅
- `cargo clippy --workspace --all-targets --all-features` ✅
- `--no-default-features` build ✅
ACKs for top commit:
peer-cat:
ACK 19ad039
da2ce7:
ACK 19ad039
Tree-SHA512: 2aa79a6b30f46b49c4dea268a86db89559eb71b8c78b66972d14eb1667c1ab418330c1480327b4bb72ee5ba5adb5b18c0be526f54a95077f669109ff76bbf27a40 files changed
Lines changed: 2720 additions & 485 deletions
File tree
- adr
- src
- bin
- cache
- image
- console/commands
- seeder
- tracker_statistics_importer
- databases
- models
- services
- tests
- errors
- tracker
- utils
- web/api/server/v1
- contexts/torrent
- extractors
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
9 | 9 | | |
10 | 10 | | |
11 | 11 | | |
12 | | - | |
13 | | - | |
14 | | - | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
15 | 15 | | |
16 | 16 | | |
17 | 17 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
10 | 36 | | |
11 | 37 | | |
12 | 38 | | |
| |||
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
35 | 35 | | |
36 | 36 | | |
37 | 37 | | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
38 | 41 | | |
39 | 42 | | |
40 | 43 | | |
41 | 44 | | |
42 | 45 | | |
43 | 46 | | |
44 | | - | |
45 | 47 | | |
46 | 48 | | |
47 | 49 | | |
| |||
52 | 54 | | |
53 | 55 | | |
54 | 56 | | |
55 | | - | |
| 57 | + | |
56 | 58 | | |
57 | 59 | | |
58 | 60 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
126 | 126 | | |
127 | 127 | | |
128 | 128 | | |
| 129 | + | |
129 | 130 | | |
130 | 131 | | |
131 | 132 | | |
| |||
0 commit comments