cnuernber
Feature request: Identifiable exceptions for parse errors
Charred is pretty amazing, thanks so much for making it.
One thing I noticed trying to use it is that there is no way (I can see) to distinguish exceptions from bad input from other Java exceptions, requiring me to catch all java.lang.Exceptions (which seems like a bad practice) if I don't want my app to die on bad input.
The below examples are all java.lang.Exception instances:
Some bad input exceptions at least include the string "JSON parse error" in the message:
user> (c/read-json "x")
Execution error at charred.JSONReader/readObject (JSONReader.java:397).
JSON parse error - Unexpected character - x
Other times though the message does not seem to have any consistent characteristic:
user> (c/read-json "{\"foo\" \"bar\" ")
Execution error at charred.JSONReader/readMap (JSONReader.java:343).
Map keys must be followed by a ':'
user> (c/read-json "{\"foo\": \"bar\" ")
Execution error (EOFException) at charred.JSONReader/readMap (JSONReader.java:348).
EOF while reading map: {"foo" "bar"}
Even if all the malformed input exceptions just had the string "JSON parse error -" in the message, that at least would be more narrowly catchable.
If you're open to PR I could give it a crack.
Anyway, just a thought, thanks again for the great library.
Great idea - I agree completely - I think both a library specific subclass and consistent string prefix both seem reasonable. I would love a PR!
Hi! Sorry for the delay, yes I was planning to get to it this weekend (tonight/tomorrow night). That said don’t let me stop you if you have a hankering to tackle it now.
Update - did not mean to close issue. Will reopen
Copying in my comments from the PR:
Feel extremely free to use this as a jumping off point and/or throw out. That said I can also make further changes (plenty of bandwidth this coming week).
Notes:
-Per your notes, went with library specific subclass, CharredException, with standardized string prefixes ("JSON parse error - ", "JSON encoding error - ", "CSV parse error - ").
-It seemed logical to go beyond just parse errors into encode errors as well (there were not many).
-I did not change EOFExceptions to CharredExceptions. I only re-classed plain Exceptions. I did add the standardized string prefixes to the messages of EOFExceptions where it seemed appropriate. My logic was that this is a specific enough subclass that people might already be testing for it - and also specific enough that the case for re-classing is weaker.
-I did not change any of the tests of the form (is (thrown? Exception (read-json "foo"))) to be specific to CharredException.
-I did not switch to unchecked exceptions (RuntimeException subclass) since it seemed you were intentionally using checked exceptions (Exception) - CharredException subclasses the latter.
-I did not get fussy about standardizing capitalization of the first letter after the message string prefix - it's possible people are already doing case insensitive matching on the old messages.
-I did not get fussy about making the use of periods at the end of the message strings consistent (because this seemed extremely fussy!).
-Alternative approach: Some might argue that all the CharredExceptions could just be standard IllegalArgumentExceptions, which would be more specific than what you had before without requiring the custom subclass (plus they would have the standardized string prefixes in the message).
-Alternate approach: Conversely, you could make a custom exception bestiary, including CharredEOFException, CharredParseException, and CharredEncodeException (or similar), which would allow a reclassing of some of the current EOFExceptions without the compatibility worries (since it could subclass java.io.EOFException).
I do not have strong opinions about any of the above, I just thought I'd note the issues/thoughts that came up.
Charred is a Clojure library though, so why not throw some (ex-info) and dispatch the handling of various errors based on ex-data?
If I remember correctly, there was some critique of the Java exception hierarchy by Rich Hickey in one of his seminal talks.
Ideally I would love to have the option of getting back https://github.com/cognitect-labs/anomalies -like maps, as opposed to throwing exceptions.
