shepmaster / snafu

Easily assign underlying errors into domain-specific errors while adding context
https://docs.rs/snafu/
Apache License 2.0
1.45k stars 61 forks source link

Support Serde for the built-in types #407

Open ash0080 opened 1 year ago

ash0080 commented 1 year ago

For example, Location. While it is possible to customize the serialization and deserialization process, it can become messy when it needs to be written in every place. It would be great if it could be a built-in feature.

shepmaster commented 1 year ago

Can you expand on what this will allow you to do that you cannot do today?

If this library serializes one specific Location as H4sIAPu7/2QAA6vmUlBQSsvMSVWyUlDSL8lI1U+tSEwu0S9ILMnQL8nXz8lM0isqVtIBKcvJzAMpszAxBHOT83NKc/OAAiamXLVcAF6603ZJAAAA, would that be sufficient for your case?

For example

You'll need to be specific about exactly which types would be useful.

ash0080 commented 1 year ago

Can you expand on what this will allow you to do that you cannot do today?

If this library serializes one specific Location as H4sIAPu7/2QAA6vmUlBQSsvMSVWyUlDSL8lI1U+tSEwu0S9ILMnQL8nXz8lM0isqVtIBKcvJzAMpszAxBHOT83NKc/OAAiamXLVcAF6603ZJAAAA, would that be sufficient for your case?

For example

You'll need to be specific about exactly which types would be useful.

Are you referring to the UTF-8 encoding issue when converting from OsStr to str? If so, I believe there are already several secure character conversion crates available, or you can implement a custom closure-based conversion interface.

Actually, here's the situation: I'm working on writing a Tauri app, so naturally, I would like to display as complete error information as possible when I activate the console. Instead of logging in both the Rust and JavaScript sides, I would prefer if all the exposable built-in types could be converted into JSON, if possible. I have looked at the source code, and it seems that many of them only require adding a #derive. However, it is currently a bit cumbersome to manually implement this outside the crate, and it makes the code appear much more chaotic.

shepmaster commented 1 year ago

Are you referring to the UTF-8 encoding issue

No, I am not. My point is that you asked for Location to implement Serialize and Deserialize. I could straightforwardly implement those traits such that the serialized form of Location is that long ugly string. However, my guess is that would not be acceptable to you, even though it met your stated request. That's why I'm trying to extract information and details.

could be converted into JSON, if possible

writing a Tauri app [...] Rust and JavaScript sides

Right, here's more context. You don't just want something that can be serialized and deserialized, but you want something that can be serialized to JSON with a fixed and known serialization because you want to deserialize it outside of Rust.

Although it'd be supremely annoying, I could make it so that every time you compile the library the serialized field names changed, which I'd guess would not be acceptable to you, but would still allow roundtrip serialization/deserialization in a single process.

only require adding a #derive

Serialization is easy to add thanks to derive macros, but that doesn't mean that long-term support for serialized data is any easier. Serialization is another form of external interface that has to be carefully managed. For example, is it acceptable for the serialized form to change in a semver-compatible version? Do people expect that you can serialize a type with SNAFU 0.0.1 and then deserialize it later with SNAFU 0.1.0? That JSON could easily be written into a file and then loaded in a year.

many of them

Again, please list exactly which types you are talking about.

shepmaster commented 1 year ago

From a more positive standpoint, Location specifically is interesting for this because I've already made all the fields public, so the names and types are already fixed (with respect to semver, at least). For this one specific type, optionally adding derive(Deserialize, Serialize) would not be a big problem, and the docs would need to say that the serialized form follows the same semver constraints as the Rust type.

Even today, you could achieve this today by creating your own Location that implements Serialize, Deserialize, and GenerateImplicitData, potentially just wrapping snafu::Location and delegating where appropriate.

ash0080 commented 1 year ago
#[snafu(display("{message}"))]
    Json {
        message: String,
        #[serde(serialize_with = "serialize_json_error")]
        source: serde_json::Error,
        #[serde(serialize_with = "serialize_location")]
        location: Location,
    },
    ...

    fn serialize_location<S>(x: &Location, s: S) -> Result<S::Ok, S::Error>
where
    S: Serializer,
{
    let mut state = s.serialize_struct("Location", 3)?;
    state.serialize_field("file", &x.file)?;
    state.serialize_field("line", &x.line)?;
    state.serialize_field("column", &x.column)?;
    state.end()
}

Yes, actually, this is a minor issue. I have carefully reviewed the documentation and it seems that the only type that may need to be exposed is Location. As for Backtrace, it would be even better if it could be serialized as an array. This way, it could potentially be combined with frontend code to achieve more pretty logs, such as if someone want to create a specific debug tool. However, in most cases, using strings should be sufficient.

shepmaster commented 1 year ago

As for Backtrace

This library doesn't really control Backtrace. Specifically, we have shim and inert implementations that we control but also allow directly re-exporting the backtrace or std crate's Backtrace type.

In a few versions, when the version support lines up, SNAFU will probably drop the shim and inert versions and just default to the standard library version and maybe allow swapping to the backtrace crate.

serialized as an array

For example, that's not yet stable.