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