better error messages

[carocad]

Hey guys,
I was checking around what I thought was normal behavior for clojure and found out that it is probably a Lighttable thing. My problem is that any error that occurs in Lighttable creates a stack trace which is unreadable. For example:

(filter #() [1 2 3 4])

on the REPL produces:

ArityException Wrong number of args (1) passed to: core/eval21331/fn--21332  clojure.lang.AFn.throwArity (AFn.java:429)

Whereas on my Lighttable produces:

clojure.lang.ArityException: Wrong number of args (1) passed to: core/eval5308/fn--5309
             AFn.java:429 clojure.lang.AFn.throwArity
              AFn.java:32 clojure.lang.AFn.invoke
            core.clj:2601 clojure.core/filter[fn]
          LazySeq.java:40 clojure.lang.LazySeq.sval
          LazySeq.java:49 clojure.lang.LazySeq.seq
              RT.java:484 clojure.lang.RT.seq
             core.clj:133 clojure.core/seq
        core_print.clj:46 clojure.core/print-sequential
       core_print.clj:147 clojure.core/fn
         MultiFn.java:231 clojure.lang.MultiFn.invoke
            core.clj:3392 clojure.core/pr-on
            core.clj:3404 clojure.core/pr
             AFn.java:154 clojure.lang.AFn.applyToHelper
          RestFn.java:132 clojure.lang.RestFn.applyTo
             core.clj:624 clojure.core/apply
            core.clj:4373 clojure.core/pr-str
          RestFn.java:408 clojure.lang.RestFn.invoke
              eval.clj:68 lighttable.nrepl.eval/clean-serialize
          RestFn.java:423 clojure.lang.RestFn.invoke
              eval.clj:81 lighttable.nrepl.eval/->result
             AFn.java:156 clojure.lang.AFn.applyToHelper
             AFn.java:144 clojure.lang.AFn.applyTo
             core.clj:626 clojure.core/apply
            core.clj:2468 clojure.core/partial[fn]
          RestFn.java:408 clojure.lang.RestFn.invoke
            core.clj:2559 clojure.core/map[fn]
          LazySeq.java:40 clojure.lang.LazySeq.sval
          LazySeq.java:49 clojure.lang.LazySeq.seq
              RT.java:484 clojure.lang.RT.seq
             core.clj:133 clojure.core/seq
            core.clj:2595 clojure.core/filter[fn]
          LazySeq.java:40 clojure.lang.LazySeq.sval
          LazySeq.java:49 clojure.lang.LazySeq.seq
             Cons.java:39 clojure.lang.Cons.next
              RT.java:598 clojure.lang.RT.next
              core.clj:64 clojure.core/next
            core.clj:2856 clojure.core/dorun
            core.clj:2871 clojure.core/doall
             eval.clj:125 lighttable.nrepl.eval/eval-clj
          RestFn.java:442 clojure.lang.RestFn.invoke
             eval.clj:191 lighttable.nrepl.eval/eval2119[fn]
             AFn.java:152 clojure.lang.AFn.applyToHelper
             AFn.java:144 clojure.lang.AFn.applyTo
             core.clj:624 clojure.core/apply
            core.clj:1862 clojure.core/with-bindings*
          RestFn.java:425 clojure.lang.RestFn.invoke
             eval.clj:176 lighttable.nrepl.eval/eval2119[fn]
             eval.clj:175 lighttable.nrepl.eval/eval2119[fn]
         MultiFn.java:227 clojure.lang.MultiFn.invoke
              core.clj:97 lighttable.nrepl.core/queued[fn]
            core.clj:2402 clojure.core/comp[fn]
interruptible_eval.clj:159 clojure.tools.nrepl.middleware.interruptible-eval/run-next[fn]
              AFn.java:22 clojure.lang.AFn.run
ThreadPoolExecutor.java:1145 java.util.concurrent.ThreadPoolExecutor.runWorker
ThreadPoolExecutor.java:615 java.util.concurrent.ThreadPoolExecutor$Worker.run
          Thread.java:745 java.lang.Thread.run

I don't know if this behavior is only on my Lighttable version but I guess not.
On the other hand I saw that you use clj-stacktrace in nrepl/exception.clj which is a user specific code. Have you checked clojure.stacktrace from Clojure itself?
In tryclojure they use it and it is extremely simple; you can see the relevant lines here

Let me know your thoughts about it

[kenny-evitt]

@carocad What REPL are you using? Whatever it is, it's certainly outputting a briefer 'stacktrace' tho it only seems marginally more informative (to someone not familiar with Clojure).

That said, I think recent versions of Clojure have gotten some modestly improved stacktraces and error messages – maybe that's in clojure.stacktrace. So I imagine that we'd be open to replacing whatever library we're using now with something better. I'll gladly test out whatever you think might be a suitable change; submit a PR with the changes and I'll take a look at it.

[rundis]

I would kind of preferred if we looken into what it would take to integrate something like
https://github.com/AvisoNovate/pretty

Preferably with the proper formatting (and if possible with coloring, which probably won't be all that trivial... )

[kenny-evitt]

@rundis That library looks fantastic.

[carocad]

@kenny-evitt I'm using

• REPL-y 0.3.5
• nREPL 0.2.6
• Clojure 1.6.0

I will try to hack my clojure plugin to see if I can change the nrepl exeption formatting that we currently have.

@rundis that's great, but I don't see why we should choose one over the other. From my point of view it would be better to have levels of stacktrace. For the beginner it would probably be better to have a very simple error message like the one from clojure.stacktrace whereas some advance users might want to have more details about it like the ones in aviso/pretty. From what I see in the aviso/pretty source code it is perfectly possible to pass a normal stacktrace and get the pretty formatted one.

[kenny-evitt]

@carocad With regard to pretty, while it provides for ways to customize what exceptions are elided in the output, the defaults seem pretty reasonable:

write-exception flips around the traditional order, providing a chronologically sequential view:

• The stack trace leading to the root exception comes first, and is ordered outermost frame to innermost frame.
• The exception stack comes after the stack trace, and is ordered root exception (innermost) to outermost, reflecting how the stack has unwound, and the root exception was wrapped in new exceptions and rethrown.

The stack trace is carefully formatted for readability, with the left-most column identifying Clojure functions or Java class and method, and the right columns presenting the file name and line number.

The stack frames themselves are filtered to remove details that are not relevant. This filtering is via an optional function, so you can define filters that make sense for your code. For example, the default filter omits frames in the clojure.lang package (they are reduced to ellipses), and truncates the stack trace when when it reaches clojure.main/repl/read-eval-print.

Repeating stack frames are also identified and reduced to a single line (that identifies the number of frames). This allows your infinite loop that terminates with a StackOverflowException to be reported in just a few lines, not thousands.

I can't tell from the REPL-y README or its project.clj file, what it's using that would improve its stacktraces. In fact it seems to be using clj-stacktrace too.

Or are you somehow using clojure.stacktrace instead?

[rundis]

Correct me if I'm wrong, but what I think @carocad is trying to get at is that rather than always printing the entire stacktrace, it should be optional if you wish to see just a summary (message/root cause) or the entire stacktrace (elided with clj-stacktrace or pretty).

The current behaviour of Light Table is to always shows the entire stack on the line below the when showing inline-errors. It also shows the root-cause/message in the statusbar, but that disappears after about 5 seconds (if you blinked you missed it sorta'). The statusbar message is not far off the message that @carocad implies he would want to be shown inline I think.

This is the relevant behavior that shows the exception on the cljs side of things in the Clojure plugin:
https://github.com/LightTable/Clojure/blob/master/src/lt/plugins/clojure.cljs#L384

So it would be really neat if inline-error were expandable (like inline results are), such that default it shows just the root-cause, and if you click on it, it expands to also show the full details).

I see several different implementation alternatives with different scope:

A) Simplest option - User configurable

Make it a user setting to show either the stacktrace or just the root-cause in the above mentioned cljs behavior in the clojure plugin.

B) Expandable inline exception

