Step 1 of #202. Stacked on #201: this branch includes #201's commit until it merges; review the second commit.
The format
{"error":"Tag name must not be empty","code":"invalid_tag_name","details":{}}
error is English and safe to show a user as is; code is stable for clients to branch on (or translate); details is optional. Documented in src/error.rs.
What changed
error::ApiError (ResponseError): bad_request / unauthorized / not_found / conflict / new(status, …), plus with_details and with_legacy_field (keeps a pre-format top-level field, also copied into details). ApiResult<T> for handlers.
Unexpected errors never reach the client.From<anyhow::Error>, From<diesel::result::Error> and into_http_internal_err all become 500 internal with a generic message; the cause goes to the log. This also covers the 11 existing into_http_internal_err sites, which used to send the anyhow text.
Framework errors in the format: JSON body (invalid_body, body_too_large), query (invalid_query), path (invalid_path), the Claims extractor (unauthorized, previously text "Bad token"), unknown routes (unknown_route, previously an empty 404).
error::tests: shape, legacy fields, an internal cause never appears in the body, each framework failure through a real App.
Tag tests assert codes; rename conflict keeps existing_tag top-level. Full suite 1056 passing, clippy clean.
Over HTTP on the e2e server: no token / bad token 401, unknown route 404, bad query / body / path 400, rename conflict 409, removing an unknown tag 404; tags, photos and Enhance templates still 200. (/metrics is 404 before and after: no endpoint is configured on the Prometheus middleware.)
Not in this PR
Step 2: the ~78 plain-text and ~95 empty-body sites in other modules (faces, handlers/image, duplicates, …). The FileViewer side is FileViewer-React #93.
Step 1 of #202. **Stacked on #201**: this branch includes #201's commit until it merges; review the second commit.
## The format
```json
{ "error": "Tag name must not be empty", "code": "invalid_tag_name", "details": { } }
```
`error` is English and safe to show a user as is; `code` is stable for clients to branch on (or translate); `details` is optional. Documented in `src/error.rs`.
## What changed
- **`error::ApiError`** (`ResponseError`): `bad_request` / `unauthorized` / `not_found` / `conflict` / `new(status, …)`, plus `with_details` and `with_legacy_field` (keeps a pre-format top-level field, also copied into `details`). `ApiResult<T>` for handlers.
- **Unexpected errors never reach the client.** `From<anyhow::Error>`, `From<diesel::result::Error>` and `into_http_internal_err` all become 500 `internal` with a generic message; the cause goes to the log. This also covers the 11 existing `into_http_internal_err` sites, which used to send the anyhow text.
- **Framework errors** in the format: JSON body (`invalid_body`, `body_too_large`), query (`invalid_query`), path (`invalid_path`), the `Claims` extractor (`unauthorized`, previously text "Bad token"), unknown routes (`unknown_route`, previously an empty 404).
- **Tags** fully on `ApiResult`. Codes: `invalid_tag_name`, `tag_not_found`, `tag_name_taken` (keeps `existing_tag` top-level), `no_files`, `too_many_files` (`details.max`), `nothing_to_change`, `tag_added_and_removed` (`details.tag`).
- **Enhance** builds through `ApiError`. Codes unchanged; `internal` no longer sends its message.
- **duplicates**: 8 × 500 with `format!("{:?}", e)` bodies (SQL / paths) fixed.
## Testing
- `error::tests`: shape, legacy fields, an internal cause never appears in the body, each framework failure through a real `App`.
- Tag tests assert codes; rename conflict keeps `existing_tag` top-level. Full suite 1056 passing, clippy clean.
- Over HTTP on the e2e server: no token / bad token 401, unknown route 404, bad query / body / path 400, rename conflict 409, removing an unknown tag 404; tags, photos and Enhance templates still 200. (`/metrics` is 404 before and after: no endpoint is configured on the Prometheus middleware.)
## Not in this PR
Step 2: the ~78 plain-text and ~95 empty-body sites in other modules (`faces`, `handlers/image`, `duplicates`, …). The FileViewer side is FileViewer-React #93.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
- POST /image/tags/bulk { file_names, add: [names], remove: [ids] }: one
transaction over many photos; names match case-insensitively and are
created when missing; 400 if a tag is both added and removed.
- Adding a tag a photo already has is a no-op (was a 500 on
UNIQUE(rel_path, tag_id)).
- Adding a tag no photo carries reuses it (the lookup only saw tags in
use, so create_tag hit idx_tags_name_nocase: 500). update_tags could
likewise skip such ids silently.
- Removing a tag matches the name case-insensitively and deletes it from
every path sharing the photo's content, as reads union them; it used to
answer 200 and leave the tag on the other copy.
- update_tags returns errors instead of panicking with the TagDao lock
held, which poisoned it for every later tag request.
Closes#200
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- error::ApiError (ResponseError): status + stable snake_case code +
display-safe message + optional details. From<anyhow>/From<diesel> and
into_http_internal_err become a 500 "internal" whose cause is logged,
never sent.
- JSON body, query and path parse failures, the Claims 401 and unknown
routes now answer in the format instead of actix's plain text.
- tags handlers return ApiResult with codes (invalid_tag_name,
tag_not_found, tag_name_taken, no_files, too_many_files,
nothing_to_change, tag_added_and_removed). The rename 409 keeps
existing_tag at the top level for older clients.
- Enhance's error_response builds through ApiError; its codes are
unchanged, and an internal error no longer sends its message.
- duplicates: eight 500s sent format!("{:?}", e) to the client.
Refs #202
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
cameron
merged commit 1836f1b9ca into master2026-11-01 18:05:55 +00:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Step 1 of #202. Stacked on #201: this branch includes #201's commit until it merges; review the second commit.
The format
erroris English and safe to show a user as is;codeis stable for clients to branch on (or translate);detailsis optional. Documented insrc/error.rs.What changed
error::ApiError(ResponseError):bad_request/unauthorized/not_found/conflict/new(status, …), pluswith_detailsandwith_legacy_field(keeps a pre-format top-level field, also copied intodetails).ApiResult<T>for handlers.From<anyhow::Error>,From<diesel::result::Error>andinto_http_internal_errall become 500internalwith a generic message; the cause goes to the log. This also covers the 11 existinginto_http_internal_errsites, which used to send the anyhow text.invalid_body,body_too_large), query (invalid_query), path (invalid_path), theClaimsextractor (unauthorized, previously text "Bad token"), unknown routes (unknown_route, previously an empty 404).ApiResult. Codes:invalid_tag_name,tag_not_found,tag_name_taken(keepsexisting_tagtop-level),no_files,too_many_files(details.max),nothing_to_change,tag_added_and_removed(details.tag).ApiError. Codes unchanged;internalno longer sends its message.format!("{:?}", e)bodies (SQL / paths) fixed.Testing
error::tests: shape, legacy fields, an internal cause never appears in the body, each framework failure through a realApp.existing_tagtop-level. Full suite 1056 passing, clippy clean./metricsis 404 before and after: no endpoint is configured on the Prometheus middleware.)Not in this PR
Step 2: the ~78 plain-text and ~95 empty-body sites in other modules (
faces,handlers/image,duplicates, …). The FileViewer side is FileViewer-React #93.🤖 Generated with Claude Code
- POST /image/tags/bulk { file_names, add: [names], remove: [ids] }: one transaction over many photos; names match case-insensitively and are created when missing; 400 if a tag is both added and removed. - Adding a tag a photo already has is a no-op (was a 500 on UNIQUE(rel_path, tag_id)). - Adding a tag no photo carries reuses it (the lookup only saw tags in use, so create_tag hit idx_tags_name_nocase: 500). update_tags could likewise skip such ids silently. - Removing a tag matches the name case-insensitively and deletes it from every path sharing the photo's content, as reads union them; it used to answer 200 and leave the tag on the other copy. - update_tags returns errors instead of panicking with the TagDao lock held, which poisoned it for every later tag request. Closes #200 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>- error::ApiError (ResponseError): status + stable snake_case code + display-safe message + optional details. From<anyhow>/From<diesel> and into_http_internal_err become a 500 "internal" whose cause is logged, never sent. - JSON body, query and path parse failures, the Claims 401 and unknown routes now answer in the format instead of actix's plain text. - tags handlers return ApiResult with codes (invalid_tag_name, tag_not_found, tag_name_taken, no_files, too_many_files, nothing_to_change, tag_added_and_removed). The rename 409 keeps existing_tag at the top level for older clients. - Enhance's error_response builds through ApiError; its codes are unchanged, and an internal error no longer sends its message. - duplicates: eight 500s sent format!("{:?}", e) to the client. Refs #202 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>