Skip to content

Commit

Permalink
Add context for errors potentially caused by non self describing types
Browse files Browse the repository at this point in the history
  • Loading branch information
chmp committed Sep 30, 2024
1 parent 1132254 commit 55cb727
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 5 deletions.
12 changes: 9 additions & 3 deletions serde_arrow/src/internal/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,15 +135,21 @@ impl Error {
Self::Custom(err) => Some(&err.0.annotations),
}
}

pub(crate) fn modify_message<F: FnOnce(&mut String)>(&mut self, func: F) {
let Error::Custom(this) = self;
let inner = this.0.as_mut();
func(&mut inner.message);
}
}

#[derive(PartialEq)]
pub struct CustomError(pub(crate) Box<CustomErrorImpl>);

pub struct CustomErrorImpl {
message: String,
backtrace: Backtrace,
cause: Option<Box<dyn std::error::Error + Send + Sync + 'static>>,
pub(crate) message: String,
pub(crate) backtrace: Backtrace,
pub(crate) cause: Option<Box<dyn std::error::Error + Send + Sync + 'static>>,
pub(crate) annotations: BTreeMap<String, String>,
}

Expand Down
32 changes: 30 additions & 2 deletions serde_arrow/src/internal/schema/from_type/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,30 @@ impl Tracer {
budget = tracer.get_options().from_type_budget,
);
}
T::deserialize(TraceAny(&mut tracer))?;
let res = T::deserialize(TraceAny(&mut tracer));
if let Err(err) = res {
if !is_non_self_describing_error(err.message()) {
return Err(err);
}
let mut err = err;
err.modify_message(|message| {
*message = format!(
concat!(
"{message}{maybe_period} ",
"It seems that `from_type` encountered a non self describing type. ",
"Consider using `from_samples` instead. ",
),
message = message,
maybe_period = if message.trim_end().ends_with('.') {
""
} else {
"."
},
);
});
return Err(err);
}

budget -= 1;
}

Expand All @@ -44,6 +67,11 @@ impl Tracer {
}
}

// check for known error messages of non self describing types
fn is_non_self_describing_error(s: &str) -> bool {
s.contains("premature end of input")
}

struct TraceAny<'a>(&'a mut Tracer);

impl<'a> Context for TraceAny<'a> {
Expand All @@ -61,7 +89,7 @@ impl<'de, 'a> serde::de::Deserializer<'de> for TraceAny<'a> {
concat!(
"Non self describing types cannot be traced with `from_type`. ",
"Consider using `from_samples`. ",
"One example is `serde_json::Value`. ",
"One example is `serde_json::Value`: ",
"the schema depends on the JSON content and cannot be determined from the type alone."
));
}
Expand Down
8 changes: 8 additions & 0 deletions serde_arrow/src/test/error_messages/trace_from_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ use crate::{
fn example() {
// NOTE: Value cannot be traced with from_type, as it is not self-describing
let res = SerdeArrowSchema::from_type::<Item<Vec<Value>>>(TracingOptions::default());
assert_error_contains(
&res,
"Non self describing types cannot be traced with `from_type`.",
);
assert_error_contains(&res, "path: \"$.item.element\"");
assert_error_contains(&res, "tracer_type: \"Unknown\"");
}
Expand All @@ -23,15 +27,19 @@ fn example() {
fn chrono_types_are_not_self_describing() {
let res = SerdeArrowSchema::from_type::<Item<DateTime<Utc>>>(TracingOptions::default());
assert_error_contains(&res, "path: \"$.item\"");
assert_error_contains(&res, "non self describing type");

let res = SerdeArrowSchema::from_type::<Item<NaiveDateTime>>(TracingOptions::default());
assert_error_contains(&res, "path: \"$.item\"");
assert_error_contains(&res, "non self describing type");

let res = SerdeArrowSchema::from_type::<Item<NaiveTime>>(TracingOptions::default());
assert_error_contains(&res, "path: \"$.item\"");
assert_error_contains(&res, "non self describing type");

let res = SerdeArrowSchema::from_type::<Item<NaiveDate>>(TracingOptions::default());
assert_error_contains(&res, "path: \"$.item\"");
assert_error_contains(&res, "non self describing type");
}

#[test]
Expand Down

0 comments on commit 55cb727

Please sign in to comment.