Show root cause by default, allow user to expand inline exception to show details.
To handle this we need to either create a new inline-exception widget to allow for this or we need to modify the default inline-exception widget (https://github.com/LightTable/LightTable/blob/master/src/lt/objs/eval.cljs#L395).

If we change the default inline-exception widget, it would obviously have to be backwards compatible since it's use from many places and many different plugins.

C) Expandable inline exception with better stacktrace formatting

Similar to B, but maybe swap clj-stacktrace for pretty. That means making changes to the lt nrepl middleware as well obviously

[carocad]

@kenny-evitt I actually have no idea which part is the one that is trimming the stacktrace. I almost always work in LT so I didn't bother to check it until I figured out that tryclojure had such nice error messages; which later led me to clojure.stacktrace

@rundis is completely right. My initial thought was only the simplest option (user configurable) but your other options do look very appealing.

I'm willing to work on this if somebody can take the time to time to help me around on complicated (not-documented) stuff. Correct me if I'm wrong but all those changes would imply changing the lt nrepl middleware here right?

[cldwalker]

Hi @carocad. Having no stacktrace isn't clearer or helpful for more experienced users and makes debugging near impossible for all users. I would rather not have that be a configurable option for users. I'm open to options B or C that @rundis proposed. It's worth noting that with behaviors and the User plugin you can implement no stacktrace for yourself without having to modify the Clojure plugin:

