From df074df831ef926324c1b68f6a148af909cb8a62 Mon Sep 17 00:00:00 2001 From: Quinn Parrott Date: Thu, 25 Sep 2025 09:59:10 -0400 Subject: [PATCH 1/4] Added path to errors --- src/de/deserializer.rs | 60 +++++++++++++++--------------- src/de/deserializer_enum.rs | 42 ++++++++++++--------- src/de/deserializer_map.rs | 47 +++++++++++++----------- src/de/deserializer_number.rs | 19 ++++++---- src/de/deserializer_seq.rs | 39 ++++++++++++-------- src/de/mod.rs | 5 ++- src/error.rs | 69 +++++++++++++++++++++++++++++++++-- src/tests.rs | 8 ++-- 8 files changed, 187 insertions(+), 102 deletions(-) diff --git a/src/de/deserializer.rs b/src/de/deserializer.rs index ef0175d..a5aaa17 100644 --- a/src/de/deserializer.rs +++ b/src/de/deserializer.rs @@ -6,35 +6,40 @@ use super::{ deserializer_seq::{ DeserializerSeq, DeserializerSeqBytes, DeserializerSeqNumbers, DeserializerSeqStrings, }, - AttributeValue, Error, ErrorImpl, Result, + AttributeValue, Error, ErrorImpl, ErrorPath, Result, }; use serde_core::de::{self, IntoDeserializer, Visitor}; /// A structure that deserializes [`AttributeValue`]s into Rust values. #[derive(Debug)] -pub struct Deserializer { +pub struct Deserializer<'a> { input: AttributeValue, + path: ErrorPath<'a>, } -impl Deserializer { +impl<'a> Deserializer<'a> { /// Create a Deserializer from an AttributeValue pub fn from_attribute_value(input: AttributeValue) -> Self { - Deserializer { input } + Self::from_attribute_value_path(input, ErrorPath::Root) + } + + pub(crate) fn from_attribute_value_path(input: AttributeValue, path: ErrorPath<'a>) -> Self { + Deserializer { input, path } } } macro_rules! deserialize_number { ($self:expr, $visitor:expr, $ty:ty, $fn:ident) => { if let AttributeValue::N(n) = $self.input { - let de = DeserializerNumber::from_string(n); + let de = DeserializerNumber::from_string(n, $self.path); de.$fn($visitor) } else { - return Err(ErrorImpl::ExpectedNum.into()); + return Err(Error::from_path(ErrorImpl::ExpectedNum, &$self.path)); } }; } -impl<'de> de::Deserializer<'de> for Deserializer { +impl<'de, 'a> de::Deserializer<'de> for Deserializer<'a> { type Error = Error; // Look at the input data to decide what Serde data model type to @@ -45,7 +50,7 @@ impl<'de> de::Deserializer<'de> for Deserializer { V: Visitor<'de>, { if let AttributeValue::N(s) = self.input { - DeserializerNumber::from_string(s).deserialize_any(visitor) + DeserializerNumber::from_string(s, self.path).deserialize_any(visitor) } else { match self.input { AttributeValue::S(_) => self.deserialize_string(visitor), @@ -139,7 +144,7 @@ impl<'de> de::Deserializer<'de> for Deserializer { if let AttributeValue::S(s) = self.input { visitor.visit_string(s) } else { - Err(ErrorImpl::ExpectedString.into()) + Err(Error::from_path(ErrorImpl::ExpectedString, &self.path)) } } @@ -150,7 +155,7 @@ impl<'de> de::Deserializer<'de> for Deserializer { if let AttributeValue::S(s) = self.input { visitor.visit_string(s) } else { - Err(ErrorImpl::ExpectedString.into()) + Err(Error::from_path(ErrorImpl::ExpectedString, &self.path)) } } @@ -160,7 +165,7 @@ impl<'de> de::Deserializer<'de> for Deserializer { { match self.input { AttributeValue::L(l) => { - let deserializer_seq = DeserializerSeq::from_vec(l); + let deserializer_seq = DeserializerSeq::from_vec(l, self.path); visitor.visit_seq(deserializer_seq) } AttributeValue::Ss(ss) => { @@ -168,14 +173,14 @@ impl<'de> de::Deserializer<'de> for Deserializer { visitor.visit_seq(deserializer_seq) } AttributeValue::Ns(ns) => { - let deserializer_seq = DeserializerSeqNumbers::from_vec(ns); + let deserializer_seq = DeserializerSeqNumbers::from_vec(ns, self.path); visitor.visit_seq(deserializer_seq) } AttributeValue::Bs(bs) => { let deserializer_seq = DeserializerSeqBytes::from_vec(bs); visitor.visit_seq(deserializer_seq) } - _ => Err(ErrorImpl::ExpectedSeq.into()), + _ => Err(Error::from_path(ErrorImpl::ExpectedSeq, &self.path)), } } @@ -184,10 +189,10 @@ impl<'de> de::Deserializer<'de> for Deserializer { V: Visitor<'de>, { if let AttributeValue::M(mut m) = self.input { - let deserializer_map = DeserializerMap::from_item(&mut m); + let deserializer_map = DeserializerMap::from_item(&mut m, self.path); visitor.visit_map(deserializer_map) } else { - Err(ErrorImpl::ExpectedMap.into()) + Err(Error::from_path(ErrorImpl::ExpectedMap, &self.path)) } } @@ -198,7 +203,7 @@ impl<'de> de::Deserializer<'de> for Deserializer { if let AttributeValue::Bool(b) = self.input { visitor.visit_bool(b) } else { - Err(ErrorImpl::ExpectedBool.into()) + Err(Error::from_path(ErrorImpl::ExpectedBool, &self.path)) } } @@ -210,17 +215,12 @@ impl<'de> de::Deserializer<'de> for Deserializer { let mut chars = s.chars(); if let Some(ch) = chars.next() { let result = visitor.visit_char(ch)?; - if chars.next().is_some() { - Err(ErrorImpl::ExpectedChar.into()) - } else { - Ok(result) + if chars.next().is_none() { + return Ok(result); } - } else { - Err(ErrorImpl::ExpectedChar.into()) } - } else { - Err(ErrorImpl::ExpectedChar.into()) } + Err(Error::from_path(ErrorImpl::ExpectedChar, &self.path)) } fn deserialize_unit(self, visitor: V) -> Result @@ -230,7 +230,7 @@ impl<'de> de::Deserializer<'de> for Deserializer { if let AttributeValue::Null(true) = self.input { visitor.visit_unit() } else { - Err(ErrorImpl::ExpectedUnit.into()) + Err(Error::from_path(ErrorImpl::ExpectedUnit, &self.path)) } } @@ -245,8 +245,8 @@ impl<'de> de::Deserializer<'de> for Deserializer { { match self.input { AttributeValue::S(s) => visitor.visit_enum(s.into_deserializer()), - AttributeValue::M(m) => visitor.visit_enum(DeserializerEnum::from_item(m)), - _ => Err(ErrorImpl::ExpectedEnum.into()), + AttributeValue::M(m) => visitor.visit_enum(DeserializerEnum::from_item(m, self.path)), + _ => Err(Error::from_path(ErrorImpl::ExpectedEnum, &self.path)), } } @@ -258,7 +258,7 @@ impl<'de> de::Deserializer<'de> for Deserializer { let de = DeserializerBytes::from_bytes(b); de.deserialize_bytes(visitor) } else { - Err(ErrorImpl::ExpectedBytes.into()) + Err(Error::from_path(ErrorImpl::ExpectedBytes, &self.path)) } } @@ -310,7 +310,7 @@ impl<'de> de::Deserializer<'de> for Deserializer { if let AttributeValue::S(s) = self.input { visitor.visit_string(s) } else { - Err(ErrorImpl::ExpectedString.into()) + Err(Error::from_path(ErrorImpl::ExpectedString, &self.path)) } } @@ -325,7 +325,7 @@ impl<'de> de::Deserializer<'de> for Deserializer { if let AttributeValue::Null(true) = self.input { visitor.visit_unit() } else { - Err(ErrorImpl::ExpectedUnitStruct.into()) + Err(Error::from_path(ErrorImpl::ExpectedUnitStruct, &self.path)) } } diff --git a/src/de/deserializer_enum.rs b/src/de/deserializer_enum.rs index 1f7b8d5..f861b0b 100644 --- a/src/de/deserializer_enum.rs +++ b/src/de/deserializer_enum.rs @@ -1,21 +1,22 @@ -use super::{AttributeValue, Deserializer, Error, ErrorImpl, Result}; +use super::{AttributeValue, Deserializer, Error, ErrorImpl, ErrorPath, Result}; use serde_core::de::{ DeserializeSeed, Deserializer as _, EnumAccess, IntoDeserializer, VariantAccess, Visitor, }; use std::collections::HashMap; -pub struct DeserializerEnum { +pub struct DeserializerEnum<'a> { input: HashMap, + path: ErrorPath<'a>, } -impl DeserializerEnum { - pub fn from_item(input: HashMap) -> Self { - Self { input } +impl<'a> DeserializerEnum<'a> { + pub fn from_item(input: HashMap, path: ErrorPath<'a>) -> Self { + Self { input, path } } } -impl<'de> EnumAccess<'de> for DeserializerEnum { - type Variant = DeserializerVariant; +impl<'de, 'a> EnumAccess<'de> for DeserializerEnum<'a> { + type Variant = DeserializerVariant<'a>; type Error = Error; fn variant_seed(mut self, seed: V) -> Result<(V::Value, Self::Variant), Self::Error> @@ -25,27 +26,32 @@ impl<'de> EnumAccess<'de> for DeserializerEnum { let mut drain = self.input.drain(); let (key, value) = drain .next() - .ok_or_else(|| ErrorImpl::ExpectedSingleKey.into())?; + .ok_or_else(|| Error::from_path(ErrorImpl::ExpectedSingleKey, &self.path))?; if drain.next().is_some() { - return Err(ErrorImpl::ExpectedSingleKey.into()); + return Err(Error::from_path(ErrorImpl::ExpectedSingleKey, &self.path)); } - let deserializer = DeserializerVariant::from_attribute_value(value); + let deserializer = DeserializerVariant::from_attribute_value( + value, + ErrorPath::Enum(key.clone(), Box::new(self.path)), + ); let value = seed.deserialize(key.into_deserializer())?; + Ok((value, deserializer)) } } -pub struct DeserializerVariant { +pub struct DeserializerVariant<'a> { input: AttributeValue, + path: ErrorPath<'a>, } -impl DeserializerVariant { - pub fn from_attribute_value(input: AttributeValue) -> Self { - Self { input } +impl<'a> DeserializerVariant<'a> { + pub fn from_attribute_value(input: AttributeValue, path: ErrorPath<'a>) -> Self { + Self { input, path } } } -impl<'de> VariantAccess<'de> for DeserializerVariant { +impl<'de, 'a> VariantAccess<'de> for DeserializerVariant<'a> { type Error = Error; fn unit_variant(self) -> Result<()> { @@ -56,7 +62,7 @@ impl<'de> VariantAccess<'de> for DeserializerVariant { where S: DeserializeSeed<'de>, { - let deserializer = Deserializer::from_attribute_value(self.input); + let deserializer = Deserializer::from_attribute_value_path(self.input, self.path); seed.deserialize(deserializer) } @@ -64,7 +70,7 @@ impl<'de> VariantAccess<'de> for DeserializerVariant { where V: Visitor<'de>, { - let deserializer = Deserializer::from_attribute_value(self.input); + let deserializer = Deserializer::from_attribute_value_path(self.input, self.path); deserializer.deserialize_seq(visitor) } @@ -72,7 +78,7 @@ impl<'de> VariantAccess<'de> for DeserializerVariant { where V: Visitor<'de>, { - let deserializer = Deserializer::from_attribute_value(self.input); + let deserializer = Deserializer::from_attribute_value_path(self.input, self.path); deserializer.deserialize_map(visitor) } } diff --git a/src/de/deserializer_map.rs b/src/de/deserializer_map.rs index 6331d41..2237e32 100644 --- a/src/de/deserializer_map.rs +++ b/src/de/deserializer_map.rs @@ -1,4 +1,4 @@ -use super::{AttributeValue, Deserializer, Error, ErrorImpl, Result}; +use super::{AttributeValue, Deserializer, Error, ErrorImpl, ErrorPath, Result}; use serde_core::{ de::{self, DeserializeSeed, MapAccess, Visitor}, forward_to_deserialize_any, @@ -7,14 +7,16 @@ use std::collections::HashMap; pub struct DeserializerMap<'a> { drain: std::collections::hash_map::Drain<'a, String, AttributeValue>, - remaining_value: Option, + remaining_value: Option<(String, AttributeValue)>, + path: ErrorPath<'a>, } impl<'a> DeserializerMap<'a> { - pub fn from_item(item: &'a mut HashMap) -> Self { + pub fn from_item(item: &'a mut HashMap, path: ErrorPath<'a>) -> Self { Self { drain: item.drain(), remaining_value: None, + path, } } } @@ -27,9 +29,10 @@ impl<'de, 'a> MapAccess<'de> for DeserializerMap<'a> { K: DeserializeSeed<'de>, { if let Some((key, value)) = self.drain.next() { - self.remaining_value = Some(value); - let de = DeserializerMapKey::from_string(key); - seed.deserialize(de).map(Some) + let de = DeserializerMapKey::from_string(&key, ErrorPath::Field(&key, &self.path)); + let a = seed.deserialize(de).map(Some); + self.remaining_value = Some((key, value)); + a } else { Ok(None) } @@ -39,8 +42,8 @@ impl<'de, 'a> MapAccess<'de> for DeserializerMap<'a> { where V: DeserializeSeed<'de>, { - if let Some(value) = self.remaining_value.take() { - let de = Deserializer::from_attribute_value(value); + if let Some((key, value)) = self.remaining_value.take() { + let de = Deserializer::from_attribute_value_path(value, ErrorPath::Field(&key, &self.path)); seed.deserialize(de) } else { unreachable!("Value without a corresponding key") @@ -52,13 +55,14 @@ impl<'de, 'a> MapAccess<'de> for DeserializerMap<'a> { } } -struct DeserializerMapKey { - input: String, +struct DeserializerMapKey<'a> { + input: &'a str, + path: ErrorPath<'a>, } -impl DeserializerMapKey { - fn from_string(input: String) -> Self { - Self { input } +impl<'a> DeserializerMapKey<'a> { + fn from_string(input: &'a str, path: ErrorPath<'a>) -> Self { + Self { input, path } } } @@ -71,14 +75,14 @@ macro_rules! deserialize_integer_key { let number = self .input .parse() - .map_err(|_| ErrorImpl::ExpectedNum.into())?; + .map_err(|_| Error::from_path(ErrorImpl::ExpectedNum, &self.path))?; visitor.$visit(number) } }; } -impl<'de> de::Deserializer<'de> for DeserializerMapKey { +impl<'de, 'a> de::Deserializer<'de> for DeserializerMapKey<'a> { type Error = Error; // Look at the input data to decide what Serde data model type to @@ -95,21 +99,21 @@ impl<'de> de::Deserializer<'de> for DeserializerMapKey { where V: Visitor<'de>, { - visitor.visit_string(self.input) + visitor.visit_str(self.input) } fn deserialize_string(self, visitor: V) -> Result where V: Visitor<'de>, { - visitor.visit_string(self.input) + visitor.visit_str(self.input) } fn deserialize_identifier(self, visitor: V) -> Result where V: Visitor<'de>, { - visitor.visit_string(self.input) + visitor.visit_str(self.input) } fn deserialize_enum( @@ -121,7 +125,8 @@ impl<'de> de::Deserializer<'de> for DeserializerMapKey { where V: de::Visitor<'de>, { - let de = Deserializer::from_attribute_value(AttributeValue::S(self.input)); + let de = + Deserializer::from_attribute_value_path(AttributeValue::S(self.input.to_owned()), self.path); de.deserialize_enum(name, variants, visitor) } @@ -151,10 +156,10 @@ impl<'de> de::Deserializer<'de> for DeserializerMapKey { where V: Visitor<'de>, { - match self.input.as_str() { + match self.input { "true" => visitor.visit_bool(true), "false" => visitor.visit_bool(false), - _ => Err(ErrorImpl::ExpectedString.into()), + _ => Err(Error::from_path(ErrorImpl::ExpectedString, &self.path)), } } diff --git a/src/de/deserializer_number.rs b/src/de/deserializer_number.rs index bf7f02f..5d7bf46 100644 --- a/src/de/deserializer_number.rs +++ b/src/de/deserializer_number.rs @@ -1,14 +1,17 @@ +use crate::de::ErrorPath; + use super::{Error, ErrorImpl, Result}; use serde_core::de::{self, Visitor}; use serde_core::forward_to_deserialize_any; -pub struct DeserializerNumber { +pub struct DeserializerNumber<'a> { input: String, + path: ErrorPath<'a>, } -impl DeserializerNumber { - pub fn from_string(input: String) -> Self { - DeserializerNumber { input } +impl<'a> DeserializerNumber<'a> { + pub fn from_string(input: String, path: ErrorPath<'a>) -> Self { + DeserializerNumber { input, path } } fn deserialize_number<'de, V>(self, visitor: V) -> Result @@ -22,7 +25,7 @@ impl DeserializerNumber { (Ok(i), _, _) => visitor.visit_i64(i), (_, Ok(u), _) => visitor.visit_u64(u), (_, _, Ok(f)) => visitor.visit_f64(f), - (Err(_), Err(_), Err(e)) => Err(ErrorImpl::FailedToParseFloat(self.input, e).into()), + (Err(_), Err(_), Err(e)) => Err(Error::from_path(ErrorImpl::FailedToParseFloat(self.input, e), &self.path)), } } } @@ -32,7 +35,7 @@ macro_rules! deserialize_int { let n = $self .input .parse::<$ty>() - .map_err(|e| ErrorImpl::FailedToParseInt($self.input, e).into())?; + .map_err(|e| Error::from_path(ErrorImpl::FailedToParseInt($self.input, e), &$self.path))?; $visitor.$fn(n) }}; } @@ -42,12 +45,12 @@ macro_rules! deserialize_float { let n = $self .input .parse::<$ty>() - .map_err(|e| ErrorImpl::FailedToParseFloat($self.input, e).into())?; + .map_err(|e| Error::from_path(ErrorImpl::FailedToParseFloat($self.input, e), &$self.path))?; $visitor.$fn(n) }}; } -impl<'de> de::Deserializer<'de> for DeserializerNumber { +impl<'de, 'a> de::Deserializer<'de> for DeserializerNumber<'a> { type Error = Error; // Look at the input data to decide what Serde data model type to diff --git a/src/de/deserializer_seq.rs b/src/de/deserializer_seq.rs index 224a776..dc3af08 100644 --- a/src/de/deserializer_seq.rs +++ b/src/de/deserializer_seq.rs @@ -1,29 +1,33 @@ +use crate::de::ErrorPath; + use super::deserializer_bytes::DeserializerBytes; use super::deserializer_number::DeserializerNumber; use super::{AttributeValue, Deserializer, Error, Result}; use serde_core::de::{DeserializeSeed, IntoDeserializer, SeqAccess}; -pub struct DeserializerSeq { - iter: std::vec::IntoIter, +pub struct DeserializerSeq<'a> { + iter: std::iter::Enumerate>, + path: ErrorPath<'a>, } -impl DeserializerSeq { - pub fn from_vec(vec: Vec) -> Self { +impl<'a> DeserializerSeq<'a> { + pub fn from_vec(vec: Vec, path: ErrorPath<'a>) -> Self { Self { - iter: vec.into_iter(), + iter: vec.into_iter().enumerate(), + path, } } } -impl<'de> SeqAccess<'de> for DeserializerSeq { +impl<'de, 'a> SeqAccess<'de> for DeserializerSeq<'a> { type Error = Error; fn next_element_seed(&mut self, seed: S) -> Result, Self::Error> where S: DeserializeSeed<'de>, { - if let Some(value) = self.iter.next() { - let de = Deserializer::from_attribute_value(value); + if let Some((i, value)) = self.iter.next() { + let de = Deserializer::from_attribute_value_path(value, ErrorPath::Elem(i, &self.path)); seed.deserialize(de).map(Some) } else { Ok(None) @@ -56,6 +60,7 @@ impl<'de> SeqAccess<'de> for DeserializerSeqStrings { { if let Some(value) = self.iter.next() { let de = value.into_deserializer(); + // TODO: Add path seed.deserialize(de).map(Some) } else { Ok(None) @@ -63,27 +68,29 @@ impl<'de> SeqAccess<'de> for DeserializerSeqStrings { } } -pub struct DeserializerSeqNumbers { - iter: std::vec::IntoIter, +pub struct DeserializerSeqNumbers<'a> { + iter: std::iter::Enumerate>, + path: ErrorPath<'a>, } -impl DeserializerSeqNumbers { - pub fn from_vec(vec: Vec) -> Self { +impl<'a> DeserializerSeqNumbers<'a> { + pub fn from_vec(vec: Vec, path: ErrorPath<'a>) -> Self { Self { - iter: vec.into_iter(), + iter: vec.into_iter().enumerate(), + path, } } } -impl<'de> SeqAccess<'de> for DeserializerSeqNumbers { +impl<'de, 'a> SeqAccess<'de> for DeserializerSeqNumbers<'a> { type Error = Error; fn next_element_seed(&mut self, seed: T) -> Result, Self::Error> where T: DeserializeSeed<'de>, { - if let Some(value) = self.iter.next() { - let de = DeserializerNumber::from_string(value); + if let Some((i, value)) = self.iter.next() { + let de = DeserializerNumber::from_string(value, ErrorPath::Elem(i, &self.path)); seed.deserialize(de).map(Some) } else { Ok(None) diff --git a/src/de/mod.rs b/src/de/mod.rs index 8ffc29d..eb441f0 100644 --- a/src/de/mod.rs +++ b/src/de/mod.rs @@ -1,5 +1,8 @@ use super::AttributeValue; -use crate::{error::ErrorImpl, Error, Item, Items, Result}; +use crate::{ + error::{ErrorImpl, ErrorPath}, + Error, Item, Items, Result, +}; use serde_core::Deserialize; use std::collections::HashMap; diff --git a/src/error.rs b/src/error.rs index 88b821c..2fc9cd7 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,14 +1,50 @@ use serde_core::{de, ser}; -use std::fmt::{self, Display}; +use std::fmt::{self, Display, Write}; /// This type represents all possible errors that can occur when serializing or deserializing /// DynamoDB data. #[derive(Debug, Clone, PartialEq, Eq)] -pub struct Error(ErrorImpl); +pub struct Error(Box<(ErrorImpl, String)>); + +impl Error { + /// Build a new error + pub fn new(error: ErrorImpl, path: String) -> Self { + Self(Box::new((error, path))) + } + + pub(crate) fn from_path(error: ErrorImpl, path: &ErrorPath<'_>) -> Self { + let mut path_str = String::new(); + path.visit_path_depth_first(&mut |path| { + match path { + ErrorPath::Root => {} + ErrorPath::Field(string, _) => { + path_str.push_str(string); + } + ErrorPath::Elem(i, _) => { + write!(&mut path_str, "[{i}]").unwrap(); + } + ErrorPath::Enum(string, _) => { + path_str.push_str(string); + } + } + path_str.push('.'); + }); + + // Remove trailing '.' + path_str.pop(); + + Self::new(error, path_str) + } +} impl Display for Error { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.0.fmt(f) + let (error_kind, path) = &*self.0; + error_kind.fmt(f)?; + if !path.is_empty() { + write!(f, " at '{path}'")?; + } + Ok(()) } } @@ -79,7 +115,7 @@ pub enum ErrorImpl { #[allow(clippy::from_over_into)] impl Into for ErrorImpl { fn into(self) -> Error { - Error(self) + Error(Box::new((self, String::new()))) } } @@ -142,3 +178,28 @@ impl de::Error for ErrorImpl { /// Alias for a `Result` with the error type `serde_dynamo::Error` pub type Result = std::result::Result; + +/// Used to construct error paths while minimizing allocations when there are no errors. +#[derive(Debug, Clone)] +pub(crate) enum ErrorPath<'a> { + Root, + Field(&'a str, &'a ErrorPath<'a>), + Elem(usize, &'a ErrorPath<'a>), + Enum(String, Box>), +} + +impl<'a> ErrorPath<'a> { + pub(crate) fn visit_path_depth_first(&self, fun: &mut impl FnMut(&ErrorPath<'a>)) { + match self { + ErrorPath::Root => {} + ErrorPath::Field(_, path) | ErrorPath::Elem(_, path) => { + path.visit_path_depth_first(fun); + fun(self); + } + ErrorPath::Enum(_, path) => { + path.visit_path_depth_first(fun); + fun(self); + } + } + } +} diff --git a/src/tests.rs b/src/tests.rs index f16e9f2..e709585 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -103,7 +103,7 @@ fn error_eq() { #[cfg(test)] mod from_items { - use crate::{error::ErrorImpl, from_items, to_attribute_value, AttributeValue, Error, Items}; + use crate::{from_items, to_attribute_value, AttributeValue, Items}; use serde_derive::{Deserialize, Serialize}; use std::collections::HashMap; @@ -157,7 +157,7 @@ mod from_items { (String::from("age"), to_attribute_value(20).unwrap()), ]), HashMap::from([ - (String::from("id"), to_attribute_value(42).unwrap()), + (String::from("id"), to_attribute_value("two").unwrap()), (String::from("name"), to_attribute_value("John").unwrap()), ( String::from("age"), @@ -166,8 +166,8 @@ mod from_items { ]), ]; - let err = from_items::>(items.into()).unwrap_err(); - assert_eq!(Into::::into(ErrorImpl::ExpectedSeq), err); + let err = from_items::(items.into()).unwrap_err(); + assert_eq!(String::from("Expected num at '[1].age'"), err.to_string()); } } From 46cc889365612ddbfbf55599206d1b27880e2e55 Mon Sep 17 00:00:00 2001 From: Quinn Parrott Date: Thu, 25 Sep 2025 10:27:05 -0400 Subject: [PATCH 2/4] Add value that failed to deserialize to Error --- src/de/deserializer.rs | 31 ++++++++++++++++++------------- src/de/deserializer_enum.rs | 15 ++++++++------- src/de/deserializer_map.rs | 17 ++++++++++++----- src/de/deserializer_number.rs | 31 ++++++++++++++++++++----------- src/de/deserializer_seq.rs | 3 +-- src/error.rs | 30 +++++++++++++++++------------- src/tests.rs | 2 +- 7 files changed, 77 insertions(+), 52 deletions(-) diff --git a/src/de/deserializer.rs b/src/de/deserializer.rs index a5aaa17..1493748 100644 --- a/src/de/deserializer.rs +++ b/src/de/deserializer.rs @@ -26,6 +26,11 @@ impl<'a> Deserializer<'a> { pub(crate) fn from_attribute_value_path(input: AttributeValue, path: ErrorPath<'a>) -> Self { Deserializer { input, path } } + + /// Helper that creates an error with context + fn error(self, kind: ErrorImpl) -> Error { + Error::from_path(kind, &self.path, self.input) + } } macro_rules! deserialize_number { @@ -34,7 +39,7 @@ macro_rules! deserialize_number { let de = DeserializerNumber::from_string(n, $self.path); de.$fn($visitor) } else { - return Err(Error::from_path(ErrorImpl::ExpectedNum, &$self.path)); + return Err($self.error(ErrorImpl::ExpectedNum)); } }; } @@ -144,7 +149,7 @@ impl<'de, 'a> de::Deserializer<'de> for Deserializer<'a> { if let AttributeValue::S(s) = self.input { visitor.visit_string(s) } else { - Err(Error::from_path(ErrorImpl::ExpectedString, &self.path)) + Err(self.error(ErrorImpl::ExpectedString)) } } @@ -155,7 +160,7 @@ impl<'de, 'a> de::Deserializer<'de> for Deserializer<'a> { if let AttributeValue::S(s) = self.input { visitor.visit_string(s) } else { - Err(Error::from_path(ErrorImpl::ExpectedString, &self.path)) + Err(self.error(ErrorImpl::ExpectedString)) } } @@ -180,7 +185,7 @@ impl<'de, 'a> de::Deserializer<'de> for Deserializer<'a> { let deserializer_seq = DeserializerSeqBytes::from_vec(bs); visitor.visit_seq(deserializer_seq) } - _ => Err(Error::from_path(ErrorImpl::ExpectedSeq, &self.path)), + _ => Err(self.error(ErrorImpl::ExpectedSeq)), } } @@ -192,7 +197,7 @@ impl<'de, 'a> de::Deserializer<'de> for Deserializer<'a> { let deserializer_map = DeserializerMap::from_item(&mut m, self.path); visitor.visit_map(deserializer_map) } else { - Err(Error::from_path(ErrorImpl::ExpectedMap, &self.path)) + Err(self.error(ErrorImpl::ExpectedMap)) } } @@ -203,7 +208,7 @@ impl<'de, 'a> de::Deserializer<'de> for Deserializer<'a> { if let AttributeValue::Bool(b) = self.input { visitor.visit_bool(b) } else { - Err(Error::from_path(ErrorImpl::ExpectedBool, &self.path)) + Err(self.error(ErrorImpl::ExpectedBool)) } } @@ -211,7 +216,7 @@ impl<'de, 'a> de::Deserializer<'de> for Deserializer<'a> { where V: Visitor<'de>, { - if let AttributeValue::S(s) = self.input { + if let AttributeValue::S(s) = &self.input { let mut chars = s.chars(); if let Some(ch) = chars.next() { let result = visitor.visit_char(ch)?; @@ -220,7 +225,7 @@ impl<'de, 'a> de::Deserializer<'de> for Deserializer<'a> { } } } - Err(Error::from_path(ErrorImpl::ExpectedChar, &self.path)) + Err(self.error(ErrorImpl::ExpectedChar)) } fn deserialize_unit(self, visitor: V) -> Result @@ -230,7 +235,7 @@ impl<'de, 'a> de::Deserializer<'de> for Deserializer<'a> { if let AttributeValue::Null(true) = self.input { visitor.visit_unit() } else { - Err(Error::from_path(ErrorImpl::ExpectedUnit, &self.path)) + Err(self.error(ErrorImpl::ExpectedUnit)) } } @@ -246,7 +251,7 @@ impl<'de, 'a> de::Deserializer<'de> for Deserializer<'a> { match self.input { AttributeValue::S(s) => visitor.visit_enum(s.into_deserializer()), AttributeValue::M(m) => visitor.visit_enum(DeserializerEnum::from_item(m, self.path)), - _ => Err(Error::from_path(ErrorImpl::ExpectedEnum, &self.path)), + _ => Err(self.error(ErrorImpl::ExpectedEnum)), } } @@ -258,7 +263,7 @@ impl<'de, 'a> de::Deserializer<'de> for Deserializer<'a> { let de = DeserializerBytes::from_bytes(b); de.deserialize_bytes(visitor) } else { - Err(Error::from_path(ErrorImpl::ExpectedBytes, &self.path)) + Err(self.error(ErrorImpl::ExpectedBytes)) } } @@ -310,7 +315,7 @@ impl<'de, 'a> de::Deserializer<'de> for Deserializer<'a> { if let AttributeValue::S(s) = self.input { visitor.visit_string(s) } else { - Err(Error::from_path(ErrorImpl::ExpectedString, &self.path)) + Err(self.error(ErrorImpl::ExpectedString)) } } @@ -325,7 +330,7 @@ impl<'de, 'a> de::Deserializer<'de> for Deserializer<'a> { if let AttributeValue::Null(true) = self.input { visitor.visit_unit() } else { - Err(Error::from_path(ErrorImpl::ExpectedUnitStruct, &self.path)) + Err(self.error(ErrorImpl::ExpectedUnitStruct)) } } diff --git a/src/de/deserializer_enum.rs b/src/de/deserializer_enum.rs index f861b0b..3b852f2 100644 --- a/src/de/deserializer_enum.rs +++ b/src/de/deserializer_enum.rs @@ -19,17 +19,18 @@ impl<'de, 'a> EnumAccess<'de> for DeserializerEnum<'a> { type Variant = DeserializerVariant<'a>; type Error = Error; - fn variant_seed(mut self, seed: V) -> Result<(V::Value, Self::Variant), Self::Error> + fn variant_seed(self, seed: V) -> Result<(V::Value, Self::Variant), Self::Error> where V: DeserializeSeed<'de>, { - let mut drain = self.input.drain(); - let (key, value) = drain - .next() - .ok_or_else(|| Error::from_path(ErrorImpl::ExpectedSingleKey, &self.path))?; - if drain.next().is_some() { - return Err(Error::from_path(ErrorImpl::ExpectedSingleKey, &self.path)); + if self.input.len() != 1 { + return Err(Error::from_path( + ErrorImpl::ExpectedSingleKey, + &self.path, + AttributeValue::M(self.input), + )); } + let (key, value) = self.input.into_iter().next().unwrap(); let deserializer = DeserializerVariant::from_attribute_value( value, ErrorPath::Enum(key.clone(), Box::new(self.path)), diff --git a/src/de/deserializer_map.rs b/src/de/deserializer_map.rs index 2237e32..c2fa1bc 100644 --- a/src/de/deserializer_map.rs +++ b/src/de/deserializer_map.rs @@ -43,7 +43,8 @@ impl<'de, 'a> MapAccess<'de> for DeserializerMap<'a> { V: DeserializeSeed<'de>, { if let Some((key, value)) = self.remaining_value.take() { - let de = Deserializer::from_attribute_value_path(value, ErrorPath::Field(&key, &self.path)); + let de = + Deserializer::from_attribute_value_path(value, ErrorPath::Field(&key, &self.path)); seed.deserialize(de) } else { unreachable!("Value without a corresponding key") @@ -75,7 +76,7 @@ macro_rules! deserialize_integer_key { let number = self .input .parse() - .map_err(|_| Error::from_path(ErrorImpl::ExpectedNum, &self.path))?; + .map_err(|_| Error::from_path(ErrorImpl::ExpectedNum, &self.path, AttributeValue::N(self.input.to_owned())))?; visitor.$visit(number) } @@ -125,8 +126,10 @@ impl<'de, 'a> de::Deserializer<'de> for DeserializerMapKey<'a> { where V: de::Visitor<'de>, { - let de = - Deserializer::from_attribute_value_path(AttributeValue::S(self.input.to_owned()), self.path); + let de = Deserializer::from_attribute_value_path( + AttributeValue::S(self.input.to_owned()), + self.path, + ); de.deserialize_enum(name, variants, visitor) } @@ -159,7 +162,11 @@ impl<'de, 'a> de::Deserializer<'de> for DeserializerMapKey<'a> { match self.input { "true" => visitor.visit_bool(true), "false" => visitor.visit_bool(false), - _ => Err(Error::from_path(ErrorImpl::ExpectedString, &self.path)), + _ => Err(Error::from_path( + ErrorImpl::ExpectedString, + &self.path, + AttributeValue::S(self.input.to_owned()), + )), } } diff --git a/src/de/deserializer_number.rs b/src/de/deserializer_number.rs index 5d7bf46..f0b405a 100644 --- a/src/de/deserializer_number.rs +++ b/src/de/deserializer_number.rs @@ -1,6 +1,5 @@ -use crate::de::ErrorPath; - use super::{Error, ErrorImpl, Result}; +use crate::{error::ErrorPath, AttributeValue}; use serde_core::de::{self, Visitor}; use serde_core::forward_to_deserialize_any; @@ -25,27 +24,37 @@ impl<'a> DeserializerNumber<'a> { (Ok(i), _, _) => visitor.visit_i64(i), (_, Ok(u), _) => visitor.visit_u64(u), (_, _, Ok(f)) => visitor.visit_f64(f), - (Err(_), Err(_), Err(e)) => Err(Error::from_path(ErrorImpl::FailedToParseFloat(self.input, e), &self.path)), + (Err(_), Err(_), Err(e)) => Err(Error::from_path( + ErrorImpl::FailedToParseFloat(e), + &self.path, + AttributeValue::N(self.input), + )), } } } macro_rules! deserialize_int { ($self:expr, $visitor:expr, $ty:ty, $fn:ident) => {{ - let n = $self - .input - .parse::<$ty>() - .map_err(|e| Error::from_path(ErrorImpl::FailedToParseInt($self.input, e), &$self.path))?; + let n = $self.input.parse::<$ty>().map_err(|e| { + Error::from_path( + ErrorImpl::FailedToParseInt(e), + &$self.path, + AttributeValue::N($self.input), + ) + })?; $visitor.$fn(n) }}; } macro_rules! deserialize_float { ($self:expr, $visitor:expr, $ty:ty, $fn:ident) => {{ - let n = $self - .input - .parse::<$ty>() - .map_err(|e| Error::from_path(ErrorImpl::FailedToParseFloat($self.input, e), &$self.path))?; + let n = $self.input.parse::<$ty>().map_err(|e| { + Error::from_path( + ErrorImpl::FailedToParseFloat(e), + &$self.path, + AttributeValue::N($self.input), + ) + })?; $visitor.$fn(n) }}; } diff --git a/src/de/deserializer_seq.rs b/src/de/deserializer_seq.rs index dc3af08..ed26125 100644 --- a/src/de/deserializer_seq.rs +++ b/src/de/deserializer_seq.rs @@ -1,8 +1,7 @@ -use crate::de::ErrorPath; - use super::deserializer_bytes::DeserializerBytes; use super::deserializer_number::DeserializerNumber; use super::{AttributeValue, Deserializer, Error, Result}; +use crate::error::ErrorPath; use serde_core::de::{DeserializeSeed, IntoDeserializer, SeqAccess}; pub struct DeserializerSeq<'a> { diff --git a/src/error.rs b/src/error.rs index 2fc9cd7..1153f6c 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,18 +1,20 @@ use serde_core::{de, ser}; use std::fmt::{self, Display, Write}; +use crate::AttributeValue; + /// This type represents all possible errors that can occur when serializing or deserializing /// DynamoDB data. #[derive(Debug, Clone, PartialEq, Eq)] -pub struct Error(Box<(ErrorImpl, String)>); +pub struct Error(Box<(ErrorImpl, String, AttributeValue)>); impl Error { /// Build a new error - pub fn new(error: ErrorImpl, path: String) -> Self { - Self(Box::new((error, path))) + pub fn new(error: ErrorImpl, path: String, input: AttributeValue) -> Self { + Self(Box::new((error, path, input))) } - pub(crate) fn from_path(error: ErrorImpl, path: &ErrorPath<'_>) -> Self { + pub(crate) fn from_path(error: ErrorImpl, path: &ErrorPath<'_>, input: AttributeValue) -> Self { let mut path_str = String::new(); path.visit_path_depth_first(&mut |path| { match path { @@ -33,17 +35,18 @@ impl Error { // Remove trailing '.' path_str.pop(); - Self::new(error, path_str) + Self::new(error, path_str, input) } } impl Display for Error { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let (error_kind, path) = &*self.0; + let (error_kind, path, input) = &*self.0; error_kind.fmt(f)?; if !path.is_empty() { write!(f, " at '{path}'")?; } + write!(f, ". Value: {input:?}")?; Ok(()) } } @@ -95,9 +98,9 @@ pub enum ErrorImpl { /// Expected an item with a single key ExpectedSingleKey, /// Failed to parse as an integer - FailedToParseInt(String, std::num::ParseIntError), + FailedToParseInt(std::num::ParseIntError), /// Failed to parse as a float - FailedToParseFloat(String, std::num::ParseFloatError), + FailedToParseFloat(std::num::ParseFloatError), /// Key must be a string KeyMustBeAString, /// SerializeMap's serialize_key called twice! @@ -112,10 +115,11 @@ pub enum ErrorImpl { BinarySetExpectedType, } +// TODO: Remove this impl #[allow(clippy::from_over_into)] impl Into for ErrorImpl { fn into(self) -> Error { - Error(Box::new((self, String::new()))) + Error(Box::new((self, String::new(), AttributeValue::Null(true)))) } } @@ -136,11 +140,11 @@ impl Display for ErrorImpl { ErrorImpl::ExpectedEnum => f.write_str("Expected enum"), ErrorImpl::ExpectedBytes => f.write_str("Expected binary data"), ErrorImpl::ExpectedSingleKey => f.write_str("Expected an item with a single key"), - ErrorImpl::FailedToParseInt(s, err) => { - write!(f, "Failed to parse '{s}' as an integer: {err}") + ErrorImpl::FailedToParseInt(err) => { + write!(f, "Failed to parse integer {err}") } - ErrorImpl::FailedToParseFloat(s, err) => { - write!(f, "Failed to parse '{s}' as a float: {err}") + ErrorImpl::FailedToParseFloat(err) => { + write!(f, "Failed to parse float {err}") } ErrorImpl::KeyMustBeAString => f.write_str("Key must be a string"), ErrorImpl::SerializeMapKeyCalledTwice => { diff --git a/src/tests.rs b/src/tests.rs index e709585..59d3e7f 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -167,7 +167,7 @@ mod from_items { ]; let err = from_items::(items.into()).unwrap_err(); - assert_eq!(String::from("Expected num at '[1].age'"), err.to_string()); + assert_eq!(String::from("Expected num at '[1].age'. Value: S(\"not a number\")"), err.to_string()); } } From 7aac42fe43997a8e6608555a56798aaf14269d96 Mon Sep 17 00:00:00 2001 From: Quinn Parrott Date: Thu, 25 Sep 2025 10:40:30 -0400 Subject: [PATCH 3/4] Improved set serialize --- src/binary_set.rs | 14 ++++++++++++-- src/error.rs | 13 +++++++------ src/number_set.rs | 14 ++++++++++++-- src/string_set.rs | 14 ++++++++++++-- 4 files changed, 43 insertions(+), 12 deletions(-) diff --git a/src/binary_set.rs b/src/binary_set.rs index efbb80b..548f855 100644 --- a/src/binary_set.rs +++ b/src/binary_set.rs @@ -123,7 +123,13 @@ where pub(crate) fn convert_to_set(value: crate::AttributeValue) -> crate::Result { let vals = match value { crate::AttributeValue::L(vals) => vals, - _ => return Err(crate::error::ErrorImpl::NotSetlike.into()), + _ => { + return Err(crate::error::Error::new( + crate::error::ErrorImpl::NotSetlike, + String::new(), + Some(value), + )) + } }; let set = vals @@ -132,7 +138,11 @@ pub(crate) fn convert_to_set(value: crate::AttributeValue) -> crate::Result>()?; diff --git a/src/error.rs b/src/error.rs index 1153f6c..090217a 100644 --- a/src/error.rs +++ b/src/error.rs @@ -6,12 +6,12 @@ use crate::AttributeValue; /// This type represents all possible errors that can occur when serializing or deserializing /// DynamoDB data. #[derive(Debug, Clone, PartialEq, Eq)] -pub struct Error(Box<(ErrorImpl, String, AttributeValue)>); +pub struct Error(Box<(ErrorImpl, String, Option)>); impl Error { /// Build a new error - pub fn new(error: ErrorImpl, path: String, input: AttributeValue) -> Self { - Self(Box::new((error, path, input))) + pub fn new(error: ErrorImpl, path: String, input: impl Into>) -> Self { + Self(Box::new((error, path, input.into()))) } pub(crate) fn from_path(error: ErrorImpl, path: &ErrorPath<'_>, input: AttributeValue) -> Self { @@ -46,7 +46,9 @@ impl Display for Error { if !path.is_empty() { write!(f, " at '{path}'")?; } - write!(f, ". Value: {input:?}")?; + if let Some(input) = input { + write!(f, ". Value: {input:?}")?; + } Ok(()) } } @@ -115,11 +117,10 @@ pub enum ErrorImpl { BinarySetExpectedType, } -// TODO: Remove this impl #[allow(clippy::from_over_into)] impl Into for ErrorImpl { fn into(self) -> Error { - Error(Box::new((self, String::new(), AttributeValue::Null(true)))) + Error::new(self, String::new(), None) } } diff --git a/src/number_set.rs b/src/number_set.rs index 115e629..870042f 100644 --- a/src/number_set.rs +++ b/src/number_set.rs @@ -118,7 +118,13 @@ where pub(crate) fn convert_to_set(value: crate::AttributeValue) -> crate::Result { let vals = match value { crate::AttributeValue::L(vals) => vals, - _ => return Err(crate::error::ErrorImpl::NotSetlike.into()), + _ => { + return Err(crate::error::Error::new( + crate::error::ErrorImpl::NotSetlike, + String::new(), + Some(value), + )) + } }; let set = vals @@ -127,7 +133,11 @@ pub(crate) fn convert_to_set(value: crate::AttributeValue) -> crate::Result>()?; diff --git a/src/string_set.rs b/src/string_set.rs index 2999806..4b32136 100644 --- a/src/string_set.rs +++ b/src/string_set.rs @@ -118,7 +118,13 @@ where pub(crate) fn convert_to_set(value: crate::AttributeValue) -> crate::Result { let vals = match value { crate::AttributeValue::L(vals) => vals, - _ => return Err(crate::error::ErrorImpl::NotSetlike.into()), + _ => { + return Err(crate::error::Error::new( + crate::error::ErrorImpl::NotSetlike, + String::new(), + Some(value), + )) + } }; let set = vals @@ -127,7 +133,11 @@ pub(crate) fn convert_to_set(value: crate::AttributeValue) -> crate::Result>()?; From 04e3e18f2020ea1f41ddfa9e784e982eeeac2b76 Mon Sep 17 00:00:00 2001 From: Quinn Parrott Date: Sat, 27 Sep 2025 10:32:56 -0400 Subject: [PATCH 4/4] Remove an allocation --- src/de/deserializer.rs | 2 +- src/de/deserializer_enum.rs | 6 +++--- src/error.rs | 8 ++------ 3 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/de/deserializer.rs b/src/de/deserializer.rs index 1493748..58299fe 100644 --- a/src/de/deserializer.rs +++ b/src/de/deserializer.rs @@ -250,7 +250,7 @@ impl<'de, 'a> de::Deserializer<'de> for Deserializer<'a> { { match self.input { AttributeValue::S(s) => visitor.visit_enum(s.into_deserializer()), - AttributeValue::M(m) => visitor.visit_enum(DeserializerEnum::from_item(m, self.path)), + AttributeValue::M(m) => visitor.visit_enum(DeserializerEnum::from_item(m, &self.path)), _ => Err(self.error(ErrorImpl::ExpectedEnum)), } } diff --git a/src/de/deserializer_enum.rs b/src/de/deserializer_enum.rs index 3b852f2..35014e3 100644 --- a/src/de/deserializer_enum.rs +++ b/src/de/deserializer_enum.rs @@ -6,11 +6,11 @@ use std::collections::HashMap; pub struct DeserializerEnum<'a> { input: HashMap, - path: ErrorPath<'a>, + path: &'a ErrorPath<'a>, } impl<'a> DeserializerEnum<'a> { - pub fn from_item(input: HashMap, path: ErrorPath<'a>) -> Self { + pub fn from_item(input: HashMap, path: &'a ErrorPath<'a>) -> Self { Self { input, path } } } @@ -33,7 +33,7 @@ impl<'de, 'a> EnumAccess<'de> for DeserializerEnum<'a> { let (key, value) = self.input.into_iter().next().unwrap(); let deserializer = DeserializerVariant::from_attribute_value( value, - ErrorPath::Enum(key.clone(), Box::new(self.path)), + ErrorPath::Enum(key.clone(), self.path), ); let value = seed.deserialize(key.into_deserializer())?; diff --git a/src/error.rs b/src/error.rs index 090217a..8ce27fb 100644 --- a/src/error.rs +++ b/src/error.rs @@ -190,18 +190,14 @@ pub(crate) enum ErrorPath<'a> { Root, Field(&'a str, &'a ErrorPath<'a>), Elem(usize, &'a ErrorPath<'a>), - Enum(String, Box>), + Enum(String, &'a ErrorPath<'a>), } impl<'a> ErrorPath<'a> { pub(crate) fn visit_path_depth_first(&self, fun: &mut impl FnMut(&ErrorPath<'a>)) { match self { ErrorPath::Root => {} - ErrorPath::Field(_, path) | ErrorPath::Elem(_, path) => { - path.visit_path_depth_first(fun); - fun(self); - } - ErrorPath::Enum(_, path) => { + ErrorPath::Field(_, path) | ErrorPath::Elem(_, path) | ErrorPath::Enum(_, path) => { path.visit_path_depth_first(fun); fun(self); }