From 431c3a749232362b3531929de5e324eb42771c6e Mon Sep 17 00:00:00 2001 From: Cameron Cordes Date: Sun, 1 Nov 2026 13:29:13 -0500 Subject: [PATCH] Every error response in the { error, code, details? } format (step 2) Moves every remaining hand-built error response onto ApiError: ~300 sites across faces, image, video, files, memories, versions, duplicates, favorites, libraries, auth, clip/unified search, prefs, reels, tts, insight chat, persona chat, knowledge and personas. - 500s: the body was the error's text (anyhow / diesel / {:?}); now ApiError::internal logs it and sends a generic message. - Each 4xx/502/503 has a stable code and a message written for users; statuses are unchanged. Shared helpers per module (faces_unavailable, person_not_found, unknown_library, insight_not_found, ...). - Pre-format top-level fields kept where clients read them: upload 409 duplicate_of / content_hash / library_name. - PersonaChatError converts into ApiError in one place. - Multipart upload errors use the format (error::multipart_config). - Guard test: no non-test code builds HttpResponse::() by hand. Closes #202 Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 9 +++ src/ai/handlers.rs | 166 ++++++++++++++++++-------------------- src/ai/persona_chat.rs | 110 +++++++++++++------------ src/ai/tts.rs | 110 ++++++++++++++----------- src/auth.rs | 18 +++-- src/clip_search.rs | 52 ++++++------ src/data/mod.rs | 3 +- src/duplicates.rs | 41 +++++----- src/error.rs | 115 ++++++++++++++++++++++++++ src/faces.rs | 136 ++++++++++++++++++++++--------- src/files.rs | 34 ++++---- src/handlers/favorites.rs | 37 +++++---- src/handlers/image.rs | 95 +++++++++++++++------- src/handlers/video.rs | 63 ++++++++++----- src/knowledge.rs | 147 +++++++++++++++++---------------- src/libraries.rs | 44 ++++++++-- src/main.rs | 3 +- src/memories.rs | 9 ++- src/personas.rs | 74 +++++++++-------- src/prefs.rs | 43 +++++----- src/reels/mod.rs | 55 ++++++++----- src/unified_search.rs | 38 ++++----- src/versions/handlers.rs | 34 +++++--- 23 files changed, 894 insertions(+), 542 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 5a4b689..89eac4e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -797,6 +797,15 @@ GET /timezones (IANA zone list from chrono-tz; unauthentic **Service Builder Pattern:** Routes are registered via composable `ServiceBuilder` trait in `service.rs`. Allows modular feature addition. +**Error responses (`src/error.rs`, #202):** +Every error body is `{"error": "", "code": "", "details"?: {...}}`. +Build them with `ApiError` (`bad_request` / `not_found` / `conflict` / `unavailable` / `bad_gateway` / ... `(code, message)`), +returned as `Err(..)` from an `ApiResult` handler or via `.into_response()`. Unexpected failures go through +`ApiError::internal(cause)` (or `?` on anyhow / diesel): a 500 `internal` whose cause is logged, never sent, since it can +hold SQL and library paths. Never rename a code a client may branch on; add a new one. Don't put file paths in `details`. +Request-parsing failures, the auth extractor, multipart uploads and unknown routes use the format too (configs in +`error.rs`, registered in `main.rs`). A test fails if a handler builds `HttpResponse::NotFound()` etc. by hand. + **Path Validation:** Always use `is_valid_full_path(&base_path, &requested_path, check_exists)` to prevent directory traversal attacks. diff --git a/src/ai/handlers.rs b/src/ai/handlers.rs index b1ec0ea..4f71399 100644 --- a/src/ai/handlers.rs +++ b/src/ai/handlers.rs @@ -1,3 +1,4 @@ +use crate::error::ApiError; use actix_web::{HttpResponse, Responder, delete, get, post, web}; use futures::StreamExt; use opentelemetry::trace::Status; @@ -110,15 +111,12 @@ pub async fn generation_status_handler( }); } Ok(None) => { - return HttpResponse::NotFound().json(serde_json::json!({ - "error": format!("Job {} not found", jid) - })); + return ApiError::not_found("job_not_found", "That insight job no longer exists") + .into_response(); } Err(e) => { log::error!("Failed to look up job {}: {:?}", jid, e); - return HttpResponse::InternalServerError().json(serde_json::json!({ - "error": "Failed to look up job" - })); + return ApiError::internal("Failed to look up job").into_response(); } } } @@ -153,16 +151,12 @@ pub async fn generation_status_handler( } Err(e) => { log::error!("Failed to look up active job for {}: {:?}", normalized, e); - return HttpResponse::InternalServerError().json(serde_json::json!({ - "error": "Failed to look up active job" - })); + return ApiError::internal("Failed to look up active job").into_response(); } } } - HttpResponse::BadRequest().json(serde_json::json!({ - "error": "Provide either job_id or path query parameter" - })) + ApiError::bad_request("invalid_request", "Give a job_id or a path").into_response() } #[derive(Debug, Deserialize)] @@ -212,9 +206,7 @@ pub async fn cancel_generation_handler( } Err(e) => { log::error!("Failed to cancel job {}: {:?}", jid, e); - return HttpResponse::InternalServerError().json(serde_json::json!({ - "error": "Failed to cancel job" - })); + return ApiError::internal("Failed to cancel job").into_response(); } } } @@ -267,9 +259,7 @@ pub async fn cancel_generation_handler( })); } - HttpResponse::BadRequest().json(serde_json::json!({ - "error": "Provide either job_id or file_path in the request body" - })) + ApiError::bad_request("invalid_request", "Give a job_id or a file_path").into_response() } #[derive(Debug, Deserialize)] @@ -491,9 +481,7 @@ pub async fn generate_insight_handler( Err(e) => { log::error!("Failed to create generation job: {:?}", e); span.set_status(Status::error("Failed to create generation job")); - return HttpResponse::InternalServerError().json(serde_json::json!({ - "error": "Failed to create generation job" - })); + return ApiError::internal("Failed to create generation job").into_response(); } } }; @@ -640,14 +628,10 @@ pub async fn get_insight_handler( match dao.get_insight_for_paths(&sibling_paths) { Ok(Some(insight)) => HttpResponse::Ok().json(PhotoInsightResponse::from(insight)), - Ok(None) => HttpResponse::NotFound().json(serde_json::json!({ - "error": "Insight not found" - })), + Ok(None) => insight_not_found().into_response(), Err(e) => { log::error!("Failed to fetch insight ({}): {:?}", &query.path, e); - HttpResponse::InternalServerError().json(serde_json::json!({ - "error": format!("Failed to fetch insight: {:?}", e) - })) + ApiError::internal(format!("Failed to fetch insight: {:?}", e)).into_response() } } } @@ -671,9 +655,7 @@ pub async fn delete_insight_handler( })), Err(e) => { log::error!("Failed to delete insight: {:?}", e); - HttpResponse::InternalServerError().json(serde_json::json!({ - "error": format!("Failed to delete insight: {:?}", e) - })) + ApiError::internal(format!("Failed to delete insight: {:?}", e)).into_response() } } } @@ -699,9 +681,7 @@ pub async fn get_all_insights_handler( } Err(e) => { log::error!("Failed to fetch all insights: {:?}", e); - HttpResponse::InternalServerError().json(serde_json::json!({ - "error": format!("Failed to fetch insights: {:?}", e) - })) + ApiError::internal(format!("Failed to fetch insights: {:?}", e)).into_response() } } } @@ -731,9 +711,7 @@ pub async fn get_insight_history_handler( } Err(e) => { log::error!("Failed to fetch insight history ({}): {:?}", &query.path, e); - HttpResponse::InternalServerError().json(serde_json::json!({ - "error": format!("Failed to fetch insight history: {:?}", e) - })) + ApiError::internal(format!("Failed to fetch insight history: {:?}", e)).into_response() } } } @@ -822,9 +800,7 @@ pub async fn generate_agentic_insight_handler( Err(e) => { log::error!("Failed to create agentic generation job: {:?}", e); span.set_status(Status::error("Failed to create generation job")); - return HttpResponse::InternalServerError().json(serde_json::json!({ - "error": "Failed to create generation job" - })); + return ApiError::internal("Failed to create generation job").into_response(); } } }; @@ -1159,9 +1135,7 @@ pub async fn rate_insight_handler( })), Err(e) => { log::error!("Failed to rate insight: {:?}", e); - HttpResponse::InternalServerError().json(serde_json::json!({ - "error": format!("Failed to rate insight: {:?}", e) - })) + ApiError::internal(format!("Failed to rate insight: {:?}", e)).into_response() } } } @@ -1213,9 +1187,7 @@ pub async fn export_training_data_handler( } Err(e) => { log::error!("Failed to export training data: {:?}", e); - HttpResponse::InternalServerError().json(serde_json::json!({ - "error": format!("Failed to export training data: {:?}", e) - })) + ApiError::internal(format!("Failed to export training data: {:?}", e)).into_response() } } } @@ -1308,9 +1280,7 @@ pub async fn chat_turn_handler( Ok(Some(lib)) => lib, Ok(None) => app_state.primary_library(), Err(e) => { - return HttpResponse::BadRequest().json(serde_json::json!({ - "error": format!("invalid library: {}", e) - })); + return libraries::unknown_library(e).into_response(); } }; @@ -1361,17 +1331,17 @@ pub async fn chat_turn_handler( // Map well-known errors to client-facing 4xx codes. if msg.contains("no insight found") { - HttpResponse::NotFound().json(serde_json::json!({ "error": msg })) + insight_not_found().into_response() } else if msg.contains("no chat history") { - HttpResponse::Conflict().json(serde_json::json!({ "error": msg })) + no_chat_history().into_response() } else if msg.contains("user_message") || msg.contains("unknown backend") || msg.contains("switching from local to hybrid") || msg.contains("hybrid backend unavailable") { - HttpResponse::BadRequest().json(serde_json::json!({ "error": msg })) + ApiError::bad_request("invalid_chat_request", msg).into_response() } else { - HttpResponse::InternalServerError().json(serde_json::json!({ "error": msg })) + ApiError::internal(msg).into_response() } } } @@ -1456,9 +1426,7 @@ pub async fn chat_rewind_handler( Ok(Some(lib)) => lib, Ok(None) => app_state.primary_library(), Err(e) => { - return HttpResponse::BadRequest().json(serde_json::json!({ - "error": format!("invalid library: {}", e) - })); + return libraries::unknown_library(e).into_response(); } }; @@ -1476,13 +1444,18 @@ pub async fn chat_rewind_handler( let msg = format!("{}", e); log::error!("Chat rewind failed: {}", msg); if msg.contains("no insight found") { - HttpResponse::NotFound().json(serde_json::json!({ "error": msg })) + insight_not_found().into_response() } else if msg.contains("no chat history") { - HttpResponse::Conflict().json(serde_json::json!({ "error": msg })) + no_chat_history().into_response() } else if msg.contains("cannot discard the initial") || msg.contains("out of range") { - HttpResponse::BadRequest().json(serde_json::json!({ "error": msg })) + ApiError::bad_request( + "stale_branch", + "This conversation changed. Reload it and try again.", + ) + .with_details(serde_json::json!({ "reason": msg })) + .into_response() } else { - HttpResponse::InternalServerError().json(serde_json::json!({ "error": msg })) + ApiError::internal(msg).into_response() } } } @@ -1553,11 +1526,11 @@ pub async fn chat_history_handler( Err(e) => { let msg = format!("{}", e); if msg.contains("no insight found") { - HttpResponse::NotFound().json(serde_json::json!({ "error": msg })) + insight_not_found().into_response() } else if msg.contains("no chat history") { - HttpResponse::Conflict().json(serde_json::json!({ "error": msg })) + no_chat_history().into_response() } else { - HttpResponse::InternalServerError().json(serde_json::json!({ "error": msg })) + ApiError::internal(msg).into_response() } } } @@ -1606,14 +1579,18 @@ pub async fn chat_branches_handler( Err(e) => { let msg = format!("{}", e); if msg.contains("no insight found") { - return HttpResponse::NotFound().json(serde_json::json!({ "error": msg })); + return insight_not_found().into_response(); } else if msg.contains("no chat history") { - return HttpResponse::Conflict().json(serde_json::json!({ "error": msg })); + return no_chat_history().into_response(); } else if msg.contains("not found in tree") { - return HttpResponse::BadRequest().json(serde_json::json!({ "error": msg })); + return ApiError::bad_request( + "stale_branch", + "This conversation changed. Reload it and try again.", + ) + .with_details(serde_json::json!({ "reason": msg })) + .into_response(); } else { - return HttpResponse::InternalServerError() - .json(serde_json::json!({ "error": msg })); + return ApiError::internal(msg).into_response(); } } }; @@ -1638,9 +1615,7 @@ pub async fn chat_switch_branch_handler( Ok(Some(lib)) => lib, Ok(None) => app_state.primary_library(), Err(e) => { - return HttpResponse::BadRequest().json(serde_json::json!({ - "error": format!("invalid library: {}", e) - })); + return libraries::unknown_library(e).into_response(); } }; @@ -1654,13 +1629,18 @@ pub async fn chat_switch_branch_handler( let msg = format!("{}", e); log::error!("Switch branch failed: {}", msg); if msg.contains("no insight found") { - HttpResponse::NotFound().json(serde_json::json!({ "error": msg })) + insight_not_found().into_response() } else if msg.contains("no chat history") { - HttpResponse::Conflict().json(serde_json::json!({ "error": msg })) + no_chat_history().into_response() } else if msg.contains("not found") || msg.contains("not a leaf") { - HttpResponse::BadRequest().json(serde_json::json!({ "error": msg })) + ApiError::bad_request( + "stale_branch", + "This conversation changed. Reload it and try again.", + ) + .with_details(serde_json::json!({ "reason": msg })) + .into_response() } else { - HttpResponse::InternalServerError().json(serde_json::json!({ "error": msg })) + ApiError::internal(msg).into_response() } } } @@ -1698,9 +1678,7 @@ pub async fn chat_stream_handler( Ok(Some(lib)) => lib, Ok(None) => app_state.primary_library(), Err(e) => { - return HttpResponse::BadRequest().json(serde_json::json!({ - "error": format!("invalid library: {}", e) - })); + return libraries::unknown_library(e).into_response(); } }; @@ -1853,9 +1831,7 @@ pub async fn turn_async_handler( Ok(Some(lib)) => lib, Ok(None) => app_state.primary_library(), Err(e) => { - return HttpResponse::BadRequest().json(serde_json::json!({ - "error": format!("invalid library: {}", e) - })); + return libraries::unknown_library(e).into_response(); } }; @@ -1936,9 +1912,7 @@ pub(crate) async fn turn_replay_impl( Some(e) => e, None => { span.set_status(Status::error("turn not found")); - return HttpResponse::NotFound().json(serde_json::json!({ - "error": format!("turn {} not found", turn_id) - })); + return turn_not_found().into_response(); } }; @@ -1953,9 +1927,11 @@ pub(crate) async fn turn_replay_impl( let (initial_frames, start_skip) = match entry.replay_from(skip_before).await { ReplayOutcome::Gone => { span.set_status(Status::error("buffer evicted")); - return HttpResponse::Gone().json(serde_json::json!({ - "error": "turn history has expired (buffer evicted)" - })); + return ApiError::gone( + "turn_expired", + "This reply is too old to resume. Reload the chat.", + ) + .into_response(); } ReplayOutcome::CaughtUp { next_skip } => (Vec::new(), next_skip), ReplayOutcome::Events { events, next_skip } => { @@ -2100,9 +2076,7 @@ pub(crate) async fn cancel_turn_impl( Some(e) => e, None => { span.set_status(Status::error("turn not found")); - return HttpResponse::NotFound().json(serde_json::json!({ - "error": format!("turn {} not found", turn_id) - })); + return turn_not_found().into_response(); } }; @@ -2441,3 +2415,17 @@ mod owner_defaults_tests { assert_eq!(persona.as_deref(), Some("journal")); } } + +// ---- error responses (#202) --------------------------------------------- + +fn insight_not_found() -> ApiError { + ApiError::not_found("insight_not_found", "There's no insight for this photo yet") +} + +fn no_chat_history() -> ApiError { + ApiError::conflict("no_chat_history", "There's no chat for this insight yet") +} + +fn turn_not_found() -> ApiError { + ApiError::not_found("turn_not_found", "That reply is no longer running") +} diff --git a/src/ai/persona_chat.rs b/src/ai/persona_chat.rs index 4c27fc3..a0252b1 100644 --- a/src/ai/persona_chat.rs +++ b/src/ai/persona_chat.rs @@ -22,6 +22,7 @@ //! registry. //! - The HTTP handlers under ` /persona_chat/*`. +use crate::error::ApiError; use actix_web::{HttpResponse, Responder, delete, get, post, web}; use anyhow::{Context, Result, anyhow, bail}; use chrono::Utc; @@ -85,6 +86,39 @@ impl std::fmt::Display for PersonaChatError { impl std::error::Error for PersonaChatError {} +impl From<&PersonaChatError> for ApiError { + fn from(e: &PersonaChatError) -> Self { + match e { + PersonaChatError::UnknownPersona(_) => { + ApiError::not_found("persona_not_found", "That persona no longer exists") + } + PersonaChatError::UnknownConversation(_) => ApiError::not_found( + "conversation_not_found", + "That conversation no longer exists", + ), + PersonaChatError::EmptyMessage => { + ApiError::bad_request("empty_message", "Type a message first") + } + PersonaChatError::MessageTooLong => ApiError::bad_request( + "message_too_long", + "That message is too long (8192 characters at most)", + ), + PersonaChatError::ConcurrentTurn => ApiError::conflict( + "turn_in_progress", + "A reply is still being written. Wait for it to finish.", + ), + PersonaChatError::SmsThread(detail) => { + log::warn!("persona chat: SMS thread unreadable: {detail}"); + ApiError::bad_gateway( + "sms_thread_unavailable", + "Couldn't read that message thread", + ) + } + PersonaChatError::Db(err) => ApiError::internal(err), + } + } +} + impl From for PersonaChatError { fn from(e: anyhow::Error) -> Self { PersonaChatError::Db(e) @@ -1463,12 +1497,14 @@ pub async fn persona_chat_history_handler( ) { Ok(view) => HttpResponse::Ok().json(view), Err(e) => { - let msg = format!("{e}"); - if msg.contains("not found") { - HttpResponse::NotFound().json(serde_json::json!({ "error": msg })) + if format!("{e}").contains("not found") { + ApiError::not_found( + "conversation_not_found", + "That conversation no longer exists", + ) + .into_response() } else { - log::error!("persona chat history load failed: {msg}"); - HttpResponse::InternalServerError().json(serde_json::json!({ "error": msg })) + ApiError::internal(e).into_response() } } } @@ -1494,11 +1530,7 @@ pub async fn persona_chat_turn_handler( match libraries::resolve_library_param_state(&app_state, request.library.as_deref()) { Ok(Some(lib)) => lib, Ok(None) => app_state.primary_library(), - Err(e) => { - return HttpResponse::BadRequest().json(serde_json::json!({ - "error": format!("invalid library: {e}") - })); - } + Err(e) => return crate::libraries::unknown_library(e).into_response(), }; let user_id = claims.sub.parse::().unwrap_or(1); @@ -1519,21 +1551,7 @@ pub async fn persona_chat_turn_handler( } Err(e) => { span.set_status(Status::error(format!("{e}"))); - match &e { - PersonaChatError::UnknownPersona(_) | PersonaChatError::UnknownConversation(_) => { - HttpResponse::NotFound() - } - PersonaChatError::EmptyMessage | PersonaChatError::MessageTooLong => { - HttpResponse::BadRequest() - } - PersonaChatError::ConcurrentTurn => HttpResponse::Conflict(), - PersonaChatError::SmsThread(_) => HttpResponse::BadGateway(), - PersonaChatError::Db(_) => { - log::error!("persona chat dispatch failed: {e}"); - HttpResponse::InternalServerError() - } - } - .json(serde_json::json!({ "error": format!("{e}") })) + ApiError::from(&e).into_response() } } } @@ -1567,20 +1585,7 @@ pub async fn persona_chat_create_conversation_handler( Ok(conversation_id) => HttpResponse::Created().json(serde_json::json!({ "conversation_id": conversation_id, })), - Err(e) => match &e { - PersonaChatError::UnknownPersona(_) => { - HttpResponse::NotFound().json(serde_json::json!({ "error": format!("{e}") })) - } - PersonaChatError::SmsThread(_) => { - log::warn!("anchored persona chat create failed: {e}"); - HttpResponse::BadGateway().json(serde_json::json!({ "error": format!("{e}") })) - } - _ => { - log::error!("persona chat create failed: {e}"); - HttpResponse::InternalServerError() - .json(serde_json::json!({ "error": format!("{e}") })) - } - }, + Err(e) => ApiError::from(&e).into_response(), } } @@ -1600,9 +1605,7 @@ pub async fn persona_chat_delete_conversation_handler( Ok(()) => HttpResponse::Ok().json(serde_json::json!({ "deleted": true })), Err(e) => { log::error!("persona chat delete failed: {e}"); - HttpResponse::InternalServerError().json(serde_json::json!({ - "error": format!("{e}") - })) + ApiError::internal(format!("{e}")).into_response() } } } @@ -1626,9 +1629,7 @@ pub async fn persona_chat_conversations_handler( })), Err(e) => { log::error!("persona chat conversation list failed: {e}"); - HttpResponse::InternalServerError().json(serde_json::json!({ - "error": format!("{e}") - })) + ApiError::internal(format!("{e}")).into_response() } } } @@ -1669,20 +1670,29 @@ pub struct PersonaChatBranchesQuery { /// equivalents return, so the client can share its error handling. fn branch_error_response(context: &str, e: &anyhow::Error) -> HttpResponse { let msg = format!("{e}"); - log::error!("{context}: {msg}"); + log::warn!("{context}: {msg}"); if msg.contains("conversation not found") { - HttpResponse::NotFound().json(serde_json::json!({ "error": msg })) + ApiError::not_found( + "conversation_not_found", + "That conversation no longer exists", + ) } else if msg.contains("no chat history") { - HttpResponse::Conflict().json(serde_json::json!({ "error": msg })) + ApiError::conflict("no_chat_history", "There's no chat here yet") } else if msg.contains("not found") || msg.contains("not a leaf") || msg.contains("out of range") || msg.contains("cannot rewind past") { - HttpResponse::BadRequest().json(serde_json::json!({ "error": msg })) + // The client's view of the branches is stale. + ApiError::bad_request( + "stale_branch", + "This conversation changed. Reload it and try again.", + ) + .with_details(serde_json::json!({ "reason": msg })) } else { - HttpResponse::InternalServerError().json(serde_json::json!({ "error": msg })) + ApiError::internal(msg) } + .into_response() } /// POST /persona_chat/rewind — move the tail of the conversation off the diff --git a/src/ai/tts.rs b/src/ai/tts.rs index 7cc3c5b..d0f1ffc 100644 --- a/src/ai/tts.rs +++ b/src/ai/tts.rs @@ -5,6 +5,7 @@ // voice from either an uploaded clip (device) or an existing library file // (audio read directly; video has its audio track extracted via ffmpeg). +use crate::error::ApiError; use actix_multipart::Multipart; use actix_web::{HttpResponse, Responder, delete, get, post, web}; use anyhow::Context; @@ -386,12 +387,11 @@ pub async fn tts_speech_handler( let text = prepare_for_tts(&req.text); if text.is_empty() { span.set_status(Status::error("text is required")); - return HttpResponse::BadRequest().json(json!({ "error": "text is required" })); + return ApiError::bad_request("empty_text", "There's no text to read").into_response(); } let Some(client) = app_state.llamacpp.as_ref() else { span.set_status(Status::error("tts backend not configured")); - return HttpResponse::ServiceUnavailable() - .json(json!({ "error": "TTS backend not configured (set llamacpp.url in llm.toml)" })); + return tts_unavailable().into_response(); }; let format = req @@ -419,9 +419,11 @@ pub async fn tts_speech_handler( // One synthesis at a time (see TTS_PERMIT) — fast-fail when busy. let Ok(_permit) = TTS_PERMIT.try_acquire() else { span.set_status(Status::error("tts busy")); - return HttpResponse::TooManyRequests().json(json!({ - "error": "TTS is busy with another request — try again shortly" - })); + return ApiError::too_many_requests( + "tts_busy", + "Already reading something. Try again shortly.", + ) + .into_response(); }; // Wait for the LLM side to release the GPU before sending — the synthesis @@ -444,7 +446,7 @@ pub async fn tts_speech_handler( Err(e) => { span.set_status(Status::error("tts synthesis failed")); log::error!("TTS synth failed: {:?}", e); - HttpResponse::BadGateway().json(json!({ "error": format!("TTS failed: {e}") })) + tts_failed(&e).into_response() } } } @@ -516,12 +518,11 @@ pub async fn create_speech_job_handler( let text = prepare_for_tts(&req.text); if text.is_empty() { span.set_status(Status::error("text is required")); - return HttpResponse::BadRequest().json(json!({ "error": "text is required" })); + return ApiError::bad_request("empty_text", "There's no text to read").into_response(); } if app_state.llamacpp.is_none() { span.set_status(Status::error("tts backend not configured")); - return HttpResponse::ServiceUnavailable() - .json(json!({ "error": "TTS backend not configured (set llamacpp.url in llm.toml)" })); + return tts_unavailable().into_response(); } let format = req @@ -660,7 +661,7 @@ pub async fn speech_job_status_handler(_claims: Claims, path: web::Path) let Ok(id) = Uuid::parse_str(&path.into_inner()) else { span.set_status(Status::error("invalid job id")); - return HttpResponse::BadRequest().json(json!({ "error": "invalid job id" })); + return ApiError::bad_request("invalid_job_id", "Invalid read-aloud job").into_response(); }; let resp = { let jobs = TTS_JOBS.lock().unwrap(); @@ -679,8 +680,7 @@ pub async fn speech_job_status_handler(_claims: Claims, path: web::Path) } None => { span.set_status(Status::error("job not found")); - HttpResponse::NotFound() - .json(json!({ "error": "TTS job not found (it may have expired)" })) + ApiError::not_found("job_not_found", "That read-aloud job has expired").into_response() } } } @@ -696,7 +696,7 @@ pub async fn cancel_speech_job_handler(_claims: Claims, path: web::Path) let Ok(id) = Uuid::parse_str(&path.into_inner()) else { span.set_status(Status::error("invalid job id")); - return HttpResponse::BadRequest().json(json!({ "error": "invalid job id" })); + return ApiError::bad_request("invalid_job_id", "Invalid read-aloud job").into_response(); }; let status = with_job(id, |job| { if !job.status.is_terminal() { @@ -715,8 +715,7 @@ pub async fn cancel_speech_job_handler(_claims: Claims, path: web::Path) } None => { span.set_status(Status::error("job not found")); - HttpResponse::NotFound() - .json(json!({ "error": "TTS job not found (it may have expired)" })) + ApiError::not_found("job_not_found", "That read-aloud job has expired").into_response() } } } @@ -753,8 +752,7 @@ pub async fn list_voices_handler( let Some(client) = app_state.llamacpp.as_ref() else { span.set_status(Status::error("tts backend not configured")); - return HttpResponse::ServiceUnavailable() - .json(json!({ "error": "TTS backend not configured" })); + return tts_unavailable().into_response(); }; match client.list_voices().await { Ok(v) => { @@ -766,7 +764,7 @@ pub async fn list_voices_handler( Err(e) => { span.set_status(Status::error("list_voices failed")); log::error!("list_voices failed: {:?}", e); - HttpResponse::BadGateway().json(json!({ "error": format!("{e}") })) + tts_failed(&e).into_response() } } } @@ -783,8 +781,7 @@ pub async fn delete_voice_handler( let Some(client) = app_state.llamacpp.as_ref() else { span.set_status(Status::error("tts backend not configured")); - return HttpResponse::ServiceUnavailable() - .json(json!({ "error": "TTS backend not configured" })); + return tts_unavailable().into_response(); }; // Same charset rule as creation — a name that sanitizes differently was // never a voice we created, and must not reach the upstream URL. @@ -793,7 +790,7 @@ pub async fn delete_voice_handler( Some(n) if n == raw => n, _ => { span.set_status(Status::error("invalid voice name")); - return HttpResponse::BadRequest().json(json!({ "error": "invalid voice name" })); + return invalid_voice_name().into_response(); } }; span.set_attribute("tts.voice_name", name.clone()); @@ -807,7 +804,7 @@ pub async fn delete_voice_handler( Err(e) => { span.set_status(Status::error("delete_voice failed")); log::error!("delete_voice failed: {:?}", e); - HttpResponse::BadGateway().json(json!({ "error": format!("{e}") })) + tts_failed(&e).into_response() } } } @@ -827,8 +824,7 @@ pub async fn create_voice_upload_handler( let Some(client) = app_state.llamacpp.as_ref() else { span.set_status(Status::error("tts backend not configured")); - return HttpResponse::ServiceUnavailable() - .json(json!({ "error": "TTS backend not configured" })); + return tts_unavailable().into_response(); }; let mut voice_name: Option = None; @@ -853,8 +849,12 @@ pub async fn create_voice_upload_handler( while let Some(Ok(data)) = part.next().await { if file_bytes.len() + data.len() > MAX_VOICE_UPLOAD_BYTES { span.set_status(Status::error("voice clip exceeds limit")); - return HttpResponse::PayloadTooLarge() - .json(json!({ "error": "voice clip exceeds 25 MB" })); + return ApiError::new( + actix_web::http::StatusCode::PAYLOAD_TOO_LARGE, + "body_too_large", + "The voice clip is too big (25 MB at most)", + ) + .into_response(); } file_bytes.put(data); } @@ -897,21 +897,20 @@ pub async fn create_voice_upload_handler( Ok(w) => w, Err(msg) => { span.set_status(Status::error("invalid reference window")); - return HttpResponse::BadRequest().json(json!({ "error": msg })); + return ApiError::bad_request("invalid_clip_window", msg).into_response(); } }; let Some(name) = voice_name.as_deref().and_then(sanitize_voice_name) else { span.set_status(Status::error("voice_name is required")); - return HttpResponse::BadRequest() - .json(json!({ "error": "voice_name is required (alphanumerics, - and _ only)" })); + return invalid_voice_name().into_response(); }; // Tag the name with the ref-clip length (e.g. `grandma-30s`) so the // library shows which reference length produced each clone. let name = append_ref_window(&name, ref_start, ref_duration.round().max(1.0) as u32); if file_bytes.is_empty() { span.set_status(Status::error("voice_file is required")); - return HttpResponse::BadRequest().json(json!({ "error": "voice_file is required" })); + return ApiError::bad_request("no_file", "Choose an audio file").into_response(); } span.set_attribute("tts.voice_name", name.clone()); span.set_attribute("tts.upload_bytes", file_bytes.len() as i64); @@ -925,8 +924,7 @@ pub async fn create_voice_upload_handler( Err(e) => { span.set_status(Status::error("audio decode failed")); log::error!("voice upload transcode failed: {:?}", e); - return HttpResponse::BadRequest() - .json(json!({ "error": "couldn't decode that audio file" })); + return undecodable_audio().into_response(); } }; @@ -942,7 +940,7 @@ pub async fn create_voice_upload_handler( Err(e) => { span.set_status(Status::error("create_voice failed")); log::error!("create_voice (upload) failed: {:?}", e); - HttpResponse::BadGateway().json(json!({ "error": format!("{e}") })) + tts_failed(&e).into_response() } } } @@ -978,20 +976,18 @@ pub async fn create_voice_from_library_handler( let Some(client) = app_state.llamacpp.as_ref() else { span.set_status(Status::error("tts backend not configured")); - return HttpResponse::ServiceUnavailable() - .json(json!({ "error": "TTS backend not configured" })); + return tts_unavailable().into_response(); }; let Some(voice_name) = sanitize_voice_name(&req.voice_name) else { span.set_status(Status::error("voice_name is required")); - return HttpResponse::BadRequest() - .json(json!({ "error": "voice_name is required (alphanumerics, - and _ only)" })); + return invalid_voice_name().into_response(); }; let (ref_start, ref_duration) = match resolve_ref_window(req.start_seconds, req.duration_seconds) { Ok(w) => w, Err(msg) => { span.set_status(Status::error("invalid reference window")); - return HttpResponse::BadRequest().json(json!({ "error": msg })); + return ApiError::bad_request("invalid_clip_window", msg).into_response(); } }; // Tag the name with the ref-clip length (e.g. `grandma-30s`) so the @@ -1004,7 +1000,7 @@ pub async fn create_voice_from_library_handler( Ok(None) => app_state.primary_library(), Err(msg) => { span.set_status(Status::error("invalid library")); - return HttpResponse::BadRequest().json(json!({ "error": msg })); + return libraries::unknown_library(msg).into_response(); } }; @@ -1013,7 +1009,8 @@ pub async fn create_voice_from_library_handler( Some(p) if p.exists() => p, _ => { span.set_status(Status::error("file not found")); - return HttpResponse::NotFound().json(json!({ "error": "file not found in library" })); + return ApiError::not_found("file_not_found", "That file doesn't exist") + .into_response(); } }; @@ -1021,8 +1018,7 @@ pub async fn create_voice_from_library_handler( // slurp arbitrary library files into memory / ffmpeg. if !is_audio_file(&abs) && !is_video_file(&abs) { span.set_status(Status::error("not an audio/video file")); - return HttpResponse::BadRequest() - .json(json!({ "error": "file is not an audio or video file" })); + return ApiError::bad_request("not_audio", "That file has no audio").into_response(); } span.set_attribute("tts.voice_name", voice_name.clone()); @@ -1031,8 +1027,7 @@ pub async fn create_voice_from_library_handler( Err(e) => { span.set_status(Status::error("audio decode failed")); log::error!("voice reference prep failed for {:?}: {:?}", abs, e); - return HttpResponse::BadRequest() - .json(json!({ "error": "couldn't decode that file's audio" })); + return undecodable_audio().into_response(); } }; @@ -1048,7 +1043,7 @@ pub async fn create_voice_from_library_handler( Err(e) => { span.set_status(Status::error("create_voice failed")); log::error!("create_voice (from-library) failed: {:?}", e); - HttpResponse::BadGateway().json(json!({ "error": format!("{e}") })) + tts_failed(&e).into_response() } } } @@ -1060,6 +1055,29 @@ async fn prepare_reference_audio(abs: &Path, start: f64, duration: f64) -> anyho run_ffmpeg_to_wav(&abs.to_string_lossy(), start, duration).await } +// ---- error responses (#202) --------------------------------------------- + +fn tts_unavailable() -> ApiError { + ApiError::unavailable("tts_unavailable", "Read-aloud isn't set up on this server") +} + +/// The voice backend failed; its message is kept in `details` for debugging. +fn tts_failed(e: &impl std::fmt::Display) -> ApiError { + ApiError::bad_gateway("tts_failed", "The voice service failed. Try again.") + .with_details(json!({ "reason": e.to_string() })) +} + +fn invalid_voice_name() -> ApiError { + ApiError::bad_request( + "invalid_voice_name", + "Voice names can only use letters, numbers, - and _", + ) +} + +fn undecodable_audio() -> ApiError { + ApiError::bad_request("undecodable_audio", "Couldn't read the audio in that file") +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/auth.rs b/src/auth.rs index 40f367a..c1e6d87 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -1,3 +1,4 @@ +use crate::error::ApiError; use actix_web::Responder; use actix_web::{ HttpResponse, @@ -40,20 +41,24 @@ async fn register( ) -> impl Responder { // Validate password strength if let Err(msg) = validate_password(&user.password) { - return HttpResponse::BadRequest().body(msg); + return ApiError::bad_request("weak_password", msg).into_response(); } if !user.username.is_empty() && user.password == user.confirmation { let mut dao = user_dao.lock().expect("Unable to get UserDao"); if dao.user_exists(&user.username) { - HttpResponse::BadRequest().finish() + ApiError::conflict("username_taken", "That username is taken").into_response() } else if let Some(_user) = dao.create_user(&user.username, &user.password) { HttpResponse::Ok().finish() } else { - HttpResponse::InternalServerError().finish() + ApiError::internal("create_user failed").into_response() } } else { - HttpResponse::BadRequest().finish() + ApiError::bad_request( + "invalid_registration", + "Enter a username and the same password twice", + ) + .into_response() } } @@ -77,15 +82,14 @@ pub async fn login( ) { Ok(t) => t, Err(e) => { - error!("Failed to encode JWT: {}", e); - return HttpResponse::InternalServerError().finish(); + return ApiError::internal(format!("Failed to encode JWT: {}", e)).into_response(); } }; HttpResponse::Ok().json(Token { token: &token }) } else { error!("Failed login attempt for user: '{}'", creds.username); - HttpResponse::Unauthorized().finish() + ApiError::unauthorized("invalid_credentials", "Wrong username or password").into_response() } } diff --git a/src/clip_search.rs b/src/clip_search.rs index 5b9413b..df6f838 100644 --- a/src/clip_search.rs +++ b/src/clip_search.rs @@ -24,6 +24,7 @@ use crate::AppState; use crate::ai::clip_client::ClipError; use crate::database::ExifDao; +use crate::error::ApiError; use actix_web::{HttpResponse, Result as ActixResult, web}; use base64::Engine; use serde::{Deserialize, Serialize}; @@ -104,11 +105,6 @@ pub struct SearchResponse { pub results: Vec, } -#[derive(Debug, Serialize)] -struct SearchError { - error: String, -} - /// Decode a stored `clip_embedding` BLOB back into a `Vec`. Returns /// `None` on malformed bytes — those rows get skipped rather than /// failing the whole query. @@ -316,9 +312,7 @@ pub async fn search_photos( ) -> ActixResult { let q_text = query.q.trim().to_string(); if q_text.is_empty() { - return Ok(HttpResponse::BadRequest().json(SearchError { - error: "query parameter `q` is required".into(), - })); + return Ok(empty_query().into_response()); } let limit = query.limit.clamp(1, 200); @@ -327,7 +321,7 @@ pub async fn search_photos( let library_ids = match parse_library_scope(query.library_ids.as_deref(), query.library) { Ok(ids) => ids, - Err(msg) => return Ok(HttpResponse::BadRequest().json(SearchError { error: msg })), + Err(msg) => return Ok(crate::libraries::unknown_library(msg).into_response()), }; let scored = match score_photos( @@ -370,20 +364,32 @@ pub async fn search_photos( /// returned for each failure mode. Reused by the unified endpoint. pub fn score_error_response(e: ScoreError) -> HttpResponse { match e { - ScoreError::Disabled => HttpResponse::ServiceUnavailable().json(SearchError { - error: "CLIP search is disabled (no Apollo CLIP endpoint configured)".into(), - }), - ScoreError::Rejected(msg) => HttpResponse::BadRequest().json(SearchError { - error: format!("query rejected: {msg}"), - }), - ScoreError::Unavailable(msg) => HttpResponse::BadGateway().json(SearchError { - error: format!("CLIP service unavailable: {msg}"), - }), - ScoreError::MalformedEmbedding => HttpResponse::BadGateway().json(SearchError { - error: "CLIP service returned a malformed query embedding".into(), - }), - ScoreError::Internal(msg) => { - HttpResponse::InternalServerError().json(SearchError { error: msg }) + ScoreError::Disabled => ApiError::unavailable( + "search_unavailable", + "Photo search isn't set up on this server", + ), + ScoreError::Rejected(msg) => ApiError::bad_request( + "query_rejected", + "The search service couldn't use that query", + ) + .with_details(serde_json::json!({ "reason": msg })), + ScoreError::Unavailable(msg) => { + log::warn!("CLIP service unavailable: {msg}"); + ApiError::bad_gateway( + "search_unavailable", + "Photo search is down right now. Try again shortly.", + ) } + ScoreError::MalformedEmbedding => ApiError::bad_gateway( + "search_bad_response", + "The search service sent an answer this server couldn't read", + ), + ScoreError::Internal(msg) => ApiError::internal(msg), } + .into_response() +} + +/// `q` is missing or blank. +pub fn empty_query() -> ApiError { + ApiError::bad_request("empty_query", "Type something to search for") } diff --git a/src/data/mod.rs b/src/data/mod.rs index add389b..62646ef 100644 --- a/src/data/mod.rs +++ b/src/data/mod.rs @@ -1,6 +1,7 @@ use std::{fs, str::FromStr}; use crate::database::models::ImageExif; +use crate::error::ApiError; use anyhow::{Context, anyhow}; use chrono::{DateTime, Utc}; @@ -89,7 +90,7 @@ impl FromRequest for Claims { .map_or_else( |e| { error!("{}", e); - err(crate::error::ApiError::unauthorized( + err(ApiError::unauthorized( "unauthorized", "Your session has expired. Sign in again.", ) diff --git a/src/duplicates.rs b/src/duplicates.rs index 2a86620..176672a 100644 --- a/src/duplicates.rs +++ b/src/duplicates.rs @@ -16,6 +16,7 @@ //! 5-minute window picks them up). For a single-user personal tool //! that's the right trade-off. +use crate::error::ApiError; use std::collections::HashMap; use std::sync::Mutex; use std::time::{Duration, Instant}; @@ -246,9 +247,7 @@ async fn list_exact_handler( match dao.list_duplicates_exact(library_id, include_resolved) { Ok(rows) => rows, Err(e) => { - return actix_web::ResponseError::error_response( - &crate::error::ApiError::internal(e), - ); + return actix_web::ResponseError::error_response(&ApiError::internal(e)); } } }; @@ -289,9 +288,7 @@ async fn list_perceptual_handler( match dao.list_perceptual_candidates(library_id, include_resolved) { Ok(rows) => rows, Err(e) => { - return actix_web::ResponseError::error_response( - &crate::error::ApiError::internal(e), - ); + return actix_web::ResponseError::error_response(&ApiError::internal(e)); } } }; @@ -318,7 +315,8 @@ async fn resolve_handler( exif_dao: web::Data>>, ) -> impl Responder { if body.demoted.is_empty() { - return HttpResponse::BadRequest().body("demoted list is empty"); + return ApiError::bad_request("no_files", "Choose at least one duplicate to hide") + .into_response(); } let mut dao = exif_dao.lock().expect("exif dao lock"); @@ -328,16 +326,23 @@ async fn resolve_handler( let survivor = match dao.lookup_duplicate_row(body.survivor.library_id, &body.survivor.rel_path) { Ok(Some(row)) => row, - Ok(None) => return HttpResponse::NotFound().body("survivor not found"), + Ok(None) => { + return ApiError::not_found("file_not_found", "The photo to keep no longer exists") + .into_response(); + } Err(e) => { - return actix_web::ResponseError::error_response(&crate::error::ApiError::internal(e)); + return ApiError::internal(e).into_response(); } }; // Survivor must not itself be soft-marked — otherwise the modal is // pointing at a row we've already demoted, which would create a chain. if survivor.duplicate_of_hash.is_some() { - return HttpResponse::Conflict().body("survivor is itself soft-marked as a duplicate"); + return ApiError::conflict( + "survivor_is_duplicate", + "The photo to keep is already hidden as a duplicate", + ) + .into_response(); } let now = chrono::Utc::now().timestamp(); @@ -355,9 +360,7 @@ async fn resolve_handler( continue; } Err(e) => { - return actix_web::ResponseError::error_response( - &crate::error::ApiError::internal(e), - ); + return actix_web::ResponseError::error_response(&ApiError::internal(e)); } }; @@ -395,7 +398,7 @@ async fn resolve_handler( &survivor.content_hash, now, ) { - return actix_web::ResponseError::error_response(&crate::error::ApiError::internal(e)); + return ApiError::internal(e).into_response(); } resolved_count += 1; @@ -416,7 +419,7 @@ async fn unresolve_handler( ) -> impl Responder { let mut dao = exif_dao.lock().expect("exif dao lock"); if let Err(e) = dao.clear_duplicate_of(body.library_id, &body.rel_path) { - return actix_web::ResponseError::error_response(&crate::error::ApiError::internal(e)); + return ApiError::internal(e).into_response(); } drop(dao); @@ -459,17 +462,13 @@ async fn list_folder_pairs_handler( let dup_rows = match dao.list_duplicates_exact(library_id, include_resolved) { Ok(rows) => rows, Err(e) => { - return actix_web::ResponseError::error_response( - &crate::error::ApiError::internal(e), - ); + return actix_web::ResponseError::error_response(&ApiError::internal(e)); } }; let all_paths = match dao.list_image_paths(library_id, include_resolved) { Ok(rows) => rows, Err(e) => { - return actix_web::ResponseError::error_response( - &crate::error::ApiError::internal(e), - ); + return actix_web::ResponseError::error_response(&ApiError::internal(e)); } }; (dup_rows, all_paths) diff --git a/src/error.rs b/src/error.rs index 3a88997..9aee0b6 100644 --- a/src/error.rs +++ b/src/error.rs @@ -74,6 +74,29 @@ impl ApiError { Self::new(StatusCode::CONFLICT, code, message) } + pub fn forbidden(code: &'static str, message: impl Into) -> Self { + Self::new(StatusCode::FORBIDDEN, code, message) + } + + pub fn gone(code: &'static str, message: impl Into) -> Self { + Self::new(StatusCode::GONE, code, message) + } + + pub fn too_many_requests(code: &'static str, message: impl Into) -> Self { + Self::new(StatusCode::TOO_MANY_REQUESTS, code, message) + } + + /// 502: a service this server depends on (Apollo, ComfyUI, an LLM, SMS-API) + /// failed or answered badly. + pub fn bad_gateway(code: &'static str, message: impl Into) -> Self { + Self::new(StatusCode::BAD_GATEWAY, code, message) + } + + /// 503: a feature that isn't configured, or a dependency that's down. + pub fn unavailable(code: &'static str, message: impl Into) -> Self { + Self::new(StatusCode::SERVICE_UNAVAILABLE, 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); @@ -105,6 +128,12 @@ impl ApiError { self } + /// The response, for handlers that return `HttpResponse` rather than + /// `ApiResult`. + pub fn into_response(self) -> HttpResponse { + self.error_response() + } + pub fn code(&self) -> &'static str { self.code } @@ -215,6 +244,25 @@ pub fn path_config() -> web::PathConfig { }) } +/// Multipart upload failures (malformed form, over the size limit). +pub fn multipart_config(total_limit: usize) -> actix_multipart::form::MultipartFormConfig { + actix_multipart::form::MultipartFormConfig::default() + .total_limit(total_limit) + .error_handler(|err, req: &HttpRequest| { + log::warn!("Upload error on {} {}: {}", req.method(), req.uri(), err); + let api = if err.to_string().contains("limit") { + ApiError::new( + StatusCode::PAYLOAD_TOO_LARGE, + "body_too_large", + "The upload is too large", + ) + } else { + ApiError::bad_request("invalid_upload", format!("Invalid upload: {}", 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() @@ -268,6 +316,73 @@ mod tests { assert!(!body.to_string().contains("secret"), "{body}"); } + /// Every error response goes through `ApiError` (#202). A handler that + /// builds one by hand (`HttpResponse::NotFound()...`) skips the format; + /// this fails with the file and line so it can be converted. + #[test] + fn no_handler_builds_an_error_response_by_hand() { + const BUILDERS: &[&str] = &[ + "BadRequest", + "Unauthorized", + "Forbidden", + "NotFound", + "MethodNotAllowed", + "Conflict", + "Gone", + "PayloadTooLarge", + "UnprocessableEntity", + "TooManyRequests", + "RangeNotSatisfiable", + "InternalServerError", + "NotImplemented", + "BadGateway", + "ServiceUnavailable", + "GatewayTimeout", + ]; + fn walk(dir: &std::path::Path, out: &mut Vec) { + for entry in std::fs::read_dir(dir).unwrap() { + let path = entry.unwrap().path(); + if path.is_dir() { + walk(&path, out); + } else if path.extension().is_some_and(|e| e == "rs") { + out.push(path); + } + } + } + let src = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src"); + let mut files = Vec::new(); + walk(&src, &mut files); + + let mut offenders = Vec::new(); + for file in files { + let name = file.to_string_lossy(); + if name.ends_with("tests.rs") || name.ends_with("/error.rs") { + continue; + } + let text = std::fs::read_to_string(&file).unwrap(); + // Test modules may assert on hand-built responses. + let code = match text.find("#[cfg(test)]\nmod ") { + Some(i) => &text[..i], + None => &text[..], + }; + for (n, line) in code.lines().enumerate() { + if line.trim_start().starts_with("//") { + continue; + } + for b in BUILDERS { + if line.contains(&format!("HttpResponse::{b}()")) { + offenders.push(format!("{}:{}: {}", name, n + 1, line.trim())); + } + } + } + } + assert!( + offenders.is_empty(), + "build these with crate::error::ApiError instead:\n{}", + offenders.join("\n") + ); + } + #[derive(Deserialize)] struct Q { #[allow(dead_code)] diff --git a/src/faces.rs b/src/faces.rs index 0f912f3..5ac856c 100644 --- a/src/faces.rs +++ b/src/faces.rs @@ -21,6 +21,7 @@ use crate::Claims; use crate::ai::face_client::{DetectMeta, FaceClient, FaceDetectError}; use crate::database::schema::{face_detections, image_exif, persons}; +use crate::error::ApiError; use crate::error::IntoHttpError; use crate::exif; use crate::file_types; @@ -1685,11 +1686,11 @@ async fn list_faces_handler( // the watcher catches up. return HttpResponse::Ok().json(Vec::::new()); } - Err(e) => return HttpResponse::InternalServerError().body(e.to_string()), + Err(e) => return ApiError::internal(e.to_string()).into_response(), }; match dao.list_for_content_hash(&hash) { Ok(faces) => HttpResponse::Ok().json(faces), - Err(e) => HttpResponse::InternalServerError().body(e.to_string()), + Err(e) => ApiError::internal(e.to_string()).into_response(), } } @@ -1745,7 +1746,7 @@ async fn create_face_handler( // strict path does. Surface the disabled state only when we'd // actually use the client. if !body.force && !face_client.is_enabled() { - return HttpResponse::ServiceUnavailable().body("face client disabled"); + return faces_unavailable().into_response(); } let normalized_path = normalize_path(&body.path); @@ -1763,10 +1764,13 @@ async fn create_face_handler( match dao.resolve_content_hash(library.id, &normalized_path) { Ok(Some(h)) => h, Ok(None) => { - return HttpResponse::Conflict() - .body("photo not yet hashed; wait for next watcher pass"); + return ApiError::conflict( + "photo_not_indexed", + "This photo is still being indexed. Try again in a minute.", + ) + .into_response(); } - Err(e) => return HttpResponse::InternalServerError().body(e.to_string()), + Err(e) => return ApiError::internal(e.to_string()).into_response(), } }; @@ -1798,7 +1802,7 @@ async fn create_face_handler( Ok(b) => b, Err(e) => { warn!("crop_image_to_bbox failed for {:?}: {:?}", abs_path, e); - return HttpResponse::BadRequest().body(format!("cannot crop photo: {}", e)); + return bad_crop(e).into_response(); } }; @@ -1812,13 +1816,14 @@ async fn create_face_handler( let detect = match face_client.embed(crop_bytes, meta).await { Ok(r) => r, Err(FaceDetectError::Permanent(e)) => { - return HttpResponse::UnprocessableEntity().body(format!("{}", e)); + return no_face_found(e).into_response(); } Err(FaceDetectError::Transient(e)) => { - return HttpResponse::ServiceUnavailable().body(format!("{}", e)); + warn!("manual face: face service transient error: {}", e); + return faces_busy().into_response(); } Err(FaceDetectError::Disabled) => { - return HttpResponse::ServiceUnavailable().body("face client disabled"); + return faces_unavailable().into_response(); } }; @@ -1826,14 +1831,18 @@ async fn create_face_handler( Some(f) => f.clone(), None => { // Apollo would have returned 422 on no_face_in_crop; defensive. - return HttpResponse::UnprocessableEntity().body("no face in crop"); + return no_face_found("no face in crop").into_response(); } }; let bytes = match detected.decode_embedding() { Ok(b) => b, Err(e) => { warn!("manual face: decode embedding failed: {:?}", e); - return HttpResponse::BadGateway().body("invalid embedding from face service"); + return ApiError::bad_gateway( + "faces_bad_response", + "The face service sent an answer this server couldn't read", + ) + .into_response(); } }; (bytes, detect.model_version, detected.confidence) @@ -1857,7 +1866,7 @@ async fn create_face_handler( model_version, }) { Ok(r) => r, - Err(e) => return HttpResponse::InternalServerError().body(e.to_string()), + Err(e) => return ApiError::internal(e.to_string()).into_response(), }; info!( "Created manual face id={} library={} hash={} person_id={:?}", @@ -1865,7 +1874,7 @@ async fn create_face_handler( ); match hydrate_face_with_person(&mut *dao, row) { Ok(joined) => HttpResponse::Created().json(joined), - Err(e) => HttpResponse::InternalServerError().body(e.to_string()), + Err(e) => ApiError::internal(e.to_string()).into_response(), } } @@ -1902,18 +1911,17 @@ async fn update_face_handler( Bbox edit requires Apollo to re-embed.", id ); - return HttpResponse::ServiceUnavailable() - .body("face client disabled — bbox edit requires Apollo"); + return faces_unavailable().into_response(); } // Look up the current row so we know which photo to crop. let current = { let mut dao = face_dao.lock().expect("face dao lock"); match dao.get_face(id) { Ok(Some(r)) => r, - Ok(None) => return HttpResponse::NotFound().finish(), + Ok(None) => return face_not_found().into_response(), Err(e) => { warn!("PATCH /image/faces/{}: 500 — get_face failed: {:#}", id, e); - return HttpResponse::InternalServerError().body(e.to_string()); + return ApiError::internal(e.to_string()).into_response(); } } }; @@ -1924,10 +1932,11 @@ async fn update_face_handler( "PATCH /image/faces/{}: 500 — face row references unknown library_id {}", id, current.library_id ); - return HttpResponse::InternalServerError().body(format!( + return ApiError::internal(format!( "face row references unknown library_id {}", current.library_id - )); + )) + .into_response(); } }; let abs_path = library.resolve(¤t.rel_path); @@ -1938,7 +1947,7 @@ async fn update_face_handler( "PATCH /image/faces/{}: crop failed for {:?}: {:?}", id, abs_path, e ); - return HttpResponse::BadRequest().body(format!("cannot crop new bbox: {}", e)); + return bad_crop(e).into_response(); } }; let meta = DetectMeta { @@ -1990,7 +1999,7 @@ async fn update_face_handler( error during re-embed: {}", id, e ); - return HttpResponse::ServiceUnavailable().body(format!("{}", e)); + return faces_busy().into_response(); } Err(FaceDetectError::Disabled) => { warn!( @@ -1998,7 +2007,7 @@ async fn update_face_handler( mid-flight", id ); - return HttpResponse::ServiceUnavailable().body("face client disabled mid-flight"); + return faces_unavailable().into_response(); } } } @@ -2015,7 +2024,7 @@ async fn update_face_handler( "PATCH /image/faces/{}: 500 — update_face failed: {:#}", id, e ); - return HttpResponse::InternalServerError().body(e.to_string()); + return ApiError::internal(e.to_string()).into_response(); } }; // Hydrate person_name so the response shape matches GET /image/faces @@ -2029,7 +2038,7 @@ async fn update_face_handler( "PATCH /image/faces/{}: 500 — hydrate_face_with_person failed: {:#}", id, e ); - HttpResponse::InternalServerError().body(e.to_string()) + ApiError::internal(e.to_string()).into_response() } } } @@ -2043,8 +2052,8 @@ async fn delete_face_handler( let mut dao = face_dao.lock().expect("face dao lock"); match dao.delete_face(path.into_inner()) { Ok(true) => HttpResponse::NoContent().finish(), - Ok(false) => HttpResponse::NotFound().finish(), - Err(e) => HttpResponse::InternalServerError().body(e.to_string()), + Ok(false) => face_not_found().into_response(), + Err(e) => ApiError::internal(e.to_string()).into_response(), } } @@ -2078,6 +2087,53 @@ async fn ignore_bucket_handler( .into_http_internal_err() } +// ---- error responses (#202) --------------------------------------------- + +/// Apollo's face service isn't configured. +fn faces_unavailable() -> ApiError { + ApiError::unavailable( + "faces_unavailable", + "Face recognition isn't set up on this server", + ) +} + +/// Apollo's face service failed in a way a retry may fix. +fn faces_busy() -> ApiError { + ApiError::unavailable( + "faces_busy", + "Face recognition is busy or down. Try again shortly.", + ) +} + +fn no_face_found(reason: impl std::fmt::Display) -> ApiError { + ApiError::new( + actix_web::http::StatusCode::UNPROCESSABLE_ENTITY, + "no_face_found", + "No face was found in that box", + ) + .with_details(serde_json::json!({ "reason": reason.to_string() })) +} + +/// The crop error can name the file's absolute path; the caller logs it. +fn bad_crop(_e: impl std::fmt::Display) -> ApiError { + ApiError::bad_request("invalid_face_box", "Couldn't crop the photo to that box") +} + +fn face_not_found() -> ApiError { + ApiError::not_found("face_not_found", "That face no longer exists") +} + +fn person_not_found() -> ApiError { + ApiError::not_found("person_not_found", "That person no longer exists") +} + +fn person_name_taken() -> ApiError { + ApiError::conflict( + "person_name_taken", + "There's already a person with that name", + ) +} + #[tracing::instrument(name = "persons.create", skip_all)] async fn create_person_handler( _: Claims, @@ -2085,7 +2141,7 @@ async fn create_person_handler( face_dao: web::Data>, ) -> impl Responder { if body.name.trim().is_empty() { - return HttpResponse::BadRequest().body("name required"); + return ApiError::bad_request("invalid_person_name", "Enter a name").into_response(); } let mut dao = face_dao.lock().expect("face dao lock"); @@ -2098,9 +2154,9 @@ async fn create_person_handler( // the outermost context ("insert person ...") which hides the // diesel "UNIQUE constraint failed" we're keying on. if is_unique_violation(&e) { - HttpResponse::Conflict().body("person name already exists") + person_name_taken().into_response() } else { - HttpResponse::InternalServerError().body(format!("{:#}", e)) + ApiError::internal(format!("{:#}", e)).into_response() } } } @@ -2115,8 +2171,8 @@ async fn get_person_handler( let mut dao = face_dao.lock().expect("face dao lock"); match dao.get_person(path.into_inner()) { Ok(Some(p)) => HttpResponse::Ok().json(p), - Ok(None) => HttpResponse::NotFound().finish(), - Err(e) => HttpResponse::InternalServerError().body(e.to_string()), + Ok(None) => person_not_found().into_response(), + Err(e) => ApiError::internal(e.to_string()).into_response(), } } @@ -2132,9 +2188,9 @@ async fn update_person_handler( Ok(p) => HttpResponse::Ok().json(p), Err(e) => { if is_unique_violation(&e) { - HttpResponse::Conflict().body("person name already exists") + person_name_taken().into_response() } else { - HttpResponse::InternalServerError().body(format!("{:#}", e)) + ApiError::internal(format!("{:#}", e)).into_response() } } } @@ -2153,8 +2209,8 @@ async fn delete_person_handler( let mut dao = face_dao.lock().expect("face dao lock"); match dao.delete_person(path.into_inner(), cascade) { Ok(true) => HttpResponse::NoContent().finish(), - Ok(false) => HttpResponse::NotFound().finish(), - Err(e) => HttpResponse::InternalServerError().body(e.to_string()), + Ok(false) => person_not_found().into_response(), + Err(e) => ApiError::internal(e.to_string()).into_response(), } } @@ -2172,9 +2228,13 @@ async fn merge_persons_handler( Err(e) => { let msg = format!("{:#}", e); if msg.contains("itself") { - HttpResponse::BadRequest().body(msg) + ApiError::bad_request( + "merge_into_self", + "A person can't be merged into themselves", + ) + .into_response() } else { - HttpResponse::InternalServerError().body(msg) + ApiError::internal(msg).into_response() } } } diff --git a/src/files.rs b/src/files.rs index 586fa7b..db93d66 100644 --- a/src/files.rs +++ b/src/files.rs @@ -1,3 +1,4 @@ +use crate::error::ApiError; use ::anyhow; use actix::{Handler, Message}; use anyhow::Context; @@ -190,6 +191,11 @@ fn in_memory_date_sort( (result, result_libs, total_count) } +/// The requested folder is outside every library (or traverses out of one). +fn folder_not_found() -> ApiError { + ApiError::bad_request("folder_not_found", "That folder isn't in the library") +} + #[tracing::instrument(name = "list_photos", skip_all)] pub async fn list_photos( _: Claims, @@ -288,7 +294,7 @@ pub async fn list_photos( Ok(lib) => lib, Err(msg) => { log::warn!("Rejecting /photos request: {}", msg); - return HttpResponse::BadRequest().body(msg); + return crate::libraries::unknown_library(msg).into_response(); } }; @@ -308,7 +314,11 @@ pub async fn list_photos( { warn!("GPS search requires lat, lon, and radius_km to all be specified"); span.set_status(Status::error("Invalid GPS parameters")); - return HttpResponse::BadRequest().body("GPS search requires lat, lon, and radius_km"); + return ApiError::bad_request( + "incomplete_gps_filter", + "A location filter needs a latitude, longitude and radius", + ) + .into_response(); } // Calculate GPS bounding box if GPS search is requested @@ -722,7 +732,7 @@ pub async fn list_photos( if !any_library_resolved { error!("Bad photos request: {}", req.path); span.set_status(Status::error("Invalid path")); - return HttpResponse::BadRequest().finish(); + return folder_not_found().into_response(); } // A folder whose indexed contents are all hidden versions (typically @@ -1205,9 +1215,7 @@ pub async fn get_gps_summary( if !validated { warn!("Invalid path for GPS summary: {}", req.path); span.set_status(Status::error("Invalid path")); - return Ok(HttpResponse::BadRequest().json(serde_json::json!({ - "error": "Invalid path" - }))); + return Ok(folder_not_found().into_response()); } req.path.clone() }; @@ -1281,9 +1289,7 @@ pub async fn get_gps_summary( Err(e) => { error!("Error querying GPS data: {:?}", e); span.set_status(Status::error(format!("Database error: {:?}", e))); - Ok(HttpResponse::InternalServerError().json(serde_json::json!({ - "error": "Failed to query GPS data" - }))) + Ok(ApiError::internal("Failed to query GPS data").into_response()) } } } @@ -1336,7 +1342,7 @@ pub async fn list_exif_summary( Ok(lib) => lib.map(|l| l.id), Err(msg) => { span.set_status(Status::error(msg.clone())); - return Ok(HttpResponse::BadRequest().json(serde_json::json!({ "error": msg }))); + return Ok(crate::libraries::unknown_library(msg).into_response()); } }; @@ -1386,9 +1392,7 @@ pub async fn list_exif_summary( Err(e) => { error!("Error querying EXIF batch: {:?}", e); span.set_status(Status::error(format!("Database error: {:?}", e))); - Ok(HttpResponse::InternalServerError().json(serde_json::json!({ - "error": "Failed to query EXIF data" - }))) + Ok(ApiError::internal("Failed to query EXIF data").into_response()) } } } @@ -1432,9 +1436,9 @@ pub async fn move_file( ); if e == ErrorKind::InvalidData { - HttpResponse::BadRequest().finish() + ApiError::bad_request("invalid_move", "Can't move the file there").into_response() } else { - HttpResponse::InternalServerError().finish() + ApiError::internal("internal error").into_response() } } } diff --git a/src/handlers/favorites.rs b/src/handlers/favorites.rs index 48a73bd..afd5a48 100644 --- a/src/handlers/favorites.rs +++ b/src/handlers/favorites.rs @@ -2,6 +2,7 @@ //! and shared across libraries — a favorite created in lib1 is visible //! under lib2 if the same rel_path resolves there too. +use crate::error::ApiError; use std::sync::Mutex; use tracing_opentelemetry::OpenTelemetrySpanExt; @@ -53,10 +54,9 @@ pub async fn favorites( } Ok(Err(e)) => { span.set_status(Status::error(format!("Error getting favorites: {:?}", e))); - error!("Error getting favorites: {:?}", e); - HttpResponse::InternalServerError().finish() + ApiError::internal(format!("Error getting favorites: {:?}", e)).into_response() } - Err(_) => HttpResponse::InternalServerError().finish(), + Err(e) => ApiError::internal(e).into_response(), } } @@ -78,24 +78,24 @@ pub async fn put_add_favorite( { Ok(Err(e)) if e.kind == DbErrorKind::AlreadyExists => { warn!("Favorite: {} exists for user: {}", &body.path, user_id); - HttpResponse::Ok() - } - Ok(Err(e)) => { - error!("{:?} {}. for user: {}", e, body.path, user_id); - HttpResponse::BadRequest() + HttpResponse::Ok().finish() } + Ok(Err(e)) => ApiError::internal(format!( + "{:?} adding favorite {}. for user: {}", + e, body.path, user_id + )) + .into_response(), Ok(Ok(_)) => { info!("Adding favorite \"{}\" for userid: {}", body.path, user_id); - HttpResponse::Created() + HttpResponse::Created().finish() } Err(e) => { - error!("Blocking error while inserting favorite: {:?}", e); - HttpResponse::InternalServerError() + ApiError::internal(format!("Blocking error while inserting favorite: {:?}", e)) + .into_response() } } } else { - error!("Unable to parse sub as i32: {}", claims.sub); - HttpResponse::BadRequest() + bad_user(&claims).into_response() } } @@ -120,9 +120,14 @@ pub async fn delete_favorite( "Removing favorite \"{}\" for userid: {}", body.path, user_id ); - HttpResponse::Ok() + HttpResponse::Ok().finish() } else { - error!("Unable to parse sub as i32: {}", claims.sub); - HttpResponse::BadRequest() + bad_user(&claims).into_response() } } + +/// A token whose subject isn't a user id (favorites are per user). +fn bad_user(claims: &Claims) -> ApiError { + error!("Unable to parse sub as i32: {}", claims.sub); + ApiError::unauthorized("unauthorized", "Your session has expired. Sign in again.") +} diff --git a/src/handlers/image.rs b/src/handlers/image.rs index dfc89ec..ad55f35 100644 --- a/src/handlers/image.rs +++ b/src/handlers/image.rs @@ -2,6 +2,7 @@ //! legacy thumbnail lookup), upload, EXIF metadata read + GPS / date //! mutation, and the full exiftool dump used by Apollo's details modal. +use crate::error::ApiError; use std::error::Error; use std::fs::File; use std::io::ErrorKind; @@ -56,7 +57,7 @@ pub async fn get_image( Ok(None) => app_state.primary_library(), Err(msg) => { span.set_status(Status::error(msg.clone())); - return HttpResponse::BadRequest().body(msg); + return libraries::unknown_library(msg).into_response(); } }; @@ -231,7 +232,11 @@ pub async fn get_image( return resp; } span.set_status(Status::error("Thumbnail unavailable")); - return HttpResponse::NotFound().finish(); + return ApiError::not_found( + "thumbnail_unavailable", + "No thumbnail for this file yet", + ) + .into_response(); } } @@ -264,11 +269,11 @@ pub async fn get_image( } span.set_status(Status::error("Not found")); - HttpResponse::NotFound().finish() + file_not_found().into_response() } else { span.set_status(Status::error("Not found")); error!("Path does not exist in any library: {}", req.path); - HttpResponse::NotFound().finish() + file_not_found().into_response() } } @@ -557,7 +562,7 @@ pub async fn get_file_metadata( error!("{}", message); span.set_status(Status::error(message)); - HttpResponse::InternalServerError().finish() + ApiError::internal("internal error").into_response() } } } @@ -609,19 +614,23 @@ pub async fn set_image_gps( Some(v) => v, None => { span.set_status(Status::error("file not found")); - return HttpResponse::NotFound().body("File not found"); + return file_not_found().into_response(); } }; if !exif::supports_exif(&full_path) { - return HttpResponse::BadRequest().body("File format does not support EXIF GPS write"); + return ApiError::bad_request( + "gps_write_unsupported", + "Location can't be saved into this file type", + ) + .into_response(); } if let Err(e) = exif::write_gps(&full_path, body.latitude, body.longitude) { let msg = format!("exiftool write failed: {}", e); error!("{}", msg); span.set_status(Status::error(msg.clone())); - return HttpResponse::InternalServerError().body(msg); + return ApiError::internal(msg).into_response(); } // Re-read EXIF from disk (the write path doesn't tell us the rest of @@ -637,7 +646,7 @@ pub async fn set_image_gps( // next file scan rewrites it. let msg = format!("EXIF re-read failed after write: {}", e); error!("{}", msg); - return HttpResponse::InternalServerError().body(msg); + return ApiError::internal(msg).into_response(); } }; let now = Utc::now().timestamp(); @@ -742,7 +751,7 @@ pub async fn set_image_gps( let msg = format!("EXIF DB update failed: {:?}", e); error!("{}", msg); span.set_status(Status::error(msg.clone())); - HttpResponse::InternalServerError().body(msg) + ApiError::internal(msg).into_response() } } } @@ -790,7 +799,7 @@ pub async fn get_full_exif( Some(v) => v, None => { span.set_status(Status::error("file not found")); - return HttpResponse::NotFound().body("File not found"); + return file_not_found().into_response(); } }; @@ -825,13 +834,17 @@ pub async fn get_full_exif( span.set_status(Status::error(msg.clone())); // 503 — typically "exiftool isn't on PATH" or a transient spawn // failure. Apollo surfaces a hint in the modal. - HttpResponse::ServiceUnavailable().body(msg) + ApiError::unavailable( + "exiftool_unavailable", + "Couldn't write to the file: exiftool isn't available on the server", + ) + .into_response() } Err(e) => { let msg = format!("blocking-pool error: {}", e); error!("{}", msg); span.set_status(Status::error(msg.clone())); - HttpResponse::InternalServerError().body(msg) + ApiError::internal(msg).into_response() } } } @@ -965,7 +978,7 @@ pub async fn set_image_date( Ok(None) => app_state.primary_library(), Err(msg) => { span.set_status(Status::error(msg.clone())); - return HttpResponse::BadRequest().body(msg); + return libraries::unknown_library(msg).into_response(); } }; @@ -1008,8 +1021,8 @@ pub async fn set_image_date( error!("{}", msg); span.set_status(Status::error(msg.clone())); match e.kind { - DbErrorKind::NotFound => HttpResponse::NotFound().body(msg), - _ => HttpResponse::InternalServerError().body(msg), + DbErrorKind::NotFound => not_indexed().into_response(), + _ => ApiError::internal(msg).into_response(), } } } @@ -1031,7 +1044,7 @@ pub async fn clear_image_date( Ok(None) => app_state.primary_library(), Err(msg) => { span.set_status(Status::error(msg.clone())); - return HttpResponse::BadRequest().body(msg); + return libraries::unknown_library(msg).into_response(); } }; @@ -1058,8 +1071,8 @@ pub async fn clear_image_date( error!("{}", msg); span.set_status(Status::error(msg.clone())); match e.kind { - DbErrorKind::NotFound => HttpResponse::NotFound().body(msg), - _ => HttpResponse::InternalServerError().body(msg), + DbErrorKind::NotFound => not_indexed().into_response(), + _ => ApiError::internal(msg).into_response(), } } } @@ -1089,7 +1102,7 @@ pub async fn upload_image( Ok(None) => app_state.primary_library(), Err(msg) => { span.set_status(Status::error(msg.clone())); - return HttpResponse::BadRequest().body(msg); + return libraries::unknown_library(msg).into_response(); } }; @@ -1126,7 +1139,8 @@ pub async fn upload_image( if !file_content.is_empty() { if file_name.is_none() { span.set_status(Status::error("No filename provided")); - return HttpResponse::BadRequest().body("No filename provided"); + return ApiError::bad_request("no_file_name", "The upload has no file name") + .into_response(); } let full_path = PathBuf::from(&path).join(file_name.unwrap()); if let Some(full_path) = is_valid_full_path( @@ -1154,14 +1168,22 @@ pub async fn upload_image( .find(|l| l.id == existing.library_id) .map(|l| l.name); span.set_status(Status::Ok); - return HttpResponse::Conflict().json(serde_json::json!({ - "duplicate_of": { + // The extra fields predate the error format; clients + // read them at the top level. + return ApiError::conflict( + "duplicate_upload", + "This photo is already in your library", + ) + .with_legacy_field( + "duplicate_of", + serde_json::json!({ "library_id": existing.library_id, "rel_path": existing.file_path, - }, - "content_hash": upload_hash, - "library_name": library_name, - })); + }), + ) + .with_legacy_field("content_hash", &upload_hash) + .with_legacy_field("library_name", &library_name) + .into_response(); } } @@ -1274,11 +1296,13 @@ pub async fn upload_image( } else { error!("Invalid path for upload: {:?}", full_path); span.set_status(Status::error("Invalid path for upload")); - return HttpResponse::BadRequest().body("Path was not valid"); + return ApiError::bad_request("invalid_path", "That folder isn't in the library") + .into_response(); } } else { span.set_status(Status::error("No file body read")); - return HttpResponse::BadRequest().body("No file body read"); + return ApiError::bad_request("empty_upload", "The upload has no file in it") + .into_response(); } app_state.stream_manager.do_send(RefreshThumbnailsMessage); @@ -1318,6 +1342,19 @@ fn sniff_image_mime(head: &[u8]) -> Option { } } +/// The path names no file in any library. +fn file_not_found() -> ApiError { + ApiError::not_found("file_not_found", "That file doesn't exist") +} + +/// The file exists but has no indexed row yet. +fn not_indexed() -> ApiError { + ApiError::not_found( + "photo_not_indexed", + "This photo is still being indexed. Try again in a minute.", + ) +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/handlers/video.rs b/src/handlers/video.rs index 2be2227..806ea51 100644 --- a/src/handlers/video.rs +++ b/src/handlers/video.rs @@ -1,6 +1,7 @@ //! Video-related endpoints: HLS playlist generation, segment streaming, //! and the short-clip preview pipeline. +use crate::error::ApiError; use std::collections::HashMap; use std::path::PathBuf; use std::sync::Mutex; @@ -75,7 +76,7 @@ pub async fn generate_video( let resolved = resolve_request_path(&app_state, &body.path, body.library.as_deref()); let Some((resolved_library_id, resolved_root, full_path)) = resolved else { span.set_status(Status::error(format!("invalid path {:?}", &body.path))); - return HttpResponse::BadRequest().finish(); + return invalid_path().into_response(); }; // Build the rel_path used to look up the row. Forward-slash normalized @@ -112,7 +113,7 @@ pub async fn generate_video( e ); span.set_status(Status::error(format!("hash compute failed: {}", e))); - return HttpResponse::InternalServerError().finish(); + return ApiError::internal("internal error").into_response(); } }, }; @@ -198,11 +199,13 @@ pub async fn stream_hls_file( let (hash, file) = path.into_inner(); if !is_valid_hash(&hash) { span.set_status(Status::error("invalid hash")); - return HttpResponse::BadRequest().body("invalid hash"); + return ApiError::bad_request("invalid_path", "Invalid video stream address") + .into_response(); } if !is_allowed_hls_filename(&file) { span.set_status(Status::error("invalid file")); - return HttpResponse::BadRequest().body("invalid file"); + return ApiError::bad_request("invalid_path", "Invalid video stream address") + .into_response(); } let shard = &hash[..2]; @@ -220,7 +223,7 @@ pub async fn stream_hls_file( Err(e) => { error!("Failed to canonicalize VIDEO_PATH: {:?}", e); span.set_status(Status::error("VIDEO_PATH not canonicalisable")); - return HttpResponse::InternalServerError().finish(); + return ApiError::internal("internal error").into_response(); } }; let canonical_file = match std::fs::canonicalize(&file_path) { @@ -228,7 +231,7 @@ pub async fn stream_hls_file( Err(_) => { debug!("HLS file not found: {}", file_path.display()); span.set_status(Status::error("not found")); - return HttpResponse::NotFound().finish(); + return stream_not_found().into_response(); } }; if !canonical_file.starts_with(&canonical_base) { @@ -237,7 +240,7 @@ pub async fn stream_hls_file( file_path.display() ); span.set_status(Status::error("traversal")); - return HttpResponse::Forbidden().finish(); + return ApiError::forbidden("invalid_path", "Invalid video stream address").into_response(); } match NamedFile::open(&canonical_file) { @@ -247,7 +250,7 @@ pub async fn stream_hls_file( } Err(_) => { span.set_status(Status::error("not found")); - HttpResponse::NotFound().finish() + stream_not_found().into_response() } } } @@ -294,11 +297,11 @@ pub async fn original_video_url( resolve_request_path(&app_state, &body.path, body.library.as_deref()) else { span.set_status(Status::error(format!("invalid path {:?}", &body.path))); - return HttpResponse::BadRequest().finish(); + return invalid_path().into_response(); }; if !file_types::is_video_file(&full_path) { span.set_status(Status::error("not a video")); - return HttpResponse::BadRequest().body("not a video"); + return ApiError::bad_request("not_a_video", "That file isn't a video").into_response(); } let rel_path = rel_path_for_lookup(&full_path.to_string_lossy(), &root); @@ -372,7 +375,11 @@ pub async fn stream_original_video( .and_then(|t| original_url::verify(&t.0)); let Some(claims) = claims else { span.set_status(Status::error("bad token")); - return HttpResponse::Unauthorized().finish(); + return ApiError::unauthorized( + "stream_link_expired", + "This video link has expired. Open the video again.", + ) + .into_response(); }; let Some(path) = app_state .libraries @@ -382,7 +389,7 @@ pub async fn stream_original_video( .filter(|p| p.is_file()) else { span.set_status(Status::error("not found")); - return HttpResponse::NotFound().finish(); + return file_not_found().into_response(); }; match NamedFile::open(&path) { @@ -394,7 +401,7 @@ pub async fn stream_original_video( } Err(_) => { span.set_status(Status::error("not found")); - HttpResponse::NotFound().finish() + stream_not_found().into_response() } } } @@ -472,7 +479,7 @@ pub async fn get_video_preview( Some(path) => path, None => { span.set_status(Status::error("Invalid path")); - return HttpResponse::BadRequest().json(serde_json::json!({"error": "Invalid path"})); + return invalid_path().into_response(); } }; @@ -532,9 +539,7 @@ pub async fn get_video_preview( .error_message .unwrap_or_else(|| "Unknown error".to_string()); span.set_status(Status::error(format!("Generation failed: {}", error_msg))); - HttpResponse::InternalServerError().json(serde_json::json!({ - "error": format!("Generation failed: {}", error_msg) - })) + ApiError::internal(format!("Generation failed: {}", error_msg)).into_response() } _ => { // pending or unknown status - trigger generation @@ -569,7 +574,7 @@ pub async fn get_video_preview( } Err(_) => { span.set_status(Status::error("Database error")); - HttpResponse::InternalServerError().json(serde_json::json!({"error": "Database error"})) + ApiError::internal("Database error").into_response() } } } @@ -587,8 +592,9 @@ pub async fn get_preview_status( // Limit to 200 paths per request if body.paths.len() > 200 { span.set_status(Status::error("Too many paths")); - return HttpResponse::BadRequest() - .json(serde_json::json!({"error": "Maximum 200 paths per request"})); + return ApiError::bad_request("too_many_files", "At most 200 videos per request") + .with_details(serde_json::json!({ "max": 200 })) + .into_response(); } let previews = { @@ -665,11 +671,26 @@ pub async fn get_preview_status( } Err(_) => { span.set_status(Status::error("Database error")); - HttpResponse::InternalServerError().json(serde_json::json!({"error": "Database error"})) + ApiError::internal("Database error").into_response() } } } +/// The path names no file in the library. +fn file_not_found() -> ApiError { + ApiError::not_found("file_not_found", "That file doesn't exist") +} + +/// The path isn't a file in the library (400, as it has always been here). +fn invalid_path() -> ApiError { + ApiError::bad_request("invalid_path", "That file isn't in the library") +} + +/// An HLS playlist/segment that isn't (or is no longer) generated. +fn stream_not_found() -> ApiError { + ApiError::not_found("stream_not_found", "This video isn't ready to stream yet") +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/knowledge.rs b/src/knowledge.rs index 334b36b..3b88134 100644 --- a/src/knowledge.rs +++ b/src/knowledge.rs @@ -1,3 +1,4 @@ +use crate::error::ApiError; use actix_web::dev::{ServiceFactory, ServiceRequest}; use actix_web::{App, HttpRequest, HttpResponse, Responder, web}; use chrono::Utc; @@ -519,7 +520,7 @@ async fn list_entities( } Err(e) => { log::error!("list_entities error: {:?}", e); - HttpResponse::InternalServerError().json(serde_json::json!({"error": "Database error"})) + ApiError::internal("Database error").into_response() } } } @@ -538,12 +539,12 @@ async fn get_entity( let entity = match dao.get_entity_by_id(entity_id) { Ok(Some(e)) => e, Ok(None) => { - return HttpResponse::NotFound().json(serde_json::json!({"error": "Entity not found"})); + return ApiError::not_found("entity_not_found", "That entity no longer exists") + .into_response(); } Err(e) => { log::error!("get_entity error: {:?}", e); - return HttpResponse::InternalServerError() - .json(serde_json::json!({"error": "Database error"})); + return ApiError::internal("Database error").into_response(); } }; @@ -552,8 +553,7 @@ async fn get_entity( Ok(f) => f, Err(e) => { log::error!("get_facts_for_entity error: {:?}", e); - return HttpResponse::InternalServerError() - .json(serde_json::json!({"error": "Database error"})); + return ApiError::internal("Database error").into_response(); } }; @@ -644,8 +644,7 @@ async fn get_entity( Ok(links) => links.into_iter().map(PhotoLinkDetail::from).collect(), Err(e) => { log::error!("get_links_for_entity error: {:?}", e); - return HttpResponse::InternalServerError() - .json(serde_json::json!({"error": "Database error"})); + return ApiError::internal("Database error").into_response(); } }; @@ -693,10 +692,12 @@ async fn patch_entity( let mut dao = dao.lock().expect("Unable to lock KnowledgeDao"); match dao.update_entity(entity_id, patch) { Ok(Some(entity)) => HttpResponse::Ok().json(EntitySummary::from(entity)), - Ok(None) => HttpResponse::NotFound().json(serde_json::json!({"error": "Entity not found"})), + Ok(None) => { + ApiError::not_found("entity_not_found", "That entity no longer exists").into_response() + } Err(e) => { log::error!("patch_entity error: {:?}", e); - HttpResponse::InternalServerError().json(serde_json::json!({"error": "Database error"})) + ApiError::internal("Database error").into_response() } } } @@ -712,12 +713,12 @@ async fn delete_entity( // Verify entity exists before deleting match dao.get_entity_by_id(entity_id) { Ok(None) => { - return HttpResponse::NotFound().json(serde_json::json!({"error": "Entity not found"})); + return ApiError::not_found("entity_not_found", "That entity no longer exists") + .into_response(); } Err(e) => { log::error!("delete_entity lookup error: {:?}", e); - return HttpResponse::InternalServerError() - .json(serde_json::json!({"error": "Database error"})); + return ApiError::internal("Database error").into_response(); } Ok(Some(_)) => {} } @@ -726,7 +727,7 @@ async fn delete_entity( Ok(()) => HttpResponse::NoContent().finish(), Err(e) => { log::error!("delete_entity error: {:?}", e); - HttpResponse::InternalServerError().json(serde_json::json!({"error": "Database error"})) + ApiError::internal("Database error").into_response() } } } @@ -737,8 +738,8 @@ async fn merge_entities( dao: web::Data>, ) -> impl Responder { if body.source_id == body.target_id { - return HttpResponse::BadRequest() - .json(serde_json::json!({"error": "source_id and target_id must be different"})); + return ApiError::bad_request("merge_into_self", "An entity can't be merged into itself") + .into_response(); } let mut dao = dao.lock().expect("Unable to lock KnowledgeDao"); @@ -747,13 +748,15 @@ async fn merge_entities( for id in [body.source_id, body.target_id] { match dao.get_entity_by_id(id) { Ok(None) => { - return HttpResponse::BadRequest() - .json(serde_json::json!({"error": format!("Entity {} not found", id)})); + return ApiError::bad_request( + "entity_not_found", + format!("Entity {} no longer exists", id), + ) + .into_response(); } Err(e) => { log::error!("merge_entities lookup error: {:?}", e); - return HttpResponse::InternalServerError() - .json(serde_json::json!({"error": "Database error"})); + return ApiError::internal("Database error").into_response(); } Ok(Some(_)) => {} } @@ -768,7 +771,7 @@ async fn merge_entities( }), Err(e) => { log::error!("merge_entities error: {:?}", e); - HttpResponse::InternalServerError().json(serde_json::json!({"error": "Database error"})) + ApiError::internal("Database error").into_response() } } } @@ -790,8 +793,8 @@ async fn synthesize_merge( app_state: web::Data, ) -> impl Responder { if body.source_id == body.target_id { - return HttpResponse::BadRequest() - .json(serde_json::json!({"error": "source_id and target_id must differ"})); + return ApiError::bad_request("merge_into_self", "An entity can't be merged into itself") + .into_response(); } let (source, target) = { @@ -800,25 +803,29 @@ async fn synthesize_merge( let source = match dao.get_entity_by_id(body.source_id) { Ok(Some(e)) => e, Ok(None) => { - return HttpResponse::BadRequest() - .json(serde_json::json!({"error": "source entity not found"})); + return ApiError::bad_request( + "entity_not_found", + "The entity to merge no longer exists", + ) + .into_response(); } Err(e) => { log::error!("synthesize_merge source lookup: {:?}", e); - return HttpResponse::InternalServerError() - .json(serde_json::json!({"error": "Database error"})); + return ApiError::internal("Database error").into_response(); } }; let target = match dao.get_entity_by_id(body.target_id) { Ok(Some(e)) => e, Ok(None) => { - return HttpResponse::BadRequest() - .json(serde_json::json!({"error": "target entity not found"})); + return ApiError::bad_request( + "entity_not_found", + "The entity to merge into no longer exists", + ) + .into_response(); } Err(e) => { log::error!("synthesize_merge target lookup: {:?}", e); - return HttpResponse::InternalServerError() - .json(serde_json::json!({"error": "Database error"})); + return ApiError::internal("Database error").into_response(); } }; (source, target) @@ -890,9 +897,11 @@ async fn synthesize_merge( } Err(e) => { log::warn!("synthesize_merge generate failed: {:?}", e); - return HttpResponse::ServiceUnavailable().json(serde_json::json!({ - "error": "LLM unavailable; the merge picker should fall back to skip-synthesis." - })); + return ApiError::unavailable( + "llm_unavailable", + "The language model is unavailable. Merge without a summary, or try again later.", + ) + .into_response(); } }; @@ -967,7 +976,7 @@ async fn list_facts( } Err(e) => { log::error!("list_facts error: {:?}", e); - HttpResponse::InternalServerError().json(serde_json::json!({"error": "Database error"})) + ApiError::internal("Database error").into_response() } } } @@ -980,13 +989,11 @@ async fn create_fact( persona_dao: PersonaDaoData, ) -> impl Responder { if body.object_entity_id.is_none() && body.object_value.is_none() { - return HttpResponse::BadRequest().json(serde_json::json!({ - "error": "object_entity_id or object_value is required" - })); + return ApiError::bad_request("invalid_fact", "A fact needs an object entity or a value") + .into_response(); } if body.predicate.trim().is_empty() { - return HttpResponse::BadRequest() - .json(serde_json::json!({"error": "predicate must not be empty"})); + return ApiError::bad_request("invalid_fact", "A fact needs a predicate").into_response(); } // Persona scoping: facts are written under the active single persona. @@ -1006,14 +1013,15 @@ async fn create_fact( // Verify subject entity exists. match dao.get_entity_by_id(body.subject_entity_id) { Ok(None) => { - return HttpResponse::BadRequest().json(serde_json::json!({ - "error": format!("Subject entity {} not found", body.subject_entity_id) - })); + return ApiError::bad_request( + "entity_not_found", + "The fact's subject no longer exists", + ) + .into_response(); } Err(e) => { log::error!("create_fact subject lookup error: {:?}", e); - return HttpResponse::InternalServerError() - .json(serde_json::json!({"error": "Database error"})); + return ApiError::internal("Database error").into_response(); } Ok(Some(_)) => {} } @@ -1022,14 +1030,15 @@ async fn create_fact( if let Some(oid) = body.object_entity_id { match dao.get_entity_by_id(oid) { Ok(None) => { - return HttpResponse::BadRequest().json(serde_json::json!({ - "error": format!("Object entity {} not found", oid) - })); + return ApiError::bad_request( + "entity_not_found", + "The fact's object no longer exists", + ) + .into_response(); } Err(e) => { log::error!("create_fact object lookup error: {:?}", e); - return HttpResponse::InternalServerError() - .json(serde_json::json!({"error": "Database error"})); + return ApiError::internal("Database error").into_response(); } Ok(Some(_)) => {} } @@ -1074,7 +1083,7 @@ async fn create_fact( } Err(e) => { log::error!("create_fact upsert error: {:?}", e); - HttpResponse::InternalServerError().json(serde_json::json!({"error": "Database error"})) + ApiError::internal("Database error").into_response() } } } @@ -1101,10 +1110,12 @@ async fn patch_fact( // agent corrections. match dao.update_fact(fact_id, patch, Some(("manual", "manual"))) { Ok(Some(fact)) => HttpResponse::Ok().json(fact), - Ok(None) => HttpResponse::NotFound().json(serde_json::json!({"error": "Fact not found"})), + Ok(None) => { + ApiError::not_found("fact_not_found", "That fact no longer exists").into_response() + } Err(e) => { log::error!("patch_fact error: {:?}", e); - HttpResponse::InternalServerError().json(serde_json::json!({"error": "Database error"})) + ApiError::internal("Database error").into_response() } } } @@ -1120,7 +1131,7 @@ async fn delete_fact( Ok(()) => HttpResponse::NoContent().finish(), Err(e) => { log::warn!("delete_fact({}) error: {:?}", fact_id, e); - HttpResponse::NotFound().json(serde_json::json!({"error": "Fact not found"})) + ApiError::not_found("fact_not_found", "That fact no longer exists").into_response() } } } @@ -1133,8 +1144,8 @@ async fn supersede_fact( ) -> impl Responder { let old_id = id.into_inner(); if old_id == body.by_fact_id { - return HttpResponse::BadRequest() - .json(serde_json::json!({"error": "old_id and by_fact_id must differ"})); + return ApiError::bad_request("supersede_self", "A fact can't replace itself") + .into_response(); } let mut dao = dao.lock().expect("Unable to lock KnowledgeDao"); // Manual supersede from the curation UI — same stamping rule as @@ -1142,11 +1153,11 @@ async fn supersede_fact( match dao.supersede_fact(old_id, body.by_fact_id, Some(("manual", "manual"))) { Ok(Some(fact)) => HttpResponse::Ok().json(fact), Ok(None) => { - HttpResponse::NotFound().json(serde_json::json!({"error": "Old or new fact not found"})) + ApiError::not_found("fact_not_found", "That fact no longer exists").into_response() } Err(e) => { log::error!("supersede_fact error: {:?}", e); - HttpResponse::InternalServerError().json(serde_json::json!({"error": "Database error"})) + ApiError::internal("Database error").into_response() } } } @@ -1160,10 +1171,12 @@ async fn restore_fact( let mut dao = dao.lock().expect("Unable to lock KnowledgeDao"); match dao.revert_supersession(fact_id, Some(("manual", "manual"))) { Ok(Some(fact)) => HttpResponse::Ok().json(fact), - Ok(None) => HttpResponse::NotFound().json(serde_json::json!({"error": "Fact not found"})), + Ok(None) => { + ApiError::not_found("fact_not_found", "That fact no longer exists").into_response() + } Err(e) => { log::error!("restore_fact error: {:?}", e); - HttpResponse::InternalServerError().json(serde_json::json!({"error": "Database error"})) + ApiError::internal("Database error").into_response() } } } @@ -1193,7 +1206,7 @@ async fn get_recent( } Err(e) => { log::error!("get_recent error: {:?}", e); - HttpResponse::InternalServerError().json(serde_json::json!({"error": "Database error"})) + ApiError::internal("Database error").into_response() } } } @@ -1217,7 +1230,7 @@ async fn get_predicate_stats( }), Err(e) => { log::error!("get_predicate_stats error: {:?}", e); - HttpResponse::InternalServerError().json(serde_json::json!({"error": "Database error"})) + ApiError::internal("Database error").into_response() } } } @@ -1231,8 +1244,7 @@ async fn bulk_reject_predicate( ) -> impl Responder { let predicate = predicate.into_inner(); if predicate.trim().is_empty() { - return HttpResponse::BadRequest() - .json(serde_json::json!({"error": "predicate must not be empty"})); + return ApiError::bad_request("invalid_fact", "A fact needs a predicate").into_response(); } let persona = resolve_persona_filter(&req, &claims, &persona_dao); let mut dao = dao.lock().expect("Unable to lock KnowledgeDao"); @@ -1240,7 +1252,7 @@ async fn bulk_reject_predicate( Ok(rejected) => HttpResponse::Ok().json(BulkRejectResponse { rejected }), Err(e) => { log::error!("bulk_reject_predicate error: {:?}", e); - HttpResponse::InternalServerError().json(serde_json::json!({"error": "Database error"})) + ApiError::internal("Database error").into_response() } } } @@ -1278,7 +1290,7 @@ async fn get_graph( }), Err(e) => { log::error!("build_entity_graph error: {:?}", e); - HttpResponse::InternalServerError().json(serde_json::json!({"error": "Database error"})) + ApiError::internal("Database error").into_response() } } } @@ -1302,8 +1314,7 @@ async fn get_consolidation_proposals( Ok(g) => g, Err(e) => { log::error!("find_consolidation_proposals: {:?}", e); - return HttpResponse::InternalServerError() - .json(serde_json::json!({"error": "Database error"})); + return ApiError::internal("Database error").into_response(); } }; diff --git a/src/libraries.rs b/src/libraries.rs index aada324..05a07bd 100644 --- a/src/libraries.rs +++ b/src/libraries.rs @@ -1,3 +1,4 @@ +use crate::error::ApiError; use actix_web::{HttpResponse, Responder, get, patch, web, web::Data}; use chrono::Utc; use diesel::prelude::*; @@ -319,6 +320,22 @@ pub fn resolve_library_param<'a>( /// Resolve a library request parameter against the AppState's libraries. /// Returns `Ok(None)` when the param is absent, meaning "span all libraries". /// Returns `Err` when a value is provided but does not match any library. +/// The response to a `library` param that names no library. +pub fn unknown_library(reason: impl std::fmt::Display) -> ApiError { + ApiError::bad_request( + "unknown_library", + "That library doesn't exist on this server", + ) + .with_details(serde_json::json!({ "reason": reason.to_string() })) +} + +fn library_not_found() -> ApiError { + ApiError::not_found( + "library_not_found", + "That library doesn't exist on this server", + ) +} + pub fn resolve_library_param_state<'a>( state: &'a AppState, param: Option<&str>, @@ -546,7 +563,12 @@ pub async fn patch_library( let body = body.into_inner(); if body.enabled.is_none() && body.excluded_dirs.is_none() { - return HttpResponse::UnprocessableEntity().body("empty patch body"); + return ApiError::new( + actix_web::http::StatusCode::UNPROCESSABLE_ENTITY, + "nothing_to_change", + "Nothing to change", + ) + .into_response(); } let mut conn = crate::database::connect(); @@ -566,7 +588,7 @@ pub async fn patch_library( "PATCH /libraries/{}: enabled update failed: {:?}", lib_id, e ); - return HttpResponse::InternalServerError().body(format!("{}", e)); + return ApiError::internal(format!("{}", e)).into_response(); } } } @@ -579,7 +601,15 @@ pub async fn patch_library( // back as 422 so the operator can fix the typo. let normalised = match normalize_excluded_dirs_input(raw) { Ok(v) => v, - Err(msg) => return HttpResponse::UnprocessableEntity().body(msg), + // The message names the entry at fault and how to fix it. + Err(msg) => { + return ApiError::new( + actix_web::http::StatusCode::UNPROCESSABLE_ENTITY, + "invalid_excluded_dirs", + msg, + ) + .into_response(); + } }; let stored: Option<&str> = normalised.as_deref(); match diesel::update(libraries::table.filter(libraries::id.eq(lib_id))) @@ -592,13 +622,13 @@ pub async fn patch_library( "PATCH /libraries/{}: excluded_dirs update failed: {:?}", lib_id, e ); - return HttpResponse::InternalServerError().body(format!("{}", e)); + return ApiError::internal(format!("{}", e)).into_response(); } } } if affected == 0 { - return HttpResponse::NotFound().body(format!("library id {} not found", lib_id)); + return library_not_found().into_response(); } // Refresh the live view from the canonical DB state. Reloading the @@ -623,9 +653,7 @@ pub async fn patch_library( ); HttpResponse::Ok().json(lib) } - None => { - HttpResponse::NotFound().body(format!("library id {} not found after update", lib_id)) - } + None => library_not_found().into_response(), } } diff --git a/src/main.rs b/src/main.rs index fe46de7..97a52bd 100644 --- a/src/main.rs +++ b/src/main.rs @@ -11,7 +11,6 @@ use std::sync::{Arc, Mutex}; use actix_cors::Cors; use actix_governor::{Governor, GovernorConfigBuilder}; -use actix_multipart as mp; use actix_web::{App, HttpServer, middleware, web}; use diesel::sqlite::Sqlite; use std::error::Error; @@ -443,7 +442,7 @@ fn main() -> std::io::Result<()> { .app_data::>(Data::new( app_data.face_client.clone(), )) - .app_data(mp::form::MultipartFormConfig::default().total_limit(1024 * 1024 * 1024)) // 1GB upload limit + .app_data(image_api::error::multipart_config(1024 * 1024 * 1024)) // 1GB upload limit // Request-parsing failures and unknown routes answer in the // shared error format (#202), not actix's plain text. .app_data(image_api::error::json_config()) diff --git a/src/memories.rs b/src/memories.rs index 496c299..a741d00 100644 --- a/src/memories.rs +++ b/src/memories.rs @@ -1,3 +1,4 @@ +use crate::error::ApiError; use actix_web::web::Data; use actix_web::{HttpResponse, Responder, get, web}; use chrono::{DateTime, Datelike, NaiveDate, NaiveDateTime, TimeZone, Utc}; @@ -569,7 +570,11 @@ pub async fn list_memories( Ok(d) => d, Err(msg) => { warn!("Rejecting /memories request: {}", msg); - return HttpResponse::BadRequest().body(msg); + return ApiError::bad_request( + "invalid_date", + "That date isn't valid (expected YYYY-MM-DD)", + ) + .into_response(); } }; @@ -584,7 +589,7 @@ pub async fn list_memories( Ok(items) => items, Err(msg) => { warn!("Rejecting /memories request: {}", msg); - return HttpResponse::BadRequest().body(msg); + return crate::libraries::unknown_library(msg).into_response(); } }; diff --git a/src/personas.rs b/src/personas.rs index f03f00d..308ec75 100644 --- a/src/personas.rs +++ b/src/personas.rs @@ -8,6 +8,7 @@ //! migration. Customs are created here and may be migrated up from a //! device's local store via `POST /personas/migrate`. +use crate::error::ApiError; use actix_web::dev::{ServiceFactory, ServiceRequest}; use actix_web::{App, HttpResponse, Responder, web}; use serde::{Deserialize, Serialize}; @@ -148,7 +149,8 @@ fn user_id_from_claims(claims: &Claims) -> Option { async fn list_personas(claims: Claims, dao: PersonaDaoData) -> impl Responder { let Some(uid) = user_id_from_claims(&claims) else { - return HttpResponse::Unauthorized().json(serde_json::json!({"error": "Invalid claims"})); + return ApiError::unauthorized("unauthorized", "Your session has expired. Sign in again.") + .into_response(); }; let mut dao = dao.lock().expect("Unable to lock PersonaDao"); match dao.list_personas(uid) { @@ -158,7 +160,7 @@ async fn list_personas(claims: Claims, dao: PersonaDaoData) -> impl Responder { } Err(e) => { log::error!("list_personas error: {:?}", e); - HttpResponse::InternalServerError().json(serde_json::json!({"error": "Database error"})) + ApiError::internal("Database error").into_response() } } } @@ -169,15 +171,16 @@ async fn create_persona( dao: PersonaDaoData, ) -> impl Responder { let Some(uid) = user_id_from_claims(&claims) else { - return HttpResponse::Unauthorized().json(serde_json::json!({"error": "Invalid claims"})); + return ApiError::unauthorized("unauthorized", "Your session has expired. Sign in again.") + .into_response(); }; if body.name.trim().is_empty() { - return HttpResponse::BadRequest().json(serde_json::json!({"error": "name is required"})); + return ApiError::bad_request("invalid_persona", "Give the persona a name").into_response(); } if body.system_prompt.trim().is_empty() { - return HttpResponse::BadRequest() - .json(serde_json::json!({"error": "systemPrompt is required"})); + return ApiError::bad_request("invalid_persona", "Give the persona a system prompt") + .into_response(); } let mut dao = dao.lock().expect("Unable to lock PersonaDao"); @@ -188,23 +191,26 @@ async fn create_persona( }; if matches!(pid.as_str(), "default" | "journal" | "factual") { - return HttpResponse::Conflict() - .json(serde_json::json!({"error": "persona id collides with a built-in"})); + return ApiError::conflict( + "persona_id_taken", + "A built-in persona already uses that id", + ) + .into_response(); } // Pre-check existence so we can return 409 cleanly. The DB UNIQUE // would also catch it, but parsing Diesel's "constraint violation" // out of a generic DbError is uglier than a quick lookup. if let Ok(Some(_)) = dao.get_persona(uid, &pid) { - return HttpResponse::Conflict() - .json(serde_json::json!({"error": "persona already exists"})); + return ApiError::conflict("persona_id_taken", "That persona already exists") + .into_response(); } match dao.create_persona(uid, &pid, &body.name, &body.system_prompt, false, false) { Ok(p) => HttpResponse::Created().json(PersonaView::from(p)), Err(e) => { log::error!("create_persona error: {:?}", e); - HttpResponse::InternalServerError().json(serde_json::json!({"error": "Database error"})) + ApiError::internal("Database error").into_response() } } } @@ -216,7 +222,8 @@ async fn update_persona( dao: PersonaDaoData, ) -> impl Responder { let Some(uid) = user_id_from_claims(&claims) else { - return HttpResponse::Unauthorized().json(serde_json::json!({"error": "Invalid claims"})); + return ApiError::unauthorized("unauthorized", "Your session has expired. Sign in again.") + .into_response(); }; let pid = path.into_inner(); let mut dao = dao.lock().expect("Unable to lock PersonaDao"); @@ -232,19 +239,20 @@ async fn update_persona( Ok(Some(p)) if p.is_built_in => { let editing_identity = body.name.is_some() || body.system_prompt.is_some(); if editing_identity { - return HttpResponse::Conflict().json(serde_json::json!({ - "error": "Cannot edit name or systemPrompt of a built-in persona" - })); + return ApiError::conflict( + "builtin_persona", + "A built-in persona's name and prompt can't be changed", + ) + .into_response(); } } Ok(None) => { - return HttpResponse::NotFound() - .json(serde_json::json!({"error": "Persona not found"})); + return ApiError::not_found("persona_not_found", "That persona no longer exists") + .into_response(); } Err(e) => { log::error!("update_persona lookup error: {:?}", e); - return HttpResponse::InternalServerError() - .json(serde_json::json!({"error": "Database error"})); + return ApiError::internal("Database error").into_response(); } Ok(Some(_)) => {} } @@ -258,12 +266,11 @@ async fn update_persona( }; match dao.update_persona(uid, &pid, patch) { Ok(Some(p)) => HttpResponse::Ok().json(PersonaView::from(p)), - Ok(None) => { - HttpResponse::NotFound().json(serde_json::json!({"error": "Persona not found"})) - } + Ok(None) => ApiError::not_found("persona_not_found", "That persona no longer exists") + .into_response(), Err(e) => { log::error!("update_persona error: {:?}", e); - HttpResponse::InternalServerError().json(serde_json::json!({"error": "Database error"})) + ApiError::internal("Database error").into_response() } } } @@ -274,24 +281,24 @@ async fn delete_persona( dao: PersonaDaoData, ) -> impl Responder { let Some(uid) = user_id_from_claims(&claims) else { - return HttpResponse::Unauthorized().json(serde_json::json!({"error": "Invalid claims"})); + return ApiError::unauthorized("unauthorized", "Your session has expired. Sign in again.") + .into_response(); }; let pid = path.into_inner(); let mut dao = dao.lock().expect("Unable to lock PersonaDao"); match dao.get_persona(uid, &pid) { Ok(Some(p)) if p.is_built_in => { - return HttpResponse::Conflict() - .json(serde_json::json!({"error": "Cannot delete built-in persona"})); + return ApiError::conflict("builtin_persona", "Built-in personas can't be deleted") + .into_response(); } Ok(None) => { - return HttpResponse::NotFound() - .json(serde_json::json!({"error": "Persona not found"})); + return ApiError::not_found("persona_not_found", "That persona no longer exists") + .into_response(); } Err(e) => { log::error!("delete_persona lookup error: {:?}", e); - return HttpResponse::InternalServerError() - .json(serde_json::json!({"error": "Database error"})); + return ApiError::internal("Database error").into_response(); } Ok(Some(_)) => {} } @@ -300,7 +307,7 @@ async fn delete_persona( Ok(_) => HttpResponse::NoContent().finish(), Err(e) => { log::error!("delete_persona error: {:?}", e); - HttpResponse::InternalServerError().json(serde_json::json!({"error": "Database error"})) + ApiError::internal("Database error").into_response() } } } @@ -311,7 +318,8 @@ async fn migrate_personas( dao: PersonaDaoData, ) -> impl Responder { let Some(uid) = user_id_from_claims(&claims) else { - return HttpResponse::Unauthorized().json(serde_json::json!({"error": "Invalid claims"})); + return ApiError::unauthorized("unauthorized", "Your session has expired. Sign in again.") + .into_response(); }; let mut dao = dao.lock().expect("Unable to lock PersonaDao"); @@ -336,7 +344,7 @@ async fn migrate_personas( Ok(inserted) => HttpResponse::Ok().json(MigrateResponse { inserted }), Err(e) => { log::error!("migrate_personas error: {:?}", e); - HttpResponse::InternalServerError().json(serde_json::json!({"error": "Database error"})) + ApiError::internal("Database error").into_response() } } } diff --git a/src/prefs.rs b/src/prefs.rs index 39c0d7a..07d95a0 100644 --- a/src/prefs.rs +++ b/src/prefs.rs @@ -18,11 +18,11 @@ //! single row, so they're idempotent. The legacy `tz_offset_minutes` //! column is preserved verbatim on every write. +use crate::error::ApiError; use actix_web::{HttpResponse, web}; use chrono_tz::Tz; use serde::de::IntoDeserializer; use serde::{Deserialize, Serialize}; -use serde_json::json; use std::str::FromStr; use std::time::{SystemTime, UNIX_EPOCH}; @@ -264,16 +264,12 @@ pub async fn get_prefs(_claims: Claims, app_state: web::Data) -> HttpR let mut dao = match app_state.user_ai_prefs_dao.lock() { Ok(dao) => dao, Err(_) => { - return HttpResponse::InternalServerError().json(json!({ - "error": "prefs store unavailable" - })); + return ApiError::internal("prefs store unavailable").into_response(); } }; match dao.get_prefs() { Ok(row) => HttpResponse::Ok().json(row_to_response(row.as_ref())), - Err(e) => HttpResponse::InternalServerError().json(json!({ - "error": format!("failed to read prefs: {e}") - })), + Err(e) => ApiError::internal(format!("failed to read prefs: {e}")).into_response(), } } @@ -289,39 +285,38 @@ pub async fn put_prefs( if let TriState::Val(name) = &patch.display_tz && !is_valid_display_tz(name) { - return HttpResponse::BadRequest().json(json!({ - "error": format!("invalid display_tz: {name}") - })); + return ApiError::bad_request("invalid_time_zone", format!("\"{name}\" isn't a time zone")) + .into_response(); } if let TriState::Val(name) = &patch.default_backend && !is_valid_backend(name) { - return HttpResponse::BadRequest().json(json!({ - "error": format!("invalid default_backend: {name} (expected local|hybrid)") - })); + return ApiError::bad_request( + "invalid_backend", + format!("\"{name}\" isn't a backend (expected local or hybrid)"), + ) + .into_response(); } if let TriState::Val(name) = &patch.reel_span && MemoriesSpan::from_str(name).is_err() { - return HttpResponse::BadRequest().json(json!({ - "error": format!("invalid reel_span: {name} (expected day|week|month)") - })); + return ApiError::bad_request( + "invalid_span", + format!("\"{name}\" isn't a reel span (expected day, week or month)"), + ) + .into_response(); } let mut dao = match app_state.user_ai_prefs_dao.lock() { Ok(dao) => dao, Err(_) => { - return HttpResponse::InternalServerError().json(json!({ - "error": "prefs store unavailable" - })); + return ApiError::internal("prefs store unavailable").into_response(); } }; let existing = match dao.get_prefs() { Ok(row) => row, Err(e) => { - return HttpResponse::InternalServerError().json(json!({ - "error": format!("failed to read prefs: {e}") - })); + return ApiError::internal(format!("failed to read prefs: {e}")).into_response(); } }; @@ -363,9 +358,7 @@ pub async fn put_prefs( }; match dao.upsert_prefs(&upsert) { Ok(()) => HttpResponse::Ok().json(resp), - Err(e) => HttpResponse::InternalServerError().json(json!({ - "error": format!("failed to write prefs: {e}") - })), + Err(e) => ApiError::internal(format!("failed to write prefs: {e}")).into_response(), } } diff --git a/src/reels/mod.rs b/src/reels/mod.rs index b4ddc6d..8281560 100644 --- a/src/reels/mod.rs +++ b/src/reels/mod.rs @@ -16,6 +16,7 @@ pub mod render; pub mod script; pub mod selector; +use crate::error::ApiError; use std::collections::HashMap; use std::path::{Path, PathBuf}; use std::str::FromStr; @@ -449,9 +450,11 @@ pub async fn create_reel_handler( insight_dao: web::Data>>, ) -> impl Responder { if app_state.llamacpp.is_none() { - return HttpResponse::ServiceUnavailable().json(json!({ - "error": "Reel narration needs the LLM/TTS backend (set llamacpp.url in llm.toml)" - })); + return ApiError::unavailable( + "reels_unavailable", + "Reels need the language model and voice backend, which isn't set up on this server", + ) + .into_response(); } // Owner defaults: explicit request param → stored pref → hard default. @@ -482,12 +485,15 @@ pub async fn create_reel_handler( // check. Insight enrichment + scripting happen in the background job. let (planned, meta) = match selector::resolve(&app_state, &exif_dao, &selector) { Ok(r) => r, - Err(msg) => return HttpResponse::BadRequest().body(msg), + Err(msg) => return ApiError::bad_request("invalid_reel_request", msg).into_response(), }; if planned.is_empty() { - return HttpResponse::UnprocessableEntity().json(json!({ - "error": "No photo memories found for this span" - })); + return ApiError::new( + actix_web::http::StatusCode::UNPROCESSABLE_ENTITY, + "no_memories", + "No photos found for this span", + ) + .into_response(); } // Flatten every media item across beats (in order) into the cache key — the @@ -609,7 +615,7 @@ pub async fn create_reel_handler( pub async fn reel_status_handler(_claims: Claims, path: web::Path) -> impl Responder { let id_str = path.into_inner(); let Ok(id) = Uuid::parse_str(&id_str) else { - return HttpResponse::BadRequest().json(json!({ "error": "invalid job id" })); + return ApiError::bad_request("invalid_job_id", "Invalid reel job").into_response(); }; let resp = with_job(id, |job| ReelStatusResponse { job_id: id_str.clone(), @@ -622,7 +628,7 @@ pub async fn reel_status_handler(_claims: Claims, path: web::Path) -> im }); match resp { Some(r) => HttpResponse::Ok().json(r), - None => HttpResponse::NotFound().json(json!({ "error": "job not found or expired" })), + None => ApiError::not_found("job_not_found", "That reel job has expired").into_response(), } } @@ -636,17 +642,17 @@ pub async fn reel_video_handler( ) -> impl Responder { let id_str = path.into_inner(); let Ok(id) = Uuid::parse_str(&id_str) else { - return HttpResponse::BadRequest().json(json!({ "error": "invalid job id" })); + return ApiError::bad_request("invalid_job_id", "Invalid reel job").into_response(); }; let output = with_job(id, |job| job.output_path.clone()).flatten(); let Some(path) = output else { - return HttpResponse::NotFound().json(json!({ "error": "reel not ready" })); + return ApiError::not_found("reel_not_ready", "The reel isn't ready yet").into_response(); }; match NamedFile::open(&path) { Ok(file) => file.into_response(&request), Err(e) => { log::error!("opening reel mp4 {path:?} failed: {e:?}"); - HttpResponse::NotFound().json(json!({ "error": "reel file missing" })) + reel_missing().into_response() } } } @@ -694,32 +700,31 @@ pub async fn precomputed_reel_handler( .exists_fresh(span, &library_key, RENDER_VERSION as i32, min_generated_at) .unwrap_or(false) { - return HttpResponse::NotFound().json(json!({ "error": "no precomputed reel found" })); + return no_precomputed_reel("none").into_response(); } // Fetch the latest row for full validity checks. let reel = match dao.latest_for(span, &library_key) { Ok(Some(r)) => r, _ => { - return HttpResponse::NotFound().json(json!({ "error": "no precomputed reel found" })); + return no_precomputed_reel("none").into_response(); } }; // Validity gate 1: render version must match. if reel.render_version != RENDER_VERSION as i32 { - return HttpResponse::NotFound() - .json(json!({ "error": "precomputed reel is stale (render version mismatch)" })); + return no_precomputed_reel("stale").into_response(); } // Validity gate 2: output_path must exist. let output = std::path::Path::new(&reel.output_path); if !output.exists() { - return HttpResponse::NotFound().json(json!({ "error": "precomputed reel file missing" })); + return no_precomputed_reel("file_missing").into_response(); } // Validity gate 3: age <= max_age (re-checked via min_generated_at). if reel.generated_at < min_generated_at { - return HttpResponse::NotFound().json(json!({ "error": "precomputed reel has expired" })); + return no_precomputed_reel("expired").into_response(); } HttpResponse::Ok().json(PrecomputedReelResponse { @@ -742,7 +747,7 @@ pub async fn precomputed_video_handler( Ok(file) => file.into_response(&request), Err(e) => { log::error!("opening precomputed reel {key} failed: {e:?}"); - HttpResponse::NotFound().json(json!({ "error": "precomputed reel file missing" })) + reel_missing().into_response() } } } @@ -1368,6 +1373,18 @@ fn sweep_reel_cache(app_state: &AppState) -> usize { removed } +/// No usable precomputed reel; `reason` (none / stale / file_missing / +/// expired) is for debugging, the client just generates one. +fn no_precomputed_reel(reason: &str) -> ApiError { + ApiError::not_found("no_precomputed_reel", "No ready-made reel for this span") + .with_details(json!({ "reason": reason })) +} + +/// The job finished but its video file is gone (cleaned up or moved). +fn reel_missing() -> ApiError { + ApiError::not_found("reel_missing", "The reel's video is gone. Make it again.") +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/unified_search.rs b/src/unified_search.rs index 042d546..d5de465 100644 --- a/src/unified_search.rs +++ b/src/unified_search.rs @@ -27,6 +27,7 @@ use crate::clip_search::{ }; use crate::data::{Claims, FilterMode, MediaType}; use crate::database::ExifDao; +use crate::error::ApiError; use crate::file_types::{is_image_file, is_video_file}; use crate::geo::{forward_geocode, gps_bounding_box, haversine_distance}; use crate::tags::TagDao; @@ -264,14 +265,7 @@ struct UnifiedResponse { results: Vec, } -#[derive(Debug, Serialize)] -struct ErrorBody { - error: String, -} - -fn bad_request(msg: impl Into) -> HttpResponse { - HttpResponse::BadRequest().json(ErrorBody { error: msg.into() }) -} +// Errors use the shared format (crate::error::ApiError, #202). /// Combine the model's semantic term with any tag words that didn't match the /// vocab, so a hallucinated/non-vocab tag becomes a soft semantic signal @@ -306,7 +300,7 @@ pub async fn unified_search( ); let nl = query.q.trim().to_string(); if nl.is_empty() { - return bad_request("query parameter `q` is required"); + return crate::clip_search::empty_query().into_response(); } let limit = query.limit.clamp(1, 200); @@ -315,7 +309,7 @@ pub async fn unified_search( let library_ids = match parse_library_scope(query.library_ids.as_deref(), query.library) { Ok(ids) => ids, - Err(msg) => return bad_request(msg), + Err(msg) => return crate::libraries::unknown_library(msg).into_response(), }; // ── 1. Translate the NL query, grounded on the real tag vocabulary ── @@ -333,7 +327,13 @@ pub async fn unified_search( }; let (sq, echo, explicit_tags, explicit_gps) = match merge_explicit(sq, &query) { Ok(merged) => merged, - Err(msg) => return bad_request(msg), + Err(_) => { + return ApiError::bad_request( + "incomplete_gps_filter", + "A location filter needs a latitude, longitude and radius", + ) + .into_response(); + } }; let folder = folder_prefix(query.path.as_deref()); @@ -597,7 +597,8 @@ pub async fn unified_search( // Filters-only: no semantic term. Require at least one filter, // then return the candidate set newest-first. if !has_struct { - return bad_request("query had no searchable terms"); + return ApiError::bad_request("nothing_to_search", "Add search words or a filter") + .into_response(); } candidate.sort_by(|a, b| b.date_taken.cmp(&a.date_taken)); let total_matching = candidate.len(); @@ -684,9 +685,11 @@ async fn translate( Ok(b) => b, Err(e) => { log::warn!("unified_search: resolve_backend failed: {e:?}"); - return Err(HttpResponse::ServiceUnavailable().json(ErrorBody { - error: "LLM backend unavailable".into(), - })); + return Err(ApiError::unavailable( + "llm_unavailable", + "The language model is unavailable. Try again shortly.", + ) + .into_response()); } }; log::info!("unified_search: translating with model={}", backend.model()); @@ -700,9 +703,8 @@ async fn translate( .await .map_err(|e| { log::warn!("unified_search: translate_nl_query failed: {e:?}"); - HttpResponse::BadGateway().json(ErrorBody { - error: "could not interpret the query".into(), - }) + ApiError::bad_gateway("query_not_understood", "Couldn't understand that search") + .into_response() }) } diff --git a/src/versions/handlers.rs b/src/versions/handlers.rs index f88481d..47dd692 100644 --- a/src/versions/handlers.rs +++ b/src/versions/handlers.rs @@ -3,6 +3,7 @@ //! Both take the path of the original *or* of any version, and answer about //! the whole set. The display choice is global (not per user). +use crate::error::ApiError; use std::sync::Mutex; use actix_web::{HttpResponse, Responder, get, put, web}; @@ -84,14 +85,14 @@ fn resolve( ) -> Result { let library_id = match libraries::resolve_library_param_state(app_state, library) { Ok(lib) => lib.map(|l| l.id), - Err(msg) => return Err(HttpResponse::BadRequest().body(msg)), + Err(msg) => return Err(libraries::unknown_library(msg).into_response()), }; let path = normalize_path(path); let mut rows = exif_dao .lock() .expect("Unable to lock ExifDao") .get_exif_batch(library_id, std::slice::from_ref(&path)) - .map_err(|_| HttpResponse::InternalServerError().finish())?; + .map_err(|_| ApiError::internal("internal error").into_response())?; // Without `library`, a path can name rows in several libraries. Take the // earliest hashed one; it then resolves to its original below, so a // version in one library and its original in another agree. @@ -100,14 +101,18 @@ fn resolve( .iter() .find_map(|r| r.content_hash.clone().map(|h| (r, h))) else { - return Err(HttpResponse::NotFound().body("no indexed file at that path")); + return Err(ApiError::not_found( + "photo_not_indexed", + "This photo isn't indexed yet. Try again in a minute.", + ) + .into_response()); }; let link = app_state .version_dao .lock() .expect("Unable to lock VersionDao") .get_link(&hash) - .map_err(|_| HttpResponse::InternalServerError().finish())?; + .map_err(|_| ApiError::internal("internal error").into_response())?; Ok(Resolved { original_hash: link.map(|l| l.original_hash).unwrap_or(hash), library_id: row.library_id, @@ -196,7 +201,7 @@ pub async fn get_versions( Ok(body) => HttpResponse::Ok().json(body), Err(e) => { warn!("get_versions failed: {:?}", e); - HttpResponse::InternalServerError().finish() + ApiError::internal("internal error").into_response() } } } @@ -233,11 +238,15 @@ pub async fn put_display( Ok(r) => r, Err(e) => { warn!("put_display lookup failed: {:?}", e); - return HttpResponse::InternalServerError().finish(); + return ApiError::internal("internal error").into_response(); } }; if !current.versions.iter().any(|v| v.hash == hash) { - return HttpResponse::BadRequest().body("display_hash is not a version of this photo"); + return ApiError::bad_request( + "not_a_version", + "That file isn't a version of this photo", + ) + .into_response(); } // The display file is fetched through the original's library. let original_lib = current.original.as_ref().map(|o| o.library_id); @@ -246,19 +255,22 @@ pub async fn put_display( .iter() .any(|v| v.hash == hash && original_lib.is_none_or(|l| l == v.library_id)) { - return HttpResponse::BadRequest() - .body("display_hash has no file in the original's library"); + return ApiError::bad_request( + "version_in_other_library", + "That version isn't in the same library as the original", + ) + .into_response(); } } if let Err(e) = dao.set_display(&resolved.original_hash, wanted) { warn!("put_display failed: {:?}", e); - return HttpResponse::InternalServerError().finish(); + return ApiError::internal("internal error").into_response(); } match build_response(dao.as_mut(), &resolved.original_hash, resolved.library_id) { Ok(body) => HttpResponse::Ok().json(body), Err(e) => { warn!("put_display response failed: {:?}", e); - HttpResponse::InternalServerError().finish() + ApiError::internal("internal error").into_response() } } }