layerware / hugsql

A Clojure library for embracing SQL
https://hugsql.org
Apache License 2.0
690 stars 54 forks source link

better error reporting when parsing queries #24

Closed yogthos closed 8 years ago

yogthos commented 8 years ago

I was translating some queries from yesql, and I ended up having a typo where the name key looked like -- :name:, it took me a little bit to track it down since the stack shows:

Caused by: java.lang.NullPointerException
    at clojure.lang.Symbol.intern(Symbol.java:59)
    at clojure.core$symbol.invokeStatic(core.clj:568)
    at clojure.core$symbol.invoke(core.clj:563)
    at hugsql.core$db_fn_map.invokeStatic(core.clj:463)
    at hugsql.core$db_fn_map.invoke(core.clj:454)

It would be good to validate the required metadata and error out explicitly if stuff like function name couldn't be parsed. Could even print the query string since it should be in context. :)

csummers commented 8 years ago

What, you don't like a good Clojure NullPointerException hunt? Ha, just kidding! I've actually run into this myself transitioning from Yesql. I'll add in a validation check for this and get it into the next release.

Thanks for the report!

yogthos commented 8 years ago

fantastic! :+1:

csummers commented 8 years ago

This is released in 0.4.3.

The strategy at this point is to give a little bit of context about which header keys it did find and to dump the sql template. Since the parser is generically picking up HugSQL headers (it doesn't expect :name or its variants, just :some-key), it doesn't catch this during the parse pass, so it checks for this right before the def pass. A later enhancement might set the reader line and column meta data for each statement, but the current fix should at least keep you from the dreaded NullPointerException.

yogthos commented 8 years ago

great, I think we can close this one unless you want to keep it here for the later enhancement part :)