diff --git a/CHANGELOG.md b/CHANGELOG.md index 338fdec7..74fb187e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ ### Changed +- Internal: errors are classified once, part of #1517 (round 2 of the architecture deepening, #1523): `Error::NotFound` and `Error::class()` (`BadRequest`, `NotFound`, `SecretDetected`, `Internal`), and one `core_error` mapping in the server instead of seven hand-written `Validation => 400, else 500` arms. The two handlers that matched `contains("not found")` (`POST /doc/move`, `DELETE /room/delete`) now read the class. HTTP statuses do not change: a missing memory on update or a missing room on rename still answer 400 until the contract with uteke-cloud moves them. The one visible difference is the text of the `POST /doc/move` 404, which no longer starts with `Validation error:`. - Internal: `ort` 2.0.0-rc.12/rc.13 to rc.14 (replaces the dependabot PR, which cannot build): rc.14 builds sessions from an explicit `Environment`, so the one process-wide environment is now created in `embed::ort_init` and handed to `Session::builder`. The bundled ONNX Runtime stays at 1.24.4 (`api-18`). Checked with the real model: `remember` writes the embedding and `recall` finds it. - Internal: privacy filter modes and flag helpers, part 2b-1a of #1464 (`uteke_core::privacy`): `PrivacyMode` (`redact`, `strict`, `off`), `PrivacyConfig::prepare`, `Error::SecretDetected` and the `metadata.redacted`/`redactions` helpers. Not wired to a write path yet. - Internal: privacy filter token rules and PEM blocks, part 2b-1a of #1464 (`uteke_core::privacy`): Stripe, `whsec_`, Slack, Google, Basic and `Token`/`Bot` auth, `curl -u`, password flags and URL passwords, and PEM private key blocks in their escaped, CRLF, collapsed, quoted and string-literal forms. Scan windows only bound the work. The shared vectors move to version 3 (113 cases so far). diff --git a/crates/uteke-core/src/error.rs b/crates/uteke-core/src/error.rs index 8403fefc..383e431f 100644 --- a/crates/uteke-core/src/error.rs +++ b/crates/uteke-core/src/error.rs @@ -19,6 +19,14 @@ pub enum Error { #[error("Validation error: {0}")] Validation(String), + /// The thing a request names does not exist (a room, a document). The + /// message is shown as it is. Transports answer with a not-found status + /// instead of matching on message text. Sites that still report a missing + /// memory, room or namespace as [`Error::Validation`] keep their 400 until + /// the contract with uteke-cloud says otherwise. + #[error("{0}")] + NotFound(String), + /// Strict privacy mode (#1464): the content holds a secret and the write is /// refused. `kinds` names what was found (never the value). #[error("Secret detected in content ({}); strict privacy mode rejects the write", .kinds.join(", "))] @@ -31,6 +39,20 @@ pub enum Error { Generic(String), } +/// What kind of failure an [`Error`] is, for the transports (see +/// [`Error::class`]). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ErrorClass { + /// The request is wrong: HTTP 400. + BadRequest, + /// The named thing does not exist: HTTP 404. + NotFound, + /// Strict privacy mode refused the write: HTTP 400 `secret_detected`. + SecretDetected, + /// Anything else: HTTP 500. + Internal, +} + impl Error { // ── Convenience constructors with context ────────────────────────── @@ -74,6 +96,26 @@ impl Error { Error::Validation(msg.into()) } + /// Not-found error: the named room, document, ... does not exist. + pub fn not_found(msg: impl Into) -> Self { + Error::NotFound(msg.into()) + } + + /// How a transport should treat this error. One mapping for HTTP, MCP and + /// the CLI, instead of a `match` in every handler. + pub fn class(&self) -> ErrorClass { + match self { + Error::Validation(_) => ErrorClass::BadRequest, + Error::NotFound(_) => ErrorClass::NotFound, + Error::SecretDetected { .. } => ErrorClass::SecretDetected, + Error::Io(_) + | Error::Database { .. } + | Error::Embedding { .. } + | Error::Lock { .. } + | Error::Generic(_) => ErrorClass::Internal, + } + } + /// Generic error with a message. pub fn generic(msg: impl Into) -> Self { Error::Generic(msg.into()) diff --git a/crates/uteke-core/src/error_class_tests.rs b/crates/uteke-core/src/error_class_tests.rs new file mode 100644 index 00000000..7cee6769 --- /dev/null +++ b/crates/uteke-core/src/error_class_tests.rs @@ -0,0 +1,56 @@ +//! How errors are classified for the transports (#1517). + +use crate::test_support::open_keyword; +use crate::{Error, ErrorClass}; + +#[test] +fn every_variant_has_a_class() { + let io = Error::Io(std::io::Error::other("x")); + assert_eq!(io.class(), ErrorClass::Internal); + assert_eq!(Error::db_msg("x").class(), ErrorClass::Internal); + assert_eq!(Error::embed_msg("x").class(), ErrorClass::Internal); + assert_eq!(Error::lock("x").class(), ErrorClass::Internal); + assert_eq!(Error::generic("x").class(), ErrorClass::Internal); + assert_eq!(Error::validation("x").class(), ErrorClass::BadRequest); + assert_eq!(Error::not_found("x").class(), ErrorClass::NotFound); + let secret = Error::SecretDetected { + kinds: vec!["jwt".into()], + }; + assert_eq!(secret.class(), ErrorClass::SecretDetected); +} + +#[test] +fn a_not_found_error_shows_its_message_as_it_is() { + assert_eq!( + Error::not_found("Room not found: r1").to_string(), + "Room not found: r1" + ); + // Validation keeps its prefix: only the new variant is bare. + assert_eq!( + Error::validation("bad").to_string(), + "Validation error: bad" + ); +} + +#[test] +fn deleting_a_missing_room_and_moving_a_missing_document_are_not_found() { + let u = open_keyword(); + let err = u.delete_room("ghost").unwrap_err(); + assert_eq!(err.class(), ErrorClass::NotFound, "{err}"); + assert_eq!(err.to_string(), "Room not found: ghost"); + + let err = u.doc_move("no-such-doc", None, None).unwrap_err(); + assert_eq!(err.class(), ErrorClass::NotFound, "{err}"); + assert!(err.to_string().contains("not found"), "{err}"); +} + +#[test] +fn sites_that_still_answer_400_stay_validation() { + // The contract with uteke-cloud decides when these become NotFound; until + // then a missing memory on update and a missing room on rename are 400s. + let u = open_keyword(); + let err = u + .update_memory("no-such-memory", Some("x"), None, None, None, None, None) + .unwrap_err(); + assert_eq!(err.class(), ErrorClass::BadRequest, "{err}"); +} diff --git a/crates/uteke-core/src/lib.rs b/crates/uteke-core/src/lib.rs index 8f00341f..cdb08c16 100644 --- a/crates/uteke-core/src/lib.rs +++ b/crates/uteke-core/src/lib.rs @@ -19,6 +19,8 @@ pub mod dream; mod edges; mod embed; mod error; +#[cfg(test)] +mod error_class_tests; pub mod extraction; mod forget_request; pub mod graph; @@ -111,7 +113,7 @@ pub use timeline::{TimelineEvent, TimelineEventType}; pub use embed::Embedder; #[cfg(feature = "onnx")] pub use embed::OnnxEmbedder; -pub use error::{Error, format_bytes}; +pub use error::{Error, ErrorClass, format_bytes}; pub use types::{ CompactionReport, DoctorCheck, DoctorReport, DoctorStatus, ReembedReport, RepairReport, VerifyReport, @@ -1988,7 +1990,7 @@ impl Uteke { None => self .store .get_document(id_or_slug)? - .ok_or_else(|| Error::validation("document not found for move"))?, + .ok_or_else(|| Error::not_found("document not found for move"))?, }; let new_parent_id = match new_parent_slug { @@ -1997,7 +1999,7 @@ impl Uteke { let parent = match self.store.get_document_by_slug(ps)? { Some(d) => d, None => self.store.get_document(ps)?.ok_or_else(|| { - Error::validation(format!("parent document '{ps}' not found")) + Error::not_found(format!("parent document '{ps}' not found")) })?, }; Some(parent.id) diff --git a/crates/uteke-core/src/memory/rooms.rs b/crates/uteke-core/src/memory/rooms.rs index ab39a564..9382171d 100644 --- a/crates/uteke-core/src/memory/rooms.rs +++ b/crates/uteke-core/src/memory/rooms.rs @@ -946,7 +946,7 @@ impl super::Store { .execute("DELETE FROM rooms WHERE id = ?1", params![room_id]) .map_err(|e| Error::db("delete room", e))?; if rows == 0 { - return Err(Error::db_msg(format!("Room not found: {room_id}"))); + return Err(Error::not_found(format!("Room not found: {room_id}"))); } Ok(memory_links) } diff --git a/crates/uteke-server/src/context.rs b/crates/uteke-server/src/context.rs index f6ddfeaf..ee7e44d0 100644 --- a/crates/uteke-server/src/context.rs +++ b/crates/uteke-server/src/context.rs @@ -335,6 +335,27 @@ impl ReqCtx { ) } + /// Answer a core error by its class: 400 for a bad request, 404 when the + /// named thing does not exist, `secret_detected` in strict privacy mode, + /// 500 otherwise. The message of the error is the body in every case + /// except the secret one: this is what the handlers it replaced already + /// sent, so a 500 reveals exactly what it did before (database and + /// embedding messages are sanitized in `error.rs`; an `Io` message is not). + pub fn core_error(&self, req: &Request, e: &uteke_core::Error) -> Response>> { + use uteke_core::{Error, ErrorClass}; + if let Error::SecretDetected { kinds } = e { + return self.secret_detected_response(req, kinds); + } + match e.class() { + ErrorClass::BadRequest => self.error_response_for(req, 400, e.to_string()), + ErrorClass::NotFound => self.error_response_for(req, 404, e.to_string()), + _ => { + tracing::error!("Internal error: {e}"); + self.error_response_for(req, 500, e.to_string()) + } + } + } + /// The privacy filter rejected the write in strict mode (#1464): 400 with /// the kinds found, never the values. pub fn secret_detected_response( diff --git a/crates/uteke-server/src/handlers/docs.rs b/crates/uteke-server/src/handlers/docs.rs index 3490bd91..3a4b436d 100644 --- a/crates/uteke-server/src/handlers/docs.rs +++ b/crates/uteke-server/src/handlers/docs.rs @@ -155,13 +155,12 @@ pub(super) fn post_doc_move(c: &mut Call) -> Response>> { let new_sort_order = req_data.new_sort_order; match uteke.doc_move(id_or_slug, new_parent, new_sort_order) { Ok(moved) => ctx.ok_response_for(req, &serde_json::json!({"moved": moved})), + Err(e) if e.class() == uteke_core::ErrorClass::NotFound => { + ctx.error_response_for(req, 404, e.to_string()) + } Err(e) => { - if e.to_string().contains("not found") { - ctx.error_response_for(req, 404, e.to_string()) - } else { - error!("doc move error: {e}"); - ctx.error_response_for(req, 500, "Internal server error") - } + error!("doc move error: {e}"); + ctx.error_response_for(req, 500, "Internal server error") } } } diff --git a/crates/uteke-server/src/handlers/error_tests.rs b/crates/uteke-server/src/handlers/error_tests.rs new file mode 100644 index 00000000..674801a8 --- /dev/null +++ b/crates/uteke-server/src/handlers/error_tests.rs @@ -0,0 +1,101 @@ +//! The status and body each handler answers for each class of core error +//! (#1517). The statuses are the ones the API had before the classification. + +use super::*; +use serde_json::{Value, json}; +use tiny_http::TestRequest; + +struct App { + uteke: Mutex, +} + +impl App { + fn new() -> Self { + Self { + uteke: Mutex::new( + Uteke::open_with_backend(":memory:", None).expect("open without embedder"), + ), + } + } + + fn call(&self, method: Method, url: &str, body: Option) -> (u16, Value) { + let mut req: Request = match body { + Some(b) => { + let leaked: &'static str = Box::leak(b.to_string().into_boxed_str()); + TestRequest::new() + .with_method(method) + .with_path(url) + .with_body(leaked) + .into() + } + None => TestRequest::new().with_method(method).with_path(url).into(), + }; + let ctx = ReqCtx { + auth_token_hash: None, + read_only_token_hash: None, + cors_origins: Vec::new(), + recall_config: None, + extraction_config: None, + host_guard: Default::default(), + store_id: String::new(), + }; + let resp = route(&self.uteke, &ctx, &mut req); + let status = resp.status_code().0; + let text = String::from_utf8(resp.into_reader().into_inner()).unwrap(); + (status, serde_json::from_str(&text).unwrap_or(Value::Null)) + } +} + +#[test] +fn a_missing_room_and_a_missing_document_are_404_without_text_matching() { + let app = App::new(); + let (status, body) = app.call(Method::Delete, "/room/delete?room_id=ghost", None); + assert_eq!(status, 404, "{body}"); + assert_eq!(body["error"], "Room not found: ghost"); + + let (status, body) = app.call( + Method::Post, + "/doc/move", + Some(json!({"id": "no-such-doc", "new_parent": null})), + ); + assert_eq!(status, 404, "{body}"); + assert!( + body["error"].as_str().unwrap().contains("not found"), + "{body}" + ); +} + +#[test] +fn a_validation_error_is_400_through_the_shared_mapping() { + let app = App::new(); + let (_, ok) = app.call(Method::Post, "/remember", Some(json!({"content": "plain"}))); + let id = ok["id"].as_str().unwrap().to_string(); + + // Update with an invalid importance: Validation, 400 (update path). + let (status, body) = app.call( + Method::Put, + "/memory", + Some(json!({"id": id, "importance": 7.0})), + ); + assert_eq!(status, 400, "{body}"); + assert!( + body["error"].as_str().unwrap().contains("Importance"), + "{body}" + ); + + // A missing memory on update stays a 400 until the contract moves it. + let (status, _) = app.call( + Method::Put, + "/memory", + Some(json!({"id": "no-such-memory", "content": "x"})), + ); + assert_eq!(status, 400); + + // Consolidate pair with unknown ids: Validation, 400. + let (status, _) = app.call( + Method::Post, + "/consolidate/pair", + Some(json!({"id_keep": "nope-a", "id_remove": "nope-b"})), + ); + assert_eq!(status, 400); +} diff --git a/crates/uteke-server/src/handlers/maintenance.rs b/crates/uteke-server/src/handlers/maintenance.rs index 30e9e5e6..b9a92411 100644 --- a/crates/uteke-server/src/handlers/maintenance.rs +++ b/crates/uteke-server/src/handlers/maintenance.rs @@ -437,14 +437,7 @@ pub(super) fn post_consolidate_pair(c: &mut Call) -> Response>> { uteke.consolidate_pair(&req_data.id_keep, &req_data.id_remove, req_data.hard); match result { Ok(r) => ctx.ok_response_for(req, &r), - Err(e) => { - let status = if matches!(e, uteke_core::Error::Validation(_)) { - 400 - } else { - 500 - }; - ctx.error_response_for(req, status, e.to_string()) - } + Err(e) => ctx.core_error(req, &e), } } Err(e) => ctx.error_response_for(req, 400, e), diff --git a/crates/uteke-server/src/handlers/memory.rs b/crates/uteke-server/src/handlers/memory.rs index 9b3d1728..0d26fedc 100644 --- a/crates/uteke-server/src/handlers/memory.rs +++ b/crates/uteke-server/src/handlers/memory.rs @@ -811,11 +811,7 @@ pub(super) fn put_memory(c: &mut Call) -> Response>> { // update — plain column change, no re-embed needed. if let Some(ns) = req_data.namespace.as_deref() { if let Err(e) = uteke.move_memory(&req_data.id, ns) { - let status = match e { - uteke_core::Error::Validation(_) => 400, - _ => 500, - }; - return ctx.error_response_for(req, status, e.to_string()); + return ctx.core_error(req, &e); } } // #1464: true when the filter masked a secret in the new @@ -834,18 +830,7 @@ pub(super) fn put_memory(c: &mut Call) -> Response>> { Ok(_) => { ctx.error_response_for(req, 404, format!("Memory not found: {}", req_data.id)) } - Err(uteke_core::Error::SecretDetected { kinds }) => { - ctx.secret_detected_response(req, &kinds) - } - Err(e) => { - error!("Update memory error: {e}"); - // Propagate validation errors with proper status codes - let status = match e { - uteke_core::Error::Validation(_) => 400, - _ => 500, - }; - ctx.error_response_for(req, status, e.to_string()) - } + Err(e) => ctx.core_error(req, &e), } } Err(e) => ctx.error_response_for(req, 400, e), diff --git a/crates/uteke-server/src/handlers/mod.rs b/crates/uteke-server/src/handlers/mod.rs index 380ec399..40bf993c 100644 --- a/crates/uteke-server/src/handlers/mod.rs +++ b/crates/uteke-server/src/handlers/mod.rs @@ -27,6 +27,8 @@ const API_VERSIONS: &[&str] = &["v1", "v2"]; #[cfg(test)] mod author_tests; mod docs; +#[cfg(test)] +mod error_tests; mod graph; mod health; #[cfg(test)] diff --git a/crates/uteke-server/src/handlers/rooms.rs b/crates/uteke-server/src/handlers/rooms.rs index daf5506a..17b27038 100644 --- a/crates/uteke-server/src/handlers/rooms.rs +++ b/crates/uteke-server/src/handlers/rooms.rs @@ -321,13 +321,7 @@ pub(super) fn post_room_rename(c: &mut Call) -> Response>> { "room": room, }), ), - Err(e) => { - let status = match e { - uteke_core::Error::Validation(_) => 400, - _ => 500, - }; - ctx.error_response_for(req, status, e.to_string()) - } + Err(e) => ctx.core_error(req, &e), }, Err(e) => ctx.error_response_for(req, 400, e), } @@ -359,13 +353,7 @@ pub(super) fn post_room_update(c: &mut Call) -> Response>> { 404, format!("Room not found: {}", req_data.room_id), ), - Err(e) => { - let status = match e { - uteke_core::Error::Validation(_) => 400, - _ => 500, - }; - ctx.error_response_for(req, status, e.to_string()) - } + Err(e) => ctx.core_error(req, &e), } } Err(e) => ctx.error_response_for(req, 400, e), @@ -408,13 +396,7 @@ pub(super) fn post_room_memory_move(c: &mut Call) -> Response>> { ), ), Ok(_) => unreachable!("move returns 0 or 1"), - Err(e) => { - let status = match e { - uteke_core::Error::Validation(_) => 400, - _ => 500, - }; - ctx.error_response_for(req, status, e.to_string()) - } + Err(e) => ctx.core_error(req, &e), } } Err(e) => ctx.error_response_for(req, 400, e), @@ -467,17 +449,7 @@ pub(super) fn post_room_remember(c: &mut Call) -> Response>> { }), ) } - Err(uteke_core::Error::SecretDetected { kinds }) => { - ctx.secret_detected_response(req, &kinds) - } - Err(e) => { - error!("room/remember error: {e}"); - let status = match e { - uteke_core::Error::Validation(_) => 400, - _ => 500, - }; - ctx.error_response_for(req, status, e.to_string()) - } + Err(e) => ctx.core_error(req, &e), } } Err(e) => ctx.error_response_for(req, 400, e), @@ -655,14 +627,12 @@ pub(super) fn delete_room_delete(c: &mut Call) -> Response>> { "note": "memories and documents are preserved in their namespaces, no longer linked to any room" }), ), + Err(e) if e.class() == uteke_core::ErrorClass::NotFound => { + ctx.error_response_for(req, 404, e.to_string()) + } Err(e) => { - let msg = format!("{e}"); - if msg.contains("not found") { - ctx.error_response_for(req, 404, &msg) - } else { - error!("Internal error: {e}"); - ctx.error_response_for(req, 500, "Internal server error") - } + error!("Internal error: {e}"); + ctx.error_response_for(req, 500, "Internal server error") } } }