Skip to content

Commit

Permalink
serde: serialize dyn Error Values using collect_str (#89)
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 8, 2022
1 parent 662a983 commit 01a4d6a
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion valuable-serde/src/lib.rs
Original file line number Diff line number Diff line change
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 01a4d6a

Please sign in to comment.