mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-08-28 05:27:41 +00:00
Move store error transience classification into fabro-store
The server's admission-error classifier hand-mapped all 17 fabro_store::Error variants to transient/permanent plus a snake_case label — store-internals knowledge that would drift on every variant change. Replace it with Error::is_transient() next to the enum and a strum IntoStaticStr derive for the kind labels, and move the structural test to fabro-store where the wrapped error types are natural deps. The server keeps only its own policy: the non-store-error fallback and the retry schedule. This also drops the slatedb dev-dependency that existed solely so the server test could construct a store-internal error. Two log-only kind labels introduced on this branch change with the derive: "serialization" is now "serde" and "internal" is now "other". Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
95d3216470
commit
ea9ecde746
5 changed files with 93 additions and 107 deletions
1
Cargo.lock
generated
1
Cargo.lock
generated
|
|
@ -3078,7 +3078,6 @@ dependencies = [
|
|||
"serde_json",
|
||||
"serde_yaml",
|
||||
"sha2 0.10.9",
|
||||
"slatedb",
|
||||
"sqlx",
|
||||
"strum 0.28.0",
|
||||
"sysinfo",
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
|
|
@ -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::<fabro_store::Error>() 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(),
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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::<serde_json::Value>("{").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",
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
pub type Result<T> = std::result::Result<T, Error>;
|
||||
|
||||
#[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::<serde_json::Value>("{").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);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue