One error response format: { error, code, details? } (step 1)

- 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>

src/error.rs

@@ -1,14 +1,318 @@
-use actix_web::{error::InternalError, http::StatusCode};
+//! The one error response format (#202):
+//!
+//! ```json
+//! { "error": "Tag name must not be empty", "code": "invalid_tag_name", "details": { } }
+//! ```
+//!
+//! - `error`: English, safe to show a user as is. Clients may translate known
+//!   codes into their own wording.
+//! - `code`: a stable snake_case id for clients to branch on. Never rename one
+//!   a client may depend on; add a new one instead.
+//! - `details` (optional): structured extras for the client.
+//!
+//! Handlers return `Result<_, ApiError>`. An unexpected failure (`anyhow`,
+//! diesel, I/O) converts into a 500 `internal` whose detail is logged and
+//! never sent: error text can hold SQL and library paths.
 
+use actix_web::error::{JsonPayloadError, PathError, QueryPayloadError};
+use actix_web::http::StatusCode;
+use actix_web::{HttpRequest, HttpResponse, ResponseError, web};
+use serde::{Deserialize, Serialize};
+use std::fmt;
+
+/// The JSON body of every error response.
+#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)]
+pub struct ErrorBody {
+    pub error: String,
+    pub code: String,
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub details: Option<serde_json::Value>,
+}
+
+/// An error response. Build one with a status constructor, e.g.
+/// `ApiError::bad_request("invalid_tag_name", "Tag name must not be empty")`.
+#[derive(Debug)]
+pub struct ApiError {
+    status: StatusCode,
+    code: &'static str,
+    message: String,
+    details: Option<serde_json::Value>,
+    /// Fields merged into the top level of the body, for responses that
+    /// carried them before this format existed. New code uses `details`.
+    legacy: Option<serde_json::Map<String, serde_json::Value>>,
+}
+
+pub type ApiResult<T> = Result<T, ApiError>;
+
+/// What a 500 says to the client; the real cause goes to the log.
+const INTERNAL_MESSAGE: &str = "Something went wrong on the server";
+
+impl ApiError {
+    pub fn new(status: StatusCode, code: &'static str, message: impl Into<String>) -> Self {
+        ApiError {
+            status,
+            code,
+            message: message.into(),
+            details: None,
+            legacy: None,
+        }
+    }
+
+    pub fn bad_request(code: &'static str, message: impl Into<String>) -> Self {
+        Self::new(StatusCode::BAD_REQUEST, code, message)
+    }
+
+    pub fn unauthorized(code: &'static str, message: impl Into<String>) -> Self {
+        Self::new(StatusCode::UNAUTHORIZED, code, message)
+    }
+
+    pub fn not_found(code: &'static str, message: impl Into<String>) -> Self {
+        Self::new(StatusCode::NOT_FOUND, code, message)
+    }
+
+    pub fn conflict(code: &'static str, message: impl Into<String>) -> Self {
+        Self::new(StatusCode::CONFLICT, code, message)
+    }
+
+    /// A 500 for an unexpected failure. `cause` is logged, never sent.
+    pub fn internal(cause: impl fmt::Debug) -> Self {
+        log::error!("Internal error: {:?}", cause);
+        Self::new(
+            StatusCode::INTERNAL_SERVER_ERROR,
+            "internal",
+            INTERNAL_MESSAGE,
+        )
+    }
+
+    pub fn with_details(mut self, details: impl Serialize) -> Self {
+        self.details = serde_json::to_value(details).ok();
+        self
+    }
+
+    /// Also put `key` at the top level of the body, where a client written
+    /// before this format reads it. The value is in `details` too.
+    pub fn with_legacy_field(mut self, key: &str, value: impl Serialize) -> Self {
+        let value = serde_json::to_value(value).unwrap_or_default();
+        let details = self
+            .details
+            .get_or_insert_with(|| serde_json::Value::Object(Default::default()));
+        if let Some(map) = details.as_object_mut() {
+            map.insert(key.to_string(), value.clone());
+        }
+        self.legacy
+            .get_or_insert_with(Default::default)
+            .insert(key.to_string(), value);
+        self
+    }
+
+    pub fn code(&self) -> &'static str {
+        self.code
+    }
+
+    pub fn body(&self) -> ErrorBody {
+        ErrorBody {
+            error: self.message.clone(),
+            code: self.code.to_string(),
+            details: self.details.clone(),
+        }
+    }
+}
+
+impl fmt::Display for ApiError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(
+            f,
+            "{} {}: {}",
+            self.status.as_u16(),
+            self.code,
+            self.message
+        )
+    }
+}
+
+impl ResponseError for ApiError {
+    fn status_code(&self) -> StatusCode {
+        self.status
+    }
+
+    fn error_response(&self) -> HttpResponse {
+        let mut body = serde_json::to_value(self.body()).unwrap_or_default();
+        if let (Some(legacy), Some(map)) = (&self.legacy, body.as_object_mut()) {
+            for (k, v) in legacy {
+                map.entry(k.clone()).or_insert_with(|| v.clone());
+            }
+        }
+        HttpResponse::build(self.status).json(body)
+    }
+}
+
+impl From<anyhow::Error> for ApiError {
+    fn from(e: anyhow::Error) -> Self {
+        ApiError::internal(e)
+    }
+}
+
+impl From<diesel::result::Error> for ApiError {
+    fn from(e: diesel::result::Error) -> Self {
+        ApiError::internal(e)
+    }
+}
+
+/// Converts an `anyhow` result into an [`ApiResult`]: an error becomes a 500
+/// `internal` (logged, not sent). Prefer `?` with an `ApiResult` return type
+/// in new handlers.
 pub trait IntoHttpError<T> {
-    fn into_http_internal_err(self) -> Result<T, actix_web::Error>;
+    fn into_http_internal_err(self) -> ApiResult<T>;
 }
 
 impl<T> IntoHttpError<T> for Result<T, anyhow::Error> {
-    fn into_http_internal_err(self) -> Result<T, actix_web::Error> {
-        self.map_err(|e| {
-            log::error!("Map to err: {:?}", e);
-            InternalError::new(e, StatusCode::INTERNAL_SERVER_ERROR).into()
-        })
+    fn into_http_internal_err(self) -> ApiResult<T> {
+        self.map_err(ApiError::internal)
+    }
+}
+
+// ---- request-parsing failures ------------------------------------------
+//
+// actix answers these itself in plain text unless an extractor config says
+// otherwise. Register all three on the App (see `main.rs`).
+
+pub fn json_config() -> web::JsonConfig {
+    web::JsonConfig::default().error_handler(|err: JsonPayloadError, req: &HttpRequest| {
+        log::warn!("JSON body error on {} {}: {}", req.method(), req.uri(), err);
+        let api = match &err {
+            JsonPayloadError::OverflowKnownLength { .. } | JsonPayloadError::Overflow { .. } => {
+                ApiError::new(
+                    StatusCode::PAYLOAD_TOO_LARGE,
+                    "body_too_large",
+                    "The request is too large",
+                )
+            }
+            JsonPayloadError::ContentType => ApiError::bad_request(
+                "invalid_body",
+                "The request body must be JSON (Content-Type: application/json)",
+            ),
+            // serde's message names the field at fault, e.g. "missing field
+            // `file_names`"; it describes the request, not the server.
+            _ => ApiError::bad_request("invalid_body", format!("Invalid request body: {}", err)),
+        };
+        actix_web::error::InternalError::from_response(err, api.error_response()).into()
+    })
+}
+
+pub fn query_config() -> web::QueryConfig {
+    web::QueryConfig::default().error_handler(|err: QueryPayloadError, req: &HttpRequest| {
+        log::warn!("Query error on {} {}: {}", req.method(), req.uri(), err);
+        let api = ApiError::bad_request("invalid_query", format!("Invalid query: {}", err));
+        actix_web::error::InternalError::from_response(err, api.error_response()).into()
+    })
+}
+
+pub fn path_config() -> web::PathConfig {
+    web::PathConfig::default().error_handler(|err: PathError, req: &HttpRequest| {
+        log::warn!("Path error on {} {}: {}", req.method(), req.uri(), err);
+        let api = ApiError::bad_request("invalid_path", format!("Invalid URL: {}", err));
+        actix_web::error::InternalError::from_response(err, api.error_response()).into()
+    })
+}
+
+/// The App's default service: an unknown route answers in the same format.
+pub async fn unknown_route() -> HttpResponse {
+    ApiError::not_found("unknown_route", "No such endpoint").error_response()
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use actix_web::body::MessageBody;
+    use actix_web::{App, test as awtest};
+
+    fn body_of(resp: HttpResponse) -> serde_json::Value {
+        let bytes = resp.into_body().try_into_bytes().unwrap();
+        serde_json::from_slice(&bytes).unwrap()
+    }
+
+    #[test]
+    fn renders_error_and_code() {
+        let resp = ApiError::bad_request("invalid_tag_name", "Tag name must not be empty")
+            .error_response();
+        assert_eq!(resp.status(), StatusCode::BAD_REQUEST);
+        assert_eq!(
+            body_of(resp),
+            serde_json::json!({ "error": "Tag name must not be empty", "code": "invalid_tag_name" })
+        );
+    }
+
+    #[test]
+    fn details_and_legacy_fields() {
+        let resp = ApiError::conflict("tag_name_taken", "Taken")
+            .with_legacy_field("existing_tag", serde_json::json!({ "id": 3 }))
+            .error_response();
+        assert_eq!(
+            body_of(resp),
+            serde_json::json!({
+                "error": "Taken",
+                "code": "tag_name_taken",
+                "details": { "existing_tag": { "id": 3 } },
+                "existing_tag": { "id": 3 },
+            })
+        );
+    }
+
+    #[test]
+    fn internal_errors_never_send_their_cause() {
+        let cause = anyhow::anyhow!("SELECT * FROM image_exif WHERE rel_path = '/secret/a.jpg'");
+        let resp = ApiError::from(cause).error_response();
+        assert_eq!(resp.status(), StatusCode::INTERNAL_SERVER_ERROR);
+        let body = body_of(resp);
+        assert_eq!(body["code"], "internal");
+        assert!(!body.to_string().contains("secret"), "{body}");
+    }
+
+    #[derive(Deserialize)]
+    struct Q {
+        #[allow(dead_code)]
+        n: i32,
+    }
+
+    #[actix_rt::test]
+    async fn request_parsing_failures_use_the_format() {
+        let app = awtest::init_service(
+            App::new()
+                .app_data(json_config())
+                .app_data(query_config())
+                .app_data(path_config())
+                .route("/j", web::post().to(|_: web::Json<Q>| async { "ok" }))
+                .route("/q", web::get().to(|_: web::Query<Q>| async { "ok" }))
+                .route("/p/{n}", web::get().to(|_: web::Path<i32>| async { "ok" }))
+                .default_service(web::to(unknown_route)),
+        )
+        .await;
+
+        for (req, status, code) in [
+            (
+                awtest::TestRequest::post()
+                    .uri("/j")
+                    .set_json(serde_json::json!({})),
+                400,
+                "invalid_body",
+            ),
+            (
+                awtest::TestRequest::get().uri("/q?n=x"),
+                400,
+                "invalid_query",
+            ),
+            (awtest::TestRequest::get().uri("/p/x"), 400, "invalid_path"),
+            (
+                awtest::TestRequest::get().uri("/nope"),
+                404,
+                "unknown_route",
+            ),
+        ] {
+            let resp = awtest::call_service(&app, req.to_request()).await;
+            assert_eq!(resp.status().as_u16(), status);
+            let body: ErrorBody = awtest::read_body_json(resp).await;
+            assert_eq!(body.code, code);
+            assert!(!body.error.is_empty());
+        }
     }
 }