• In your user.cljs (open with command "User script"), write a version of cljs-exception behavior e.g. my-cljs-exception

• In your user.behaviors:

  [:editor.clj :-lt.plugins.clojure/clj-exception]
  [:editor.clj :lt.plugins.user/my-clj-exception]

[carocad]

@cldwalker oh this is awesome! . Sorry for my ignorance but I was not aware that I could combine things in this manner. Thanks so much for the advice on implementing my own exception behavior.
On the other hand, I think you misunderstood my intentions. I don't want to take away the stacktrace, I just think that much of the information shown in just not useful. I also agree that rundis options B and C are much better than my initial idea.

Thanks to the info that you gave me I was able to modify the exeption a little bit and figure out a tiny way to improve its current behavior without much of a change. I will submit a pull request about it.
Let me know if somebody is interested in helping me to implement options B or C, I think we can all benefit from that.

[carocad]

Hey guys,
I managed to put together an "implementation" of option B. You can check it here: https://gist.github.com/carocad/7b17d71443e842938edc

I wrote "implementation" because it is in fact an almost exact copy of inline.result. The differences are only its display (text and background color) and its menu behavior. For brevity I put everything in the same gist. The intention was mainly to show it. I have tested this behavior on my Lighttable and it works fine but to be honest I don't understand completely the code used for both the inline.exception and the inline.result so I don't know if implementing it this way could cause any colateral effect. Thus I would like you to check it out.

I could also create a plugin for it but I would prefer this to be the default exception-display behavior of clojure. Let me know what you think

[rundis]

Cool !

Do you have screenshot of how it looks truncated and expanded ? I have a feeling that the expanded view might be lacking in terms of formatting ? I think maybe you need to use a pre tag or something to keep formatting. However that comes add odds with the truncate feature.

A better solution might be allow the new exception display behaviour to take a summary and details argument. Details could be hidden default, and when clicking, details becomes visible (simple css visibility toggle ?).

One of the reasons that exceptions are shown underline (rather than to the right), is that stack traces tend to be quite wide. Actually the current display of stack traces doesn't work well with narrow tab sets (no scroll). I think it would be best too keep exception display as an underline result.
The underline result stuff basically adds a codemirror line-widget

A summary of thoughts going forward:

I think I'd prefer if the new behavior for adding a toggle-able error display be added to eval.cljs. This way future plugins (or upgrade of existing plugins) may start to gradually use the new behavior whilst old plugins can happily work with the old behavior.
The downside to this approach is that the Clojure plugin can't start using it until Light Table is released.

@carocad would you be willing to submit a pull-request to LT core for a new error display behavior (taking into account the considerations mentioned above) ?

@cldwalker agreed ?

[carocad]

Hey @rundis here are the screenshots of the collapsed and expanded exception. The lines are displayed with the correct formatting except those that are longer than the exception box; those end up in several (unformatted) lines, as you can see in the screenshoot.

I would gladly submit a pull-request with the new behavior but as I said, I don't have a good knowledge of what is the complete inline-result behavior/object code doing, thus I would prefer if @kenny-evitt or @cldwalker take a first look at it before trying to properly style it.

[cldwalker]

