RUST-1406 Convert serde errors to standard error type #562
Conversation
isabelatkinson
force-pushed
the
serde-errors
branch
from
ac4ec3a to
6f35184
Compare
| Self::DeserializationError { | ||
| message: err.to_string(), | ||
| } | ||
| fn from(error: serde_json::Error) -> Self { |
There was a problem hiding this comment.
I kept this From impl in the public API because we're already exposing a bunch of serde_json conversions below. It might be worth putting all of the serde_json stuff behind the serde feature flag or giving it its own flag - if that makes sense I'll file a separate ticket.
There was a problem hiding this comment.
Makes sense; I agree that putting the serde_json stuff behind its own feature flag would definitely be a good thing.
There was a problem hiding this comment.
filed RUST-2240
| fn serialize_bool(self, v: bool) -> Result<Self::Ok> { | ||
| Err(Self::invalid_key(v)) | ||
| fn serialize_bool(self, _v: bool) -> Result<Self::Ok> { | ||
| Err(Error::invalid_key_type("bool")) |
There was a problem hiding this comment.
This is a deviation from the current API, which stores the invalid key as Bson. This leads to some weird placeholders (empty arrays and documents, Bson::Null if conversion fails). I think a string message that reports the bad type is sufficient here, and users can turn on serde_path_to_error for more detail.
| Self::DeserializationError { | ||
| message: err.to_string(), | ||
| } | ||
| fn from(error: serde_json::Error) -> Self { |
There was a problem hiding this comment.
Makes sense; I agree that putting the serde_json stuff behind its own feature flag would definitely be a good thing.
2 participants
This is the last of the conversions.