ex_json_schema icon indicating copy to clipboard operation
ex_json_schema copied to clipboard

Feature request: make enum, properties, propertyNames and additionalProperties (or the actual schema) available for custom errors

Open norpan opened this issue 3 years ago • 4 comments

Hi! I'm using ExJsonSchema (version 0.8.0-rc1) to validate json configuration files, and in some cases (enum, additionalProperties) I would very much like to display the actually allowed values in the error message.

However, the custom error does not contain the information needed. It does contain the path but it's a bit complicated to get the actual schema fragment from the path, since it may go through many layers of references. Or is there a simple way to get the schema?

So I'm thinking either we can make the Enum, PropertyNames, and AdditionalProperties include the allowed enums and properties/property names/pattern properties, or we can more generally make the schema fragment for which validation failed available.

Just extending the specific errors seems easier. It only involved a small change in the validation modules to include the part of the schema in the error. If you want I can provide a pull request (I did it in my source tree).

However, maybe it's better to extend the generic error to have a :fragment field in addition to the :error and :path fields. That way we can choose for ourselves how to format errors based on the actual schema fragment (even including using custom properties in our json schema). It will also make it easier to make good custom errors for oneOf etc.

Any one of these changes should be pretty backwards compatible since they are just adding fields to the error structs, and those are only read, not written, by user code.

norpan avatar Apr 28 '21 12:04 norpan

Yes, that sounds like a requirement that should be covered somehow. I like the idea of including the fragment in the error struct, but the issue could be that the fragment is just a reference, which doesn't really help you when trying to figure out what actually failed the validation. I will spend some time on it next week to see if I can come up with a good solution. Maybe including the actual validation failures in the specific error structs is a better way to cover all eventualities.

jonasschmidt avatar Apr 29 '21 15:04 jonasschmidt

After trying out a few things, I think it might be better to include the current fragment in the %ExJsonSchema.Validator.Error{} struct like you proposed. There's a branch where I had a first go at it and added the fragment everywhere. Tests are still red, but maybe you can try it out to see if it would cover your use case. Then I would have to see if/how to cover some potential edge cases with nested refs etc.

jonasschmidt avatar May 06 '21 14:05 jonasschmidt

I had a quick look, and I was more thinking that the fragment should be the whole object, not just the specific condition. So if you had a minimum constraint for instance, and it was violated, you'd get the whole object with {"type": "integer", ..., "minimum": 2}. Otherwise you don't really gain much by adding the fragment, I think, since it will already be in the specific error. My reasoning for adding the whole fragment was also so that you could add custom "annotations" to the fragment that could be used to give better error message, you could use the "title" and "description" fields. etc.

Obviously refs will make this harder, but any refs should be resolved at this stage, right? Perhaps it should be iterated a few times if this is to be a feature.

However, I think it should be pretty straightforward to add enum: enum, to the Error.Enum and properties/propertyNames/patternProperties to the Error.AdditionalProperty. I did this for the specific cases I have in my code (only enum and properties). If you want a pull request I can provide one.

norpan avatar May 06 '21 17:05 norpan

Ok, maybe just open a PR so I can have a look at the proposed changes. Adding more context to the fragment would definitely be more work, because the outer context is not really known in the individual validators. That's something that would have to be passed everywhere. If you feel up to it you could also have a go at it and see how that would work.

jonasschmidt avatar May 11 '21 14:05 jonasschmidt