calva icon indicating copy to clipboard operation
calva copied to clipboard

REPL eval errors omit important info and are hard to parse

Open julienvincent opened this issue 1 year ago • 10 comments

I have found that the default error printing is very hard to parse and in many cases completely omits important information. As a result I have found myself having to manually eval *e every time I want to properly interpret a previous evaluation error.

Currently this is what is shown to the user when an eval throws: Screenshot 2023-01-23 at 11 03 01

Requiring the user to click the button in the output window in order to see: Screenshot 2023-01-23 at 11 03 17

In contrast, this is what is shown when evaluating *e: Screenshot 2023-01-23 at 11 04 27

In this example there are three main issues

  • The commented line is quite hard to parse as a human as the lack of syntax highlighting means I need to read the whole line to find the info I need. If this was highlighted my eye would naturally be drawn to the key pieces of information: 1) What is the class of the error and; 2) what is the cause
  • I need to manually click the Print stacktrace text in order to see the full trace. I'm not sure the value of hiding this information - I always want to see this information immediately. Especially as a keyboard-heavy user, needing to switch to my trackpad to find and click the button isn't ideal for me.
  • Most importantly, the information I really want is completely omitted. In this example in order to resolve the underlying issue I need to see the :data key in the error.

I have a draft PR coming in which proposes a new behaviour I would like to have in place to resolve this.

julienvincent avatar Jan 23 '23 12:01 julienvincent

Your suggestion is basically the old behaviour before we added the print stacktrace button, because most users didn't want it printed automatically. 😄

There is a command for printing the last stacktrace, so you don't need to mouse it. Maybe we could add a command that prints it less filtered?

PEZ avatar Jan 23 '23 12:01 PEZ

Personally I really don't want to have to run a command every time I get a repl eval error. I just want it to show me what I need to see.

Besides, printing the stacktrace only resolves 1 of the three raised issues. The most important being the omission of important data.

What about having a config option to control this behaviour? Something like "evaluationErrorPrintBehaviour": "compact|verbose" - where "verbose" does what I am proposing?

julienvincent avatar Jan 23 '23 13:01 julienvincent

The most important being the omission of important data.

That's what I meant with:

Maybe we could add a command that prints it less filtered?

Have you checked if you can get closer to your desired behaviour using Portal? It allows custom viewers, if that should be needed.

PEZ avatar Jan 23 '23 13:01 PEZ

I strongly disagree with showing the stack trace by default. We have worked hard to build a framework in the Clojure REPL to show a concise summary by default that includes the exception message, the "phase" (reading, compiling, macroexpanding, evaluation, printing, etc), and the location information.

In some cases, it may be useful to see the stack trace but throwing 100s of lines output when it isn't useful is ... not useful AND scrolls the actual information off screen. For example, any error that occurs during reading/compilation/macroexpansion will have a stack trace in the compiler. This stack trace is generally not useful to users and does not have any user code in it.

