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

Unexpected behavior of time::serde::rfc3339::option::deserialize() #637

Open
OrangeTux opened this issue Nov 14, 2023 · 3 comments
Open
Labels
A-third-party Area: implementations of traits from other crates C-enhancement Category: an enhancement with existing code

Comments

@OrangeTux
Copy link

When deserializing some data into a struct, serde doesn't care if an field with type Option<T> is available in the source bytes or not.
For example, the following code works fine. It parses an empty json string {} as a Passes instance.

// Relies on serde and serde-json.
// Code runs fine.
use serde::Deserialize;
use serde_json::json;


#[derive(Deserialize, Debug)]
struct Passes {
    attribute: Option<i32>,
}


fn main() {
    let source = json!({});
    let concrete_type: Passes = serde_json::from_value(source).unwrap();
    dbg!(concrete_type);

I would expect the same behavior for something of type Option<time::OffsetDateTime>. But that type behaves different when using the time::serde::rfc3339::option::deserialize. Consider the following code. When you run it, it'll panic. The code tries to deserialize an empty {} as a struct Fails. And that fails. serde claims that fields datetime is missing. Although the attribute being Optional,

/// relies on serde, serde-json and time.

use time::OffsetDateTime;
use serde::Deserialize;
use serde_json::json;


#[derive(Deserialize, Debug)]
struct Fails {
    #[serde(with = "time::serde::rfc3339::option")]
    datetime: Option<OffsetDateTime>,
}

fn main() {
    let source = json!({});
    /// Code panics here.
    let concrete_type: Fails = serde_json::from_value(source).unwrap();
}

The failure is:

thread 'main' panicked at src/main.rs:27:63:
called `Result::unwrap()` on an `Err` value: Error("missing field `datetime`", line: 0, column: 0)
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Is this is bug? Shouldn't datetime be set to None when the field is not present input that's passed to serde_jsoin:from_value()?
If so, I'm happy to provide a PR to fix the behavior.

Note, I'm aware that I can work around the issue by changing the field attribute of Fails.datetime from 'm aware that I can work around #[serde(with = "time::serde::rfc3339::option")] to #[serde(with = "time::serde::rfc3339::option", default)]

@jhpratt
Copy link
Member

jhpratt commented Nov 14, 2023

This has been brought up before, and I'm not aware of how to fix it. My recommendation has always been to use #[serde(default)]. If there is a better way, I'm more than happy to review a PR.

@jhpratt jhpratt added C-enhancement Category: an enhancement with existing code A-third-party Area: implementations of traits from other crates labels Nov 14, 2023
@OrangeTux
Copy link
Author

Related #562

@OrangeTux
Copy link
Author

Thanks for the response. I'll have try to find a fix. Bit since you're aware of the issue and don't know a solution, I'm doubt that I'll fine a solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-third-party Area: implementations of traits from other crates C-enhancement Category: an enhancement with existing code
Projects
None yet
Development

No branches or pull requests

2 participants