🐛 Fixed missing models field not skipping serialization on GuardrailsConfig

[pmcjr]

This PR addresses #91, solved by (edit) adding validation to models field.

Added a test case regarding the issue as well.

[evaline-ju]

Thanks for the contribution! I don’t think this is quite the behavior we want to resolve the JSON deserialization error, since we ultimately still want to give the user an error showing that models is a required field.

I think the update I would expect is for an update of deserialization, since I would expect the same error “Failed to deserialize…” to still surface if any other required inputs are left off, but we want those errors to give a clear error in code/details like the other errors.

[declark1]

It sounds like the objective is to:

1. Simplify the error message, i.e. remove the excess "Failed to deserialize the JSON body into the target type:" bloat and just tell the user what is missing, e.g. "guardrail_config.input: missing field models at line 6 column 3"
2. Return our custom server::Error type which should be returned as JSON in the following form:

   {
       "code": 422,
       "details": "some error message"
   }

Axum features a Json extractor that handles deserializing/serializing the requests/responses via serde behind the scenes. When deserialization fails, it returns a JsonRejection error. In the case of a missing field, the JsonRejection::JsonDataError variant is returned, and if you check the Display impl for it here, you can see this is where the "Failed to deserialize..." message is set.

What we need to do is map this to server::Error and customize the message returned for JsonRejection::JsonDataError to only include the lower-level serde_json error message that tells us what is missing. There are a few ways to approach this (examples here), the simplest being to leverage axum_extra::extract::WithRejection to transform JsonRejection to our custom error type server::Error, add a JsonExtractorRejection variant to server::Error, and then update IntoResponse impl for the variant.

I went ahead and gave this a try here. The error message returned for a missing field is now:

{
    "code": 422,
    "details": "guardrail_config.input: missing field `models` at line 6 column 3"
}

If this is sufficient, feel free to steal this. If the message needs to be customized further, it will be a more complicated task. You may be able to downcast &dyn std::error::Error (returned from JsonDataError.source()) to the actual serde_json error to customize it more, but I haven't tried this. I would recommend this approach versus converting existing fields to Option and adding in additional validation logic.

Btw, here is a thread with some useful information for reference.

[declark1]

Just a couple of requested changes:

• Rebase latest changes from main
• You can drop ValidationError::Missing variant as it isn't being used

[declark1]

LGTM. Since Evaline is out and these changes should also address her comments, will approve and merge.

Addressed by changes