Skip to content

Commit

Permalink
serde: serialize dyn Error Values using collect_str
Browse files Browse the repository at this point in the history
Currently, `valuable-serde` handles `Value::Error` variants incorrectly.
When `valuable-serde` encounters a `Value::Error`, it returns the error
immediately as a serialization error:
https://github.com/tokio-rs/valuable/blob/1fc2ad50fcae7fc0da81dc40a385c235636ba525/valuable-serde/src/lib.rs#L267

This is _not_ correct. If a `serde` serializer returns an error, this
means something has gone wrong while serializing a value. Returning an
error makes the serialization fail. However, this is *not* what
`valuable`'s `Value::Error` means. `Value::Error` represents that we are
_recording a value which happens to be an `Error`_, not that something
went wrong _while_ recording the value. This means that `valuable-serde`
will currently return an `Error` (indicating that serialization failed)
any time it attempts to record an `Error` value, and serialization will
fail.

This commit changes `valuable-serde` to record the error using its
`Display` implementation, using `serde`'s `collect_str` method. Now,
`Error`s will be serialized by recording their messages as a string,
rather than causing the serialization to fail.

Using `collect_str` allows the serializer to write the display
representation to its own internal buffer, rather than having to format
the error to a temporary `String` beforehand.

Fixes #88
  • Loading branch information
hawkw committed Mar 3, 2022
1 parent 1fc2ad5 commit 290d9c8
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions valuable-serde/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ where
where
S: Serializer,
{
match self.0.as_value() {
match dbg!(self.0.as_value()) {
Value::Bool(b) => serializer.serialize_bool(b),
Value::I8(n) => serializer.serialize_i8(n),
Value::I16(n) => serializer.serialize_i16(n),
Expand Down Expand Up @@ -264,7 +264,7 @@ where
#[cfg(feature = "std")]
Value::Path(p) => Serialize::serialize(p, serializer),
#[cfg(feature = "std")]
Value::Error(e) => Err(S::Error::custom(e)),
Value::Error(e) => serializer.collect_str(e),

v => unimplemented!("{:?}", v),
}
Expand Down

0 comments on commit 290d9c8

Please sign in to comment.