Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

better error reporting when parsing queries #24

Closed
yogthos opened this issue Feb 24, 2016 · 4 comments
Closed

better error reporting when parsing queries #24

yogthos opened this issue Feb 24, 2016 · 4 comments

Comments

@yogthos
Copy link
Contributor

yogthos commented Feb 24, 2016

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
Copy link
Member

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
Copy link
Contributor Author

yogthos commented Feb 24, 2016

fantastic! 👍

@csummers
Copy link
Member

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
Copy link
Contributor Author

yogthos commented Feb 25, 2016

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants