diff --git a/Cargo.lock b/Cargo.lock index ee3bc2c61..260efaec2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3078,7 +3078,6 @@ dependencies = [ "serde_json", "serde_yaml", "sha2 0.10.9", - "slatedb", "sqlx", "strum 0.28.0", "sysinfo", diff --git a/lib/apps/fabro-server/Cargo.toml b/lib/apps/fabro-server/Cargo.toml index dd27ebb1a..8aea2bade 100644 --- a/lib/apps/fabro-server/Cargo.toml +++ b/lib/apps/fabro-server/Cargo.toml @@ -111,7 +111,6 @@ chrono = { workspace = true } [dev-dependencies] fabro-auth = { path = "../../foundation/fabro-auth", features = ["test-support"] } -slatedb.workspace = true tokio = { workspace = true, features = ["test-util", "macros"] } tower = "0.5" http-body-util = "0.1" diff --git a/lib/apps/fabro-server/src/server.rs b/lib/apps/fabro-server/src/server.rs index 0835077ca..eb7d2b1a4 100644 --- a/lib/apps/fabro-server/src/server.rs +++ b/lib/apps/fabro-server/src/server.rs @@ -4054,58 +4054,16 @@ struct AdmissionErrorClass { kind: &'static str, } -impl AdmissionErrorClass { - const fn transient(kind: &'static str) -> Self { - Self { - transient: true, - kind, - } - } - - const fn permanent(kind: &'static str) -> Self { - Self { - transient: false, - kind, - } - } -} - fn classify_admission_error(error: &anyhow::Error) -> AdmissionErrorClass { let Some(store_error) = error.downcast_ref::() else { - return AdmissionErrorClass::permanent("unrecognized"); + return AdmissionErrorClass { + transient: false, + kind: "unrecognized", + }; }; - match store_error { - fabro_store::Error::Slate(_) => AdmissionErrorClass::transient("slate"), - fabro_store::Error::ObjectStore(_) => AdmissionErrorClass::transient("object_store"), - fabro_store::Error::Sqlite(_) => AdmissionErrorClass::transient("sqlite"), - fabro_store::Error::Io(_) => AdmissionErrorClass::transient("io"), - fabro_store::Error::InvalidEvent(_) => AdmissionErrorClass::permanent("invalid_event"), - fabro_store::Error::EventRejected { .. } => { - AdmissionErrorClass::permanent("event_rejected") - } - fabro_store::Error::RunNotFound(_) => AdmissionErrorClass::permanent("run_not_found"), - fabro_store::Error::SessionNotFound(_) => { - AdmissionErrorClass::permanent("session_not_found") - } - fabro_store::Error::SessionAlreadyExists(_) => { - AdmissionErrorClass::permanent("session_already_exists") - } - fabro_store::Error::ReadOnly => AdmissionErrorClass::permanent("read_only"), - fabro_store::Error::EventSequenceExhausted { .. } => { - AdmissionErrorClass::permanent("event_sequence_exhausted") - } - fabro_store::Error::InvalidKeySegment { .. } => { - AdmissionErrorClass::permanent("invalid_key_segment") - } - fabro_store::Error::KeyParse(_) => AdmissionErrorClass::permanent("key_parse"), - fabro_store::Error::RunSummaryMismatch { .. } => { - AdmissionErrorClass::permanent("run_summary_mismatch") - } - fabro_store::Error::InvalidTransition(_) => { - AdmissionErrorClass::permanent("invalid_transition") - } - fabro_store::Error::Serde(_) => AdmissionErrorClass::permanent("serialization"), - fabro_store::Error::Other(_) => AdmissionErrorClass::permanent("internal"), + AdmissionErrorClass { + transient: store_error.is_transient(), + kind: store_error.into(), } } diff --git a/lib/apps/fabro-server/src/server/tests.rs b/lib/apps/fabro-server/src/server/tests.rs index 50aa2acf0..01f604b46 100644 --- a/lib/apps/fabro-server/src/server/tests.rs +++ b/lib/apps/fabro-server/src/server/tests.rs @@ -16467,64 +16467,32 @@ async fn admission_predicate_false_reconciles_durable_cancellation() { assert_eq!(run_store.list_events().await.unwrap().len(), event_count); } +// The per-variant transience and kind labels are covered where they live, in +// fabro-store's error tests; this only covers the downcast plumbing. #[test] -fn admission_error_classifier_is_structural() { - let transient = [ - ( - fabro_store::Error::Slate(slatedb::Error::unavailable("test outage".to_string())), - "slate", - ), - ( - fabro_store::Error::ObjectStore(object_store::Error::Generic { - store: "test", - source: Box::new(std::io::Error::other("test outage")), - }), - "object_store", - ), - ( - fabro_store::Error::Sqlite(sqlx::Error::RowNotFound), - "sqlite", - ), - ( - fabro_store::Error::Io(std::io::Error::other("test outage")), - "io", - ), - ]; - for (error, kind) in transient { - assert_eq!( - classify_admission_error(&anyhow::Error::new(error)), - AdmissionErrorClass::transient(kind), - ); - } - - let serde_error = serde_json::from_str::("{").unwrap_err(); - let permanent = [ - ( - fabro_store::Error::EventRejected { - source: Box::new(fabro_store::Error::ReadOnly), - }, - "event_rejected", - ), - ( - fabro_store::Error::EventSequenceExhausted { max_seq: 10 }, - "event_sequence_exhausted", - ), - ( - fabro_store::Error::RunNotFound("missing".to_string()), - "run_not_found", - ), - (fabro_store::Error::ReadOnly, "read_only"), - (fabro_store::Error::Serde(serde_error), "serialization"), - ]; - for (error, kind) in permanent { - assert_eq!( - classify_admission_error(&anyhow::Error::new(error)), - AdmissionErrorClass::permanent(kind), - ); - } +fn admission_error_classification_follows_store_error() { + assert_eq!( + classify_admission_error(&anyhow::Error::new(fabro_store::Error::Io( + std::io::Error::other("test outage") + ))), + AdmissionErrorClass { + transient: true, + kind: "io", + }, + ); + assert_eq!( + classify_admission_error(&anyhow::Error::new(fabro_store::Error::ReadOnly)), + AdmissionErrorClass { + transient: false, + kind: "read_only", + }, + ); assert_eq!( classify_admission_error(&anyhow::anyhow!("unrecognized")), - AdmissionErrorClass::permanent("unrecognized"), + AdmissionErrorClass { + transient: false, + kind: "unrecognized", + }, ); } diff --git a/lib/components/fabro-store/src/error.rs b/lib/components/fabro-store/src/error.rs index 3b6c93f5d..4d94d53e4 100644 --- a/lib/components/fabro-store/src/error.rs +++ b/lib/components/fabro-store/src/error.rs @@ -1,6 +1,7 @@ pub type Result = std::result::Result; -#[derive(Debug, thiserror::Error)] +#[derive(Debug, thiserror::Error, strum::IntoStaticStr)] +#[strum(serialize_all = "snake_case")] pub enum Error { #[error("SlateDB error: {0}")] Slate(#[from] slatedb::Error), @@ -43,3 +44,64 @@ pub enum Error { #[error("{0}")] Other(String), } + +impl Error { + /// True for infrastructure failures (storage engine, object store, + /// SQLite, I/O) where retrying the operation may succeed; false for + /// structural errors that fail the same way every time. + pub fn is_transient(&self) -> bool { + matches!( + self, + Self::Slate(_) | Self::ObjectStore(_) | Self::Sqlite(_) | Self::Io(_) + ) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn transience_and_kind_labels_are_structural() { + let transient = [ + ( + Error::Slate(slatedb::Error::unavailable("test outage".to_string())), + "slate", + ), + ( + Error::ObjectStore(object_store::Error::Generic { + store: "test", + source: Box::new(std::io::Error::other("test outage")), + }), + "object_store", + ), + (Error::Sqlite(sqlx::Error::RowNotFound), "sqlite"), + (Error::Io(std::io::Error::other("test outage")), "io"), + ]; + for (error, kind) in transient { + assert!(error.is_transient(), "{kind} should be transient"); + assert_eq!(<&'static str>::from(&error), kind); + } + + let serde_error = serde_json::from_str::("{").unwrap_err(); + let permanent = [ + ( + Error::EventRejected { + source: Box::new(Error::ReadOnly), + }, + "event_rejected", + ), + ( + Error::EventSequenceExhausted { max_seq: 10 }, + "event_sequence_exhausted", + ), + (Error::RunNotFound("missing".to_string()), "run_not_found"), + (Error::ReadOnly, "read_only"), + (Error::Serde(serde_error), "serde"), + ]; + for (error, kind) in permanent { + assert!(!error.is_transient(), "{kind} should be permanent"); + assert_eq!(<&'static str>::from(&error), kind); + } + } +}