roman / Haskell-etc

Declarative configuration spec for Haskell projects
MIT License
47 stars 7 forks source link

Reporting invalid input #37

Closed simonchatts closed 3 years ago

simonchatts commented 6 years ago

I've just started using this, and the experience so far has been great - thank you! A few onboarding observations, mostly around how invalid input is handled at runtime. There seems opportunity for making error messages more helpful for the end user:

Yaml files

If there's a problem parsing a user-supplied yaml file, then providing the filename in question would make for a more helpful error message. eg if an app update is incompatible with some option lurking in a pre-existing config file, then even the existence of that file probably isn't in the user's mind, so it won't be obvious that they should go look in that file to fix the problem. (It wasn't for me :)

CLI arguments

Similarly, if there's a problem parsing a CLI argument, then having the argument and metavar would help provide a more helpful error message at low developer effort. For example, suppose I have a config type including

data Verbosity = Debug | Info | Warning | Error deriving Generic

and just a generic JSON parser, but the user enters -v warn (rather than -v warning, which is what the generic decoder expects). Then the resulting message

Error in $.verbosity: The key "warn" was not found

is a bit cryptic. One could manually supply a JSON parser with better error message in each case. But assuming the provided metavar is already meaningful, one could imagine the generic infra creating an error (maybe via a helper function, rather than literally the show instance for the type) more like:

Couldn't understand argument "-v warn" (expecting "-v {debug|info|warning|error}"): The key "warn" was not found

This is at least piecing together all the relevant information already available.

ConfigurationErrors

This is a very low priority observation. But FWIW: with the app I migrated to use etc, ConfigurationFileNotFound is totally benign, not worthy of even a warning to the user, and all other ConfigurationErrors are totally fatal. It's obviously easy to filter accordingly. But the rest of the config load code is so compact, that I wonder if it might be worth something built-in (eg an annotation to etc/paths saying whether absence of any file should constitute an error) to make this even smoother. One day.

Other

I couldn't get the switch CLI type to work (assuming this is how you do something like grep -v, rather than grep -v true using an explicit bool), so an example would be handy.

roman commented 6 years ago

@simonchatts These suggestions are great feedback, thank you for mentioning them, I think it might make sense trying to build a few tickets from this ticket so that we can tackle all the suggestions one by one. Would you mind creating a ticket for each? as in, what the inputs are, what is happening and what is the expected behavior?

I'll try to flush out some of them tonight, but I would be wonderful if you could help me with some of those.

Cheers, and thanks for your feedback.

simonchatts commented 6 years ago

Of course! I'll split them out (probably will take a few days before I get a window of time). Thanks again.

simonchatts commented 3 years ago

Closing since no longer relevant.