From 461c3d78e60e5a7970b18e5950d5bd2d8bf719da Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Wed, 2 Mar 2022 16:07:28 -0800 Subject: [PATCH] serde: serialize `dyn Error` `Value`s using `collect_str` 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 --- valuable-serde/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/valuable-serde/src/lib.rs b/valuable-serde/src/lib.rs index 73d53d46..03d79422 100644 --- a/valuable-serde/src/lib.rs +++ b/valuable-serde/src/lib.rs @@ -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), }