Add what validation failed to `NimbleOptions.ValidationError`

[anthonator]

I'd like to be able to perform some additional processing when validation fails for specific reasons. This is difficult to do considering there's no way to know what caused the validation error beyond the error message.

I could see a :validation field on NimbleOptions.ValidationError that would have a value of :required, {:type, :string}, etc.

[whatyouhide]

Okay, to do this we would need a possibly significant overhaul of how we represent errors internally in nimble_options.

Right now, errors are free-form strings. This can be problematic for things like #132 for example, because the string is unstructured. Instead we could:

• Make error "types":

  {:invalid_type, expected_type, value} | {:invalid_length, ...}

  These would still be internal to nimble_options.
• Then, we add some sort of error_message(error_type) function to turn those into strings, which we do only at the last moment before printing them out.

This is a poor implementation of exceptions and Exception.message/1 maybe, but I don't want to create an exception module for each possible error type 😉

If you want, you could give this implementation a try—and ask any questions if you have them. However, I’m not so sure it will make sense in nimble_options as it has to keep one property: nimbleness 😄 I would like to maintain this lib on the smaller side.

@josevalim btw any thoughts on this? I’m open to the idea above because I think it could make the lib easier to maintain and contribute to in the future (plus, types!).

[josevalim]

It feels we could tackle this problem by adding some metadata to the ValidationError struct without changing the error message? The only issue is that validation errors can be nested (for example, a nested keyword, and tracking this whole "path" may be complicated - although still doable with a list).

[whatyouhide]

Adding metadata to the error struct and leaving the error message alone is another valid approach yeah but I think it would end up being more work than turning the reason into a structured type and then converting that into a message when needed?

[josevalim]

Another option is to allow them to be MFAs or {&fun/1, metadata}, and then folks can match on the metadata bits they find interesting.

[whatyouhide]

Ah yeah you are correct that the predefined error reason (:required, or :mismatching_type, or whatever) are only the built-in ones but we also have totally custom validation.

So I think we have two possible directions:

1. We go with the "types" option. This is gonna work great with the type system 😄 This option would mean something like: errors become structured. If we make these exceptions, then users can extend them by returning {:error, Exception.t()} from validation functions and that way we can always turn anything into a message. I love this.
2. We go the opposite direction: we keep the messages as strings everywhere but we introduce the concept of free-form metadata everywhere. So we add metadata (:required and friends), users can use {:custom, mod, fun, args, metadata} as a type, or even have :validation_metadata as a top-level key in the nimble_options schema.

Preferences?

[josevalim]

@anthonator which kind of validations do you want to know about? If it was required or a type violation?

@whatyouhide Are people supposed to use NimbleOptions for anything else? In an ideal world, this project is dead once the type system lands (either because it is not needed or because it is part of core).

[anthonator]

2. We go the opposite direction: we keep the messages as strings everywhere but we introduce the concept of free-form metadata everywhere. So we add metadata (:required and friends), users can use {:custom, mod, fun, args, metadata} as a type, or even have :validation_metadata as a top-level key in the nimble_options schema.

The freeform metadata idea was something I was actually going to suggest in the issue description. Even with the {:custom, mod, fun, args, metadata} type. I was envisioning a :private (or something to that effect) field on NimbleOptions.ValidationError for this. Wish I had now 😄 Regardless, with two people coming to this conclusion independently it seems like it might be a good approach.

Regarding 1, I'm not exactly sure what you mean by "type system". I'm not sure if you mean the new Elixir type system or maybe something you've been discussing internally for NimbleOptions. So I can't really comment on that approach. However, you seem excited about it and that kind of gets me excited about it 😄

[anthonator]

@josevalim for my specific use case I really just want to know if the error was due to :required or :type. If it was :type then what the option's type is as well.

It might also be useful to add a :type field to NimbleOptions.ValidationError for use beyond this feature as well. If that makes sense to you I can open a new issue to discuss.

[josevalim]

That sounds straight-forward to me and probably how I would tackle this (i.e. solving this specific problem), but once again, it gets trickier once we consider nesting.

[anthonator]

Why does this feature need to worry about nesting given that NimbleOptions.ValidationError has keys_path which tracks where in the nested options the error occurred.

[josevalim]

I forgot about keys path. So adding a field that has value of either :required or {:type, type} is good enough to me.

[whatyouhide]

Agreed, let's solve the specific problem for now without going nuts. 😄

@anthonator wanna make a PR?

[anthonator]

Heck yeah!

I won't be able to start on this until after #133 is resolved.