Skip to content

Commit

Permalink
Merge pull request #154 from quartiq/rj/serde-error
Browse files Browse the repository at this point in the history
preserve serde error info
  • Loading branch information
ryan-summers authored Jul 25, 2023
2 parents 471b662 + 78df731 commit 3bfe29a
Show file tree
Hide file tree
Showing 7 changed files with 100 additions and 73 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ for path in Settings::iter_paths::<3, 32>().unwrap() {
}
}

# Ok::<(), miniconf::Error>(())
# Ok::<(), miniconf::Error<miniconf::serde_json_core::de::Error>>(())
```

## MQTT
Expand Down
8 changes: 4 additions & 4 deletions miniconf_derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ fn get_path_arm(struct_field: &StructField) -> proc_macro2::TokenStream {
if path_parts.next().is_some() {
Err(miniconf::Error::PathTooLong)
} else {
miniconf::serde::ser::Serialize::serialize(&self.#match_name, ser).map_err(|_| miniconf::Error::Serialization)
Ok(miniconf::serde::ser::Serialize::serialize(&self.#match_name, ser)?)
}
}
}
Expand All @@ -80,7 +80,7 @@ fn set_path_arm(struct_field: &StructField) -> proc_macro2::TokenStream {
if path_parts.next().is_some() {
Err(miniconf::Error::PathTooLong)
} else {
self.#match_name = miniconf::serde::de::Deserialize::deserialize(de).map_err(|_| miniconf::Error::Deserialization)?;
self.#match_name = miniconf::serde::de::Deserialize::deserialize(de)?;
Ok(())
}
}
Expand Down Expand Up @@ -168,7 +168,7 @@ fn derive_struct(

quote! {
impl #impl_generics miniconf::Miniconf for #ident #ty_generics #where_clause {
fn set_path<'a, 'b: 'a, P, D>(&mut self, path_parts: &mut P, de: D) -> Result<(), miniconf::Error>
fn set_path<'a, 'b: 'a, P, D>(&mut self, path_parts: &mut P, de: D) -> Result<(), miniconf::Error<D::Error>>
where
P: Iterator<Item = &'a str>,
D: miniconf::serde::Deserializer<'b>,
Expand All @@ -179,7 +179,7 @@ fn derive_struct(
}
}

fn get_path<'a, P, S>(&self, path_parts: &mut P, ser: S) -> Result<S::Ok, miniconf::Error>
fn get_path<'a, P, S>(&self, path_parts: &mut P, ser: S) -> Result<S::Ok, miniconf::Error<S::Error>>
where
P: Iterator<Item = &'a str>,
S: miniconf::serde::Serializer,
Expand Down
53 changes: 26 additions & 27 deletions src/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,28 +110,34 @@ const fn digits(x: usize) -> usize {
}

impl<T: Miniconf, const N: usize> Miniconf for Array<T, N> {
fn set_path<'a, 'b: 'a, P, D>(&mut self, path_parts: &mut P, de: D) -> Result<(), Error>
fn set_path<'a, 'b: 'a, P, D>(
&mut self,
path_parts: &mut P,
de: D,
) -> Result<(), Error<D::Error>>
where
P: Iterator<Item = &'a str>,
D: serde::Deserializer<'b>,
{
let i = self.0.index(path_parts.next())?;
let next = path_parts.next().ok_or(Error::PathTooShort)?;
let index: usize = next.parse().map_err(|_| Error::BadIndex)?;

self.0
.get_mut(i)
.get_mut(index)
.ok_or(Error::BadIndex)?
.set_path(path_parts, de)
}

fn get_path<'a, P, S>(&self, path_parts: &mut P, ser: S) -> Result<S::Ok, Error>
fn get_path<'a, P, S>(&self, path_parts: &mut P, ser: S) -> Result<S::Ok, Error<S::Error>>
where
P: Iterator<Item = &'a str>,
S: serde::Serializer,
{
let i = self.0.index(path_parts.next())?;
let next = path_parts.next().ok_or(Error::PathTooShort)?;
let index: usize = next.parse().map_err(|_| Error::BadIndex)?;

self.0
.get(i)
.get(index)
.ok_or(Error::BadIndex)?
.get_path(path_parts, ser)
}
Expand Down Expand Up @@ -167,49 +173,42 @@ impl<T: Miniconf, const N: usize> Miniconf for Array<T, N> {
}
}

trait IndexLookup {
fn index(&self, next: Option<&str>) -> Result<usize, Error>;
}

impl<T, const N: usize> IndexLookup for [T; N] {
fn index(&self, next: Option<&str>) -> Result<usize, Error> {
let next = next.ok_or(Error::PathTooShort)?;

// Parse what should be the index value
next.parse().map_err(|_| Error::BadIndex)
}
}

impl<T: crate::Serialize + crate::DeserializeOwned, const N: usize> Miniconf for [T; N] {
fn set_path<'a, 'b: 'a, P, D>(&mut self, path_parts: &mut P, de: D) -> Result<(), Error>
fn set_path<'a, 'b: 'a, P, D>(
&mut self,
path_parts: &mut P,
de: D,
) -> Result<(), Error<D::Error>>
where
P: Iterator<Item = &'a str>,
D: serde::Deserializer<'b>,
{
let i = self.index(path_parts.next())?;
let next = path_parts.next().ok_or(Error::PathTooShort)?;
let index: usize = next.parse().map_err(|_| Error::BadIndex)?;

if path_parts.next().is_some() {
return Err(Error::PathTooLong);
}

let item = <[T]>::get_mut(self, i).ok_or(Error::BadIndex)?;
*item = serde::Deserialize::deserialize(de).map_err(|_| Error::Deserialization)?;
let item = <[T]>::get_mut(self, index).ok_or(Error::BadIndex)?;
*item = serde::Deserialize::deserialize(de)?;
Ok(())
}

fn get_path<'a, P, S>(&self, path_parts: &mut P, ser: S) -> Result<S::Ok, Error>
fn get_path<'a, P, S>(&self, path_parts: &mut P, ser: S) -> Result<S::Ok, Error<S::Error>>
where
P: Iterator<Item = &'a str>,
S: serde::Serializer,
{
let i = self.index(path_parts.next())?;
let next = path_parts.next().ok_or(Error::PathTooShort)?;
let index: usize = next.parse().map_err(|_| Error::BadIndex)?;

if path_parts.next().is_some() {
return Err(Error::PathTooLong);
}

let item = <[T]>::get(self, i).ok_or(Error::BadIndex)?;
serde::Serialize::serialize(item, ser).map_err(|_| Error::Serialization)
let item = <[T]>::get(self, index).ok_or(Error::BadIndex)?;
Ok(serde::Serialize::serialize(item, ser)?)
}

fn metadata(separator_length: usize) -> Metadata {
Expand Down
76 changes: 47 additions & 29 deletions src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#![cfg_attr(not(test), no_std)]
#![cfg_attr(not(any(test, doctest)), no_std)]
#![doc = include_str!("../README.md")]

use core::fmt::Write;
Expand Down Expand Up @@ -52,7 +52,7 @@ pub enum IterError {
/// Errors that can occur when using the [Miniconf] API.
#[non_exhaustive]
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub enum Error {
pub enum Error<T> {
/// The provided path wasn't found in the structure.
///
/// Double check the provided path to verify that it's valid.
Expand All @@ -68,22 +68,18 @@ pub enum Error {
/// Double check the ending and add the remainder of the path.
PathTooShort,

/// The value provided for configuration could not be deserialized into the proper type.
/// The value provided could not be serialized or deserialized.
///
/// Check that the serialized data is valid and of the correct type.
Deserialization,
/// Check that the buffer had sufficient space.
SerDe(T),

/// There was an error after deserializing a value.
///
/// If the `Deserializer` encounters an error only after successfully
/// deserializing a value (as is the case if there is additional unexpected data),
/// the update may have taken place but this error will still be returned.
PostDeserialization,

/// The value provided could not be serialized.
///
/// Check that the buffer had sufficient space.
Serialization,
PostDeserialization(T),

/// When indexing into an array, the index provided was out of bounds.
///
Expand All @@ -99,18 +95,10 @@ pub enum Error {
PathAbsent,
}

impl From<Error> for u8 {
fn from(err: Error) -> u8 {
match err {
Error::PathNotFound => 1,
Error::PathTooLong => 2,
Error::PathTooShort => 3,
Error::PostDeserialization => 4,
Error::Deserialization => 5,
Error::BadIndex => 6,
Error::Serialization => 7,
Error::PathAbsent => 8,
}
impl<T> From<T> for Error<T> {
fn from(err: T) -> Self {
// By default in our context every T is a SerDe error.
Error::SerDe(err)
}
}

Expand Down Expand Up @@ -138,7 +126,11 @@ pub trait Miniconf {
///
/// # Returns
/// May return an [Error].
fn set_path<'a, 'b: 'a, P, D>(&mut self, path_parts: &mut P, de: D) -> Result<(), Error>
fn set_path<'a, 'b: 'a, P, D>(
&mut self,
path_parts: &mut P,
de: D,
) -> Result<(), Error<D::Error>>
where
P: Iterator<Item = &'a str>,
D: serde::Deserializer<'b>;
Expand All @@ -151,7 +143,7 @@ pub trait Miniconf {
///
/// # Returns
/// May return an [Error].
fn get_path<'a, P, S>(&self, path_parts: &mut P, ser: S) -> Result<S::Ok, Error>
fn get_path<'a, P, S>(&self, path_parts: &mut P, ser: S) -> Result<S::Ok, Error<S::Error>>
where
P: Iterator<Item = &'a str>,
S: serde::Serializer;
Expand Down Expand Up @@ -196,6 +188,11 @@ pub trait SerDe<S>: Miniconf {
/// used in [SerDe::set] and [SerDe::get] to split the path.
const SEPARATOR: char;

/// The [serde::Serializer::Error] type.
type SerError;
/// The [serde::Deserializer::Error] type.
type DeError;

/// Create an iterator of all possible paths.
///
/// This is a depth-first walk.
Expand Down Expand Up @@ -240,7 +237,7 @@ pub trait SerDe<S>: Miniconf {
///
/// # Returns
/// The number of bytes consumed from `data` or an [Error].
fn set(&mut self, path: &str, data: &[u8]) -> Result<usize, Error>;
fn set(&mut self, path: &str, data: &[u8]) -> Result<usize, Error<Self::DeError>>;

/// Retrieve a serialized value by path.
///
Expand All @@ -250,7 +247,7 @@ pub trait SerDe<S>: Miniconf {
///
/// # Returns
/// The number of bytes used in the `data` buffer or an [Error].
fn get(&self, path: &str, data: &mut [u8]) -> Result<usize, Error>;
fn get(&self, path: &str, data: &mut [u8]) -> Result<usize, Error<Self::SerError>>;
}

/// Marker struct for [SerDe].
Expand All @@ -264,16 +261,37 @@ where
T: Miniconf,
{
const SEPARATOR: char = '/';
type DeError = serde_json_core::de::Error;
type SerError = serde_json_core::ser::Error;

fn set(&mut self, path: &str, data: &[u8]) -> Result<usize, Error> {
fn set(&mut self, path: &str, data: &[u8]) -> Result<usize, Error<Self::DeError>> {
let mut de = serde_json_core::de::Deserializer::new(data);
self.set_path(&mut path.split(Self::SEPARATOR).skip(1), &mut de)?;
de.end().map_err(|_| Error::PostDeserialization)
de.end().map_err(Error::PostDeserialization)
}

fn get(&self, path: &str, data: &mut [u8]) -> Result<usize, Error> {
fn get(&self, path: &str, data: &mut [u8]) -> Result<usize, Error<Self::SerError>> {
let mut ser = serde_json_core::ser::Serializer::new(data);
self.get_path(&mut path.split(Self::SEPARATOR).skip(1), &mut ser)?;
Ok(ser.end())
}
}

// These allow unifying serde error information to make writing examples
// and tests easier. Doing this conversion is optional.
// #[cfg(any(test, doctest))]
impl From<Error<serde_json_core::ser::Error>> for Error<serde_json_core::de::Error> {
fn from(value: Error<serde_json_core::ser::Error>) -> Self {
match value {
Error::BadIndex => Self::BadIndex,
Error::PathAbsent => Self::PathAbsent,
Error::PathNotFound => Self::PathNotFound,
Error::PathTooLong => Self::PathTooLong,
Error::PathTooShort => Self::PathTooShort,
Error::PostDeserialization(_) => {
Error::PostDeserialization(serde_json_core::de::Error::CustomError)
}
Error::SerDe(_) => Self::SerDe(serde_json_core::de::Error::CustomError),
}
}
}
10 changes: 6 additions & 4 deletions src/mqtt_client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ impl<'a> Command<'a> {
/// ```
pub struct MqttClient<Settings, Stack, Clock, const MESSAGE_SIZE: usize>
where
Settings: Miniconf + Clone,
Settings: SerDe<JsonCoreSlash> + Clone,
Stack: TcpClientStack,
Clock: embedded_time::Clock,
{
Expand All @@ -175,9 +175,11 @@ where
impl<Settings, Stack, Clock, const MESSAGE_SIZE: usize>
MqttClient<Settings, Stack, Clock, MESSAGE_SIZE>
where
Settings: Miniconf + Clone,
Settings: SerDe<JsonCoreSlash> + Clone,
Stack: TcpClientStack,
Clock: embedded_time::Clock + Clone,
Settings::DeError: core::fmt::Debug,
Settings::SerError: core::fmt::Debug,
{
/// Construct a new MQTT settings interface.
///
Expand Down Expand Up @@ -672,8 +674,8 @@ impl<T, E: AsRef<str>, const N: usize> From<Result<T, E>> for Response<N> {
}
}

impl<const N: usize> From<crate::Error> for Response<N> {
fn from(err: crate::Error) -> Self {
impl<const N: usize, T: core::fmt::Debug> From<crate::Error<T>> for Response<N> {
fn from(err: crate::Error<T>) -> Self {
let mut msg = String::new();
if write!(&mut msg, "{:?}", err).is_err() {
msg = String::from("Configuration Error");
Expand Down
20 changes: 14 additions & 6 deletions src/option.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,11 @@ impl<T> From<Option<T>> for core::option::Option<T> {
}

impl<T: Miniconf> Miniconf for Option<T> {
fn set_path<'a, 'b: 'a, P, D>(&mut self, path_parts: &mut P, de: D) -> Result<(), Error>
fn set_path<'a, 'b: 'a, P, D>(
&mut self,
path_parts: &mut P,
de: D,
) -> Result<(), Error<D::Error>>
where
P: Iterator<Item = &'a str>,
D: serde::Deserializer<'b>,
Expand All @@ -99,7 +103,7 @@ impl<T: Miniconf> Miniconf for Option<T> {
}
}

fn get_path<'a, P, S>(&self, path_parts: &mut P, ser: S) -> Result<S::Ok, Error>
fn get_path<'a, P, S>(&self, path_parts: &mut P, ser: S) -> Result<S::Ok, Error<S::Error>>
where
P: Iterator<Item = &'a str>,
S: serde::Serializer,
Expand All @@ -126,7 +130,11 @@ impl<T: Miniconf> Miniconf for Option<T> {
}

impl<T: crate::Serialize + crate::DeserializeOwned> Miniconf for core::option::Option<T> {
fn set_path<'a, 'b: 'a, P, D>(&mut self, path_parts: &mut P, de: D) -> Result<(), Error>
fn set_path<'a, 'b: 'a, P, D>(
&mut self,
path_parts: &mut P,
de: D,
) -> Result<(), Error<D::Error>>
where
P: Iterator<Item = &'a str>,
D: serde::Deserializer<'b>,
Expand All @@ -139,11 +147,11 @@ impl<T: crate::Serialize + crate::DeserializeOwned> Miniconf for core::option::O
return Err(Error::PathAbsent);
}

*self = Some(serde::Deserialize::deserialize(de).map_err(|_| Error::Deserialization)?);
*self = Some(serde::Deserialize::deserialize(de)?);
Ok(())
}

fn get_path<'a, P, S>(&self, path_parts: &mut P, ser: S) -> Result<S::Ok, Error>
fn get_path<'a, P, S>(&self, path_parts: &mut P, ser: S) -> Result<S::Ok, Error<S::Error>>
where
P: Iterator<Item = &'a str>,
S: serde::Serializer,
Expand All @@ -153,7 +161,7 @@ impl<T: crate::Serialize + crate::DeserializeOwned> Miniconf for core::option::O
}

let data = self.as_ref().ok_or(Error::PathAbsent)?;
serde::Serialize::serialize(data, ser).map_err(|_| Error::Serialization)
Ok(serde::Serialize::serialize(data, ser)?)
}

fn metadata(_separator_length: usize) -> Metadata {
Expand Down
Loading

0 comments on commit 3bfe29a

Please sign in to comment.