I think I'd prefer if the new behavior for adding a toggle-able error display be added to eval.cljs. This way future plugins (or upgrade of existing plugins) may start to gradually use the new behavior whilst old plugins can happily work with the old behavior.
The downside to this approach is that the Clojure plugin can't start using it until Light Table is released.

@carocad would you be willing to submit a pull-request to LT core for a new error display behavior (taking into account the considerations mentioned above) ?

@cldwalker agreed ?

@rundis I'm open to this feature as long as we make this an opt-in behavior and preserve existing behavior as the default - full stacktrace is viewable. Once we use it more and possibly get feedback from other users, we can see if it makes sense to change the default behavior. As for where we want the new widget to live, I'm ok w/ either LT or the plugin. It may be helpful to have it live in the plugin first for quicker iteration and to work out any bugs but it's up to you

[rundis]

@cldwalker You've convinced me :-) Let's add it to the Clojure plugin only for now.

@carocad Let's do as follows;

• Put the new exception display widget stuff in a separate namespace in the Clojure plugin
• Add a new behavior (below) https://github.com/LightTable/Clojure/blob/master/src/lt/plugins/clojure.cljs#L384 that also triggers on :editor.eval.clj.exception.
  • However this raises the behavior to show the toggle-able inline exception display. (ref https://gist.github.com/carocad/7b17d71443e842938edc#file-proposal-cljs-L58, you should give it a different trigger, something like :editor.collapsible.exception)
  • This behavior is not wired up in the clojure.behaviors file (hence it's turned off by default)
• Add a section in the readme on how to turn off the default behavior and how to turn on the new
• Optional extra: Also make it work for ClojureScript (same procedure as for clj, but a parallell behavior to https://github.com/LightTable/Clojure/blob/master/src/lt/plugins/clojure.cljs#L396) !
• I'll try to help out explaining what's what to help you along the way

[carocad]

@rundis
according to the suggestions that you made I modified the code to:

• display the exception underline not to the right
• accept a summary and the full stacktrace
• avoid the truncating the summary (I'm not fully sure how long can those be so I just modified the max-width; I couldn't get it to work otherwise :/ )

Here you can find the new code: https://gist.github.com/carocad/7b17d71443e842938edc

Here are some screenshots of the new behavior:

If this is ok like that I would proceed to create the new namespace and submit the patch.

[rundis]

@carocad Sorry for the late reply
This looks like a great start. Go ahead and make a PR to Clojure and we'll take it from there !

[carocad]

oh I just discovered the first bug in this implementation. It turns out that result-mark fixes max-height to 50% so it need to be changed to "initial" to avoid hidden parts of very long stacktraces. The change is minimum:

[:span {:class (bound this #(->collapse-class % summary))
           :style "background: #73404c; color: #ffa6a6;
                   max-width:initial; max-height:initial"}

[carocad]

@kenny-evitt @rundis @cldwalker is anyone interested in mentoring the development of aviso/pretty as the standard stacktrace tool for Clojure's plugin?

I am willing to work on it but I don't have as much experience as you guys so, some help would be much appreciated.

[carocad]

hey guys @kenny-evitt @rundis,
as I mentioned on gitter I thought that using aviso/pretty could be pretty trivial and I was right :)

Here is a gist with the change proposal to replace clj-stacktrace with aviso/pretty. I think that all functionality would remain the same since I only touched the stacktrace-string parsing part.

Nevertheless I would like to know your opinion about it, you know more about it than me anyway ;)

Here is a snapshot of the result. PS: I avoided putting colors to the output as that would require quite some more effort and possibly create a dependency between how are stacktraces parsed in lein-light-nrepl vs how are they displayed by the clojure plugin

[carocad]

It seems I was wrong and putting colors on the output was not that difficult as I thought at the beginning. I avoided stripping away the ansi codes from the stacktrace (here) and I created a parser from ansi-code to hiccup, which did the job perfectly :). The parser can be integrated with the previous collapsible exception behavior without any troubles (see specific lines here)

Here is a screenshot of the it working

[kenny-evitt]

@carocad Fantastic! I'm not ecstatic about those particular colors. Could you look at trying to match them with existing colors (assuming we have defined colors to which you can match)?

As-is tho, there's no reason not to create a pull request with your changes. I think everyone would be happy commenting on your code that way.