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

juniper_graphql_ws: serde deserialization error with null values #735

Closed
lightdiscord opened this issue Aug 12, 2020 · 2 comments · Fixed by #738
Closed

juniper_graphql_ws: serde deserialization error with null values #735

lightdiscord opened this issue Aug 12, 2020 · 2 comments · Fixed by #738
Labels
area::subscriptions Related to GraphQL subscriptions bug Something isn't working

Comments

@lightdiscord
Copy link
Contributor

Describe the bug
When a client start a subscription it sends a payload which can contain optional variables. Currently, serde default field attribute is being used.
It works if the key is not present or if it is an empty object ({}) but yield a serde error if it's null (invalid type: null, expected a map).

To Reproduce

  • Launch the actix_subscriptions example.
  • Launch a client (graphiql for example) and send a payload with "variables": null (with graphiql it should send null if no variable are specified)

Expected behavior
Treating the field as if it were absent.

Additional context
This kind of problem was already discussed on this issue.

Potential fix
One way to solve the issue is to use the default attribute and deserialize_with for example:

fn unwrap_or_default<'de, D, T>(deserializer: D) -> Result<T, D::Error>
where
    D: Deserializer<'de>,
    T: Deserialize<'de> + Default
{
    Ok(Option::<T>::deserialize(deserializer)?.unwrap_or_default())
}

#[derive(Debug, Deserialize, PartialEq)]
#[serde(bound(deserialize = "S: ScalarValue"))]
#[serde(rename_all = "camelCase")]
pub struct StartPayload<S: ScalarValue> {
    pub query: String,
    #[serde(default, deserialize_with = "unwrap_or_default")]
    pub variables: Variables<S>,
    pub operation_name: Option<String>,
}
@lightdiscord lightdiscord added bug Something isn't working needs-triage labels Aug 12, 2020
@LegNeato
Copy link
Member

Sounds great, thank you for the report and the suggested fix! Would you be willing to put a PR up?

@LegNeato LegNeato added area::subscriptions Related to GraphQL subscriptions and removed needs-triage labels Aug 14, 2020
@lightdiscord
Copy link
Contributor Author

No problem. I'll open a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area::subscriptions Related to GraphQL subscriptions bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants