Adding use of .getMessage for reporting dict-load exceptions

[JohnPostlethwait]

The current error handler for dict-load was masking the Clojure error messages, which typically tell you exactly is wrong with the map parsing. (Duplicate keys, unmatched brackets, etc...) This PR now uses .getMessage to show the original message of the exception.

[JohnPostlethwait]

Thoughts?

[ptaoussanis]

Hi John, have been travelling + just got back; going through issue backlog by priority.

On first look, not sure why this'd be necessary/helpful? The original exception is already attached as a cause. All original exception info (incl. the exception message) should be available, no?

[JohnPostlethwait]

The exception only prints the trace, which ends at this file, it doesn't tell you the line number in the dictionary file it had an issue parsing. It also obfuscates the original error message string.

Often in my cases, especially with merge conflicts, there are duplicate keys, or erroneous brackets, and the message stating what the issue is is hidden without this change.

[ptaoussanis]

Any idea why you wouldn't be seeing the attached causing exception? Maybe a tooling issue?

[JohnPostlethwait]

Not that I can think of, the trace comes directly from Tower without anything catching it and messing with it.

The error message obfuscation is pretty easy to reproduce, just create a duplicate key in a dict file and have Tower try and load it, you get a message like this:

clojure.lang.ExceptionInfo : failed compiling file:src/hg/client/i18n.cljs
clojure.lang.ExceptionInfo : Condition failed in `taoensso.tower:542` [pred-form, val]: [(map? (load1 dict)), clojure.lang.ExceptionInfo: Failed to load dictionary from resource: dict.clj {:dict "dict.clj"}] at line 11 src/hg/client/i18n.cljs
java.lang.AssertionError : Condition failed in `taoensso.tower:542` [pred-form, val]: [(map? (load1 dict)), clojure.lang.ExceptionInfo: Failed to load dictionary from resource: dict.clj {:dict "dict.clj"}]
Error on file src/hg/client/i18n.cljs, line 11, column 25

My change modifies the above error to actually show the message, which is:

clojure.lang.ExceptionInfo: Duplicate key: :a {:type :reader-exception, :line 1, :column 12, :file "dict.clj"}

That useful message isn't shown without the change, leaving one to scour their dict file wondering where the duplicate key is.

[JohnPostlethwait]

Sorry for the late reply, GitHub emails me replies maybe half of the time and I hadn't realize you responded.

[ptaoussanis]

No problem :-)

Not that I can think of, the trace comes directly from Tower without anything catching it and messing with it.

Wasn't suggesting that your tooling is modifying the exception, but that your tooling should be displaying the exception in more detail.

To clarify: the exception being generated already includes all of the information that you'd like, your tooling is just choosing for some reason not to print it.

One way of solving this (your PR) is to include some adhoc info (message in this case) from the causing exception in the wrapping exception's data.

Alternatively, you (/your tooling) can just extract/display the wrapped exception. I'm suggesting that the latter approach is the correct pattern and ultimately something that you'll probably want to resolve since this PR won't help with all the other wrapped exceptions that you're likely to encounter in other code.

(throw (Exception. "outer" (Exception. "inner"))) provides the inner exception to your tooling, which should display it. Cider and the Clojure printer both do by default, I believe.

Does that make sense?

[JohnPostlethwait]

It does! Thank you!

I will confirm in a more sandboxed location than my current application stack and then try and get our printer to also do it like you mentioned...