Add a human understandable error message for ig/ref, and ig/refset

[kwrooijen]

Problem

Currently when you write an invalid reference key you get a very cryptic message. This is especially cumbersome to new users of Integrant, since you might not know what a valid key is. You do get a stacktrace to the invalid key, but I believe this should be easier to figure out.

Solution

Add a custom error message to the :pre condition. I'm not sure if this is the correct solution but this is the desired result. Note that the stacktrace location (in this case config.cljs:34) is still available.

[weavejester]

This isn't a bad suggestion, but preconditions shouldn't be used in that way. I'll need to think on the best way to handle this.

[kwrooijen]

All right. Once you've come up with a better solution, please ping me so I can implement it.

[weavejester]

I do have one quick question: why is the current error message cryptic in your opinion? You might not immediately know what valid-config-key? does, but surely the next step is to check the docstring of the function?

[WhittlesJr]

+1. I'd love to see the misbehaving key right away.

[kwrooijen]

My intention is to create an lower entry level for new users. Giving instant feedback is a big plus when learning something new. If the end-user has to jump around, looking at source code, just to figure out what they did wrong, then that's (imho) a bad experience. Resulting in a higher bounce rate.

[weavejester]

I think I'd accept an when condition that throws an exception, as is the case in other parts of the code. Also try to match the style of other exceptions, and don't include a URL in the exception message.

[kwrooijen]

@weavejester applied your requested changes.

[weavejester]

I don't think these exceptions should be added to ref and refset, but rather to build with the other exceptions.

[kwrooijen]

Ahh I didn't notice at that the build function. I'll implement that tonight once I have more time.

[kwrooijen]

@weavejester Applied your change, I still have to remove the :pre conditions for ref and refset. This is unfortunate of you use ref or refset without build for whatever reason. I'm not sure how to otherwise fix this problem.

[weavejester]

That's a very good point. I wanted to keep the validation in one place, but I think it's unavoidable in this case, as I don't want to relax the constraints on ref and refset.

Let's revert the last change and put the errors back on ref and refset, but the error message should be worded in more neutral terminology, so we don't reference the configuration or imply that we're interested in any other keys. Something like:

(defn- invalid-ref-exception [ref]
  (ex-info (str "Invalid reference: " ref ". Must be a qualified keyword or a "
                "vector of qualified keywords.")
           {:reason ::invalid-ref, :ref ref}))

[weavejester]

Thanks! Can you change the commit message to something like:

Improve error message for ig/ref and ig/refset

[kwrooijen]

Done! Thanks for taking the time to look at my changes