Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop using #[serde(untagged)] for Message enum #206

Closed
wants to merge 13 commits into from
2 changes: 1 addition & 1 deletion chromiumoxide_types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@ include = ["src/**/*", "LICENSE-*"]

[dependencies]
serde = { version = "1", features = ["derive"] }
serde_json = "1"
serde_json = { version = "1", features = ["raw_value"] }
44 changes: 42 additions & 2 deletions chromiumoxide_types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ use std::borrow::Cow;
use std::fmt;
use std::fmt::Debug;
use std::ops::Deref;
use std::str::FromStr;

use serde::de::DeserializeOwned;
use serde::{Deserialize, Serialize};
use serde_json::value::RawValue;

pub type MethodId = Cow<'static, str>;

Expand Down Expand Up @@ -199,8 +201,10 @@ pub struct Response {
/// An incoming message read from the web socket can either be a response to a
/// previously submitted `Request`, identified by an identifier `id`, or an
/// `Event` emitted by the server.
#[derive(Deserialize, Debug, Clone)]
#[serde(untagged)]
///
/// This implements both `Deserialize` and `FromStr`, where the latter is preferred in performance
/// and error reporting.
#[derive(Debug, Clone)]
#[allow(clippy::large_enum_variant)]
pub enum Message<T = CdpJsonEventMessage> {
/// A response for a request
Expand All @@ -209,6 +213,42 @@ pub enum Message<T = CdpJsonEventMessage> {
Event(T),
}

// Manually implements Deserialize because `#[serde(untagged)]` does not work correctly with serde_json and some numeric types in the schema,
// without activating `arbitrary_precision` feature of serde_json and using `serde_json::Number` instead of all usage of `f64` or else.
// - https://github.com/serde-rs/serde/issues/2661
// For now, we implement the `FromStr` trait to deserialize the message, and use it in this `Deserialize` impl.
impl<'de, T> Deserialize<'de> for Message<T>
where
T: DeserializeOwned,
{
fn deserialize<D>(deserializer: D) -> Result<Message<T>, D::Error>
where
D: serde::Deserializer<'de>,
{
let value: &RawValue = Deserialize::deserialize(deserializer)?;
value.get().parse().map_err(serde::de::Error::custom)
}
}

impl<T> FromStr for Message<T>
where
T: DeserializeOwned,
{
type Err = serde_json::Error;

fn from_str(s: &str) -> Result<Self, Self::Err> {
if let Ok(response) = serde_json::from_str::<Response>(s) {
Ok(Message::Response(response))
} else {
// For now, returns an error only about the event deserialization.
// Ideally, we can return an custom error type that contains errors for both response and event.
// It seems not necessary for now because the response seems to be deserialized correctly in most cases.
let event = serde_json::from_str::<T>(s)?;
Ok(Message::Event(event))
}
}
}

/// A response can either contain the `Command::Response` type in the `result`
/// field of the payload or an `Error` in the `error` field if the request
/// resulted in an error.
Expand Down
Loading