← Back to context

Comment by kelnos

5 months ago

I started reading this a bit skeptically, but my main concern was addressed: this doesn't reimplement JSON deserialization; eserde's deserializer first uses serde_json's deserializer, and if it succeeds, it returns the result. So you don't have to trust someone else's deserializer that might not be maintained as well as serde_json is.

The downside to this approach is that if serde_json's deserializer encounters an error, they then have to deserialize the input again with their own deserializer (that wraps serde_json's) in order to get all the errors out. Fortunately the happy path is just as fast as using serde_json directly, but the unhappy path will be at least twice as slow as serde_json when it encounters an error. But I think this could be an acceptable trade off for some people.

What would be really nice, though, would be to change serde/serde_json/serde_${WHATEVER} so that it can do this (optionally, since even with built-in support, the failure case will still be slower and take up more resources). I expect it could be done in such a way to maintain API/ABI compat with the current state of things.

6 comments

kelnos

Reply
jrimbault  5 months ago

I wonder if the feature could be added to serde itself and gated behind a feature flag.

  • kelnos  5 months ago

    I'm sure it could be (possibly breaking the current serde API), but I think I'd prefer it be a runtime option. That way you can toggle it on and off for different uses of different APIs.

    For example, my service might deserialize in two different places: 1) when deserializing request bodies sent from clients, and 2) when deserializing response bodies when making API calls to other services.

    I might want to enable this feature for #1 (so I can return complete error messages to clients), but disable it for #2 (because I expect this issue to be rare, and don't want to incur a performance penalty when handling deserialization errors if remote API servers give me something I don't expect).

    • dietr1ch  5 months ago

      I think that each request needs some extension to ask for this if needed. Which would be enabled if there's a person typing data used to craft the request. If there's no human actively involved you can reject bad requests eagerly and save resources (and hopefully don't get hammered with bad requests)