During eval or print, the stack trace may be useful but then you need to consider which part of the stack trace is useful. Exceptions may be caught and rethrown creating a chain of exceptions. The clojure.repl/pst function does not show the top exception but instead shows the root exception. It selectively elides the top frames of the root exception stacktrace to remove frames in the internals of Clojure so that the user's call is at the top of the stack. Even in this case, this can still be misleading due to macros (which rewrite the user's original code). I believe that Calva actually shows the top exception rather than the root exception under the "Print stacktrace" expander, which is almost never what I want (if anything, that's what I would like fixed).

Another way that stack traces and top location info can be misleading is seen in the example here - the use of helper functions to construct and/or throw the error. This is difficult to "fix" from the exception receiving side but there are things the calling side can do to improve the outcomes in this case.

Regarding showing ExceptionInfo by default, this is somewhat tricky and it's probably impossible to have a rule that always works. It is quite common for Clojure code to throw large ExceptionInfos that contain data useful for programmatic error handlng (like including request and/or response maps in a web handler). This can potentially be very large printed output, or even carry infinite sequences. This can overwhelm human readers and again obscure the actual problem, but also can be useful to dig into via *e. In clojure.main, we chose not to show it by default due to these problems.

My meta point here is - this issue requests a change in how error handling should be for everyone everywhere from a single example. That is not an approach that is likely to make anyone happy with the outcome. The Clojure repl has a bunch of helper functions that are available for tool makers (like ex-triage) and these can provide the Calva repl with structured information. In a text-only repl, there are limits to what you can do but in Calva or other tooling, this structured information can be used to power UIs with a lot better feedback for a user, and allow them to reveal that info progressively. If you want to make a change here, someone needs to consider a much wide range of errors and their information and build a framework for what to show and what controls to provide.

puredanger avatar Jan 23 '23 14:01 puredanger

@puredanger Thanks for the detailed comment! I recall reading somewhere advice around focusing on the error message before starting to dig in the stack trace. I don't recall where I read it, and couldn't find it earlier today when searching for it. Anyway, that has helped me a lot, the error message almost always has the info I need, and I only occasionally check the trace.

When checking the trace, I often feel that the way Calva prints it is not super helpful, but I lack a clear idea about how it should work better. Time for some research, and maybe a survey, it seems.

PEZ avatar Jan 23 '23 15:01 PEZ

There is also Custom REPL commands, that could be used to bind keyboard shortcuts to things like (clojure.repl/pst).

PEZ avatar Jan 23 '23 15:01 PEZ

Just sharing that I also find Calva’s stack traces as not very helpful, eg in test errors even the Print Stacktrace button doesn’t show me the info I want and I have to bail out to evaluate *e instead. So definitely worth revisiting this behavior.

I would defer to @puredanger for the details since the core team has done extensive work on this. I would personally love a way to get the root exception and a way to get the data of an ExceptionInfo without having to do further evaluations.

orestis avatar Jan 23 '23 20:01 orestis

The commented line is quite hard to parse as a human as the lack of syntax highlighting means I need to read the whole line to find the info I need. If this was highlighted my eye would naturally be drawn to the key pieces of information: 1) What is the class of the error and; 2) what is the cause

I do think we should probably not print that output commented (or maybe even other things we print that way). @PEZ Would there be an functional issue if we didn't print output like that as comments?

I also do not want Calva to print stack traces by default, but think it would be great to provide some improved experience using Clojure repl helper functions, as Alex mentioned, or otherwise.

bpringe avatar Jan 24 '23 04:01 bpringe

For reference:

  • https://insideclojure.org/2018/12/17/errors/
  • https://clojure.org/reference/repl_and_main#_error_printing

puredanger avatar Jan 24 '23 04:01 puredanger

@puredanger Thanks for the input.

Your reasoning for wanting to hide the stacktrace by default makes a lot of sense and I can see why this was done here.

Regarding showing ExceptionInfo by default, this is somewhat tricky and it's probably impossible to have a rule that always works.

Even if not shown by default I would think it makes sense to make this information available somehow. I think omitting the information entirely is worse than keeping it hidden by default but viewable similarly to how the stacktrace is handled now.

My meta point here is - this issue requests a change in how error handling should be for everyone everywhere from a single example. That is not an approach that is likely to make anyone happy with the outcome.

To be clear, I wasn't proposing that we make this behaviour the default for everyone but rather that we make this a configuration option and allow the user to decide how they prefer to have exceptions reported.

The config's default can certainly remain as it is now leaving the reporting behaviour unchanged unless explicitly overridden.


I understand that exception reporting is hard to do well in a general way and that to do it well will require a lot of thought and effort.

That being said I am sure there are some small changes we can make that can improve the current reporting experience without turning this into a big project just yet. Things like:

  • Adding syntax highlighting to the current reporting.
  • Having a way to show ExceptionInfo data similarly to the stacktrace.
  • Having some config (default off) to allow reporting errors more verbosely

julienvincent avatar Jan 24 '23 15:01 julienvincent