Clojure

improvements to exception messages and printing

Details

  • Patch:
    Code
  • Approval:
    Vetted

Description

Problems

  • Discussions about "make errors better" often conflate error message, stacktraces, exception data
  • Clojure prints data into Throwable messages
    • big and ugly
    • outside user control

Principles

  • exception messages should be small strings
    • with the expectation that getMessage will be printed
    • totally controlled by throwing code
    • never print arbitrary data into a message
  • flow data all the way to the edge
    • ex-info & ex-data
    • macroexpand spec errors have a well known format
  • edge printing should be concise by default, configurable by user
    • edge functions pst and repl-caught respect print and spec bindings
    • concise summary by default
    • all the details when you want

Proposed impl changes

  • stop printing data into message strings
    • ExceptionInfo.toString should list keys, not entire map contents
    • spec.alpha/macroexpand-check should stop including explain-out in message
  • and instead print data (configurably) at the edges repl/pst and main/repl-caught
    • print spec per explain-out
    • print ExceptionInfo keys
  • make CompilerException wrappers special at print time instead of construct time
    • CE message is "CompilerException + file/line"
    • wrapped message is whatever it is
    • edge printers should print both

Discussion

  • what should tools do?
    • whatever they want, Clojure should be out of their way after this change
    • can use repl/pst and main/repl-caught as a guide
  • what happens to be programs that parse exception messages?
    • probably should not do this
    • but Clojure's tests do
    • this proposal changes some message texts, they were never part of the contract

Patch Status

  • clj-2373-spec-alpha-1.patch - for spec
  • clj-2373-2.patch - for clojure
  1. clj-2373-2.patch
    12/Jul/18 12:12 AM
    7 kB
    Alex Miller
  2. clj-2373-clojure-1.patch
    09/Jul/18 11:59 AM
    5 kB
    Stuart Halloway
  3. clj-2373-spec-alpha-1.patch
    09/Jul/18 11:59 AM
    1 kB
    Stuart Halloway

Activity

Hide
Stuart Halloway added a comment - - edited

Output from a REPL session with the "-1" patches: https://gist.github.com/stuarthalloway/d3055d1975b464e8949b854e77a746b1

Show
Stuart Halloway added a comment - - edited Output from a REPL session with the "-1" patches: https://gist.github.com/stuarthalloway/d3055d1975b464e8949b854e77a746b1
Hide
jcr added a comment -

re "many Clojure tests that parse exception strings need to change":

Is it desirable to decouple the error message (as printed to the user) from the error tag/type/kind (for tooling, tests, etc), and make the latter a part of the contract?

Show
jcr added a comment - re "many Clojure tests that parse exception strings need to change": Is it desirable to decouple the error message (as printed to the user) from the error tag/type/kind (for tooling, tests, etc), and make the latter a part of the contract?
Hide
Alexander Yakushev added a comment -

I get the eliding map values from the output is meant to hide the ginormous Spec explanations. But printing only the keys doesn't have much value (pun intended). Is there possibly another solution? I can see two suboptimal ones:

1) Dissoc only the :clojure.spec.alpha/ keywords from the ex-data, Spec being the main offender.
2) Print ex-data with small print-length and print-depth (not sure whether that achieves anything, have to analyze different Spec errors to see if it does).

Show
Alexander Yakushev added a comment - I get the eliding map values from the output is meant to hide the ginormous Spec explanations. But printing only the keys doesn't have much value (pun intended). Is there possibly another solution? I can see two suboptimal ones: 1) Dissoc only the :clojure.spec.alpha/ keywords from the ex-data, Spec being the main offender. 2) Print ex-data with small print-length and print-depth (not sure whether that achieves anything, have to analyze different Spec errors to see if it does).
Hide
Stuart Halloway added a comment -

Thanks for the suggestions!

jcr: That might be desirable, but it is an expansion of scope and is not necessary to solve this problem.

Alexander: This is not just about spec errors, but all ex-data. The data is already present and accessible, and printing the keys gives the user a hint where to look.

Show
Stuart Halloway added a comment - Thanks for the suggestions! jcr: That might be desirable, but it is an expansion of scope and is not necessary to solve this problem. Alexander: This is not just about spec errors, but all ex-data. The data is already present and accessible, and printing the keys gives the user a hint where to look.
Hide
Alex Miller added a comment -

I have dug into the test failures more and there are really two things here...

1) the top level exception messages have changed (and are generally worse for the particular case of a CompilerException with nested init exception, which is not in the example set). I think this bears revisiting for those cases where you might be in a "Java calling Clojure" environment.

2) the message printed to the repl has generally improved, however most of the failing tests are checking exception message as a proxy for what the user sees in a particular error situation via thrown-with-msg?. It would be helpful if "what is printed" could be more easily extracted and if the tests were actually checking that, rather than checking the exception message - I think this better reflects the tests' intent.

I'm working on an updated patch.

Show
Alex Miller added a comment - I have dug into the test failures more and there are really two things here... 1) the top level exception messages have changed (and are generally worse for the particular case of a CompilerException with nested init exception, which is not in the example set). I think this bears revisiting for those cases where you might be in a "Java calling Clojure" environment. 2) the message printed to the repl has generally improved, however most of the failing tests are checking exception message as a proxy for what the user sees in a particular error situation via thrown-with-msg?. It would be helpful if "what is printed" could be more easily extracted and if the tests were actually checking that, rather than checking the exception message - I think this better reflects the tests' intent. I'm working on an updated patch.
Hide
Alex Miller added a comment - - edited

clj-2373-2.patch makes fewer changes in the CompilerException message (tests all continue to pass) but does more in the printing of exception chains in the repl, ignoring the computed message of the wrapper CompilerException completely and focusing more attention on the initial cause (but adding back in the source location info in the CompilerException wrapper if it's available).

user=> (/ 1 0)
ArithmeticException Divide by zero clojure.lang.Numbers.divide (Numbers.java:163)

user=> (throw (ex-info "oops" {:a 1 :b "foo"}))
ExceptionInfo oops clojure.core/ex-info (core.clj:4754)
ex-data keys: [:a :b]

user=> (let [a])
ExceptionInfo Call to clojure.core/let did not conform to spec., compiling:(3:1) clojure.core/ex-info (core.clj:4754)
In: [0] val: () fails spec: :clojure.core.specs.alpha/bindings at: [:args :bindings :init-expr] predicate: any?,  Insufficient input
Show
Alex Miller added a comment - - edited clj-2373-2.patch makes fewer changes in the CompilerException message (tests all continue to pass) but does more in the printing of exception chains in the repl, ignoring the computed message of the wrapper CompilerException completely and focusing more attention on the initial cause (but adding back in the source location info in the CompilerException wrapper if it's available).
user=> (/ 1 0)
ArithmeticException Divide by zero clojure.lang.Numbers.divide (Numbers.java:163)

user=> (throw (ex-info "oops" {:a 1 :b "foo"}))
ExceptionInfo oops clojure.core/ex-info (core.clj:4754)
ex-data keys: [:a :b]

user=> (let [a])
ExceptionInfo Call to clojure.core/let did not conform to spec., compiling:(3:1) clojure.core/ex-info (core.clj:4754)
In: [0] val: () fails spec: :clojure.core.specs.alpha/bindings at: [:args :bindings :init-expr] predicate: any?,  Insufficient input
Hide
Alex Miller added a comment -

Dumped some further design work at https://dev.clojure.org/display/design/Exception+handling+update

Still a WIP, take nothing here as final.

Show
Alex Miller added a comment - Dumped some further design work at https://dev.clojure.org/display/design/Exception+handling+update Still a WIP, take nothing here as final.

People

Vote (16)
Watch (11)

Dates

  • Created:
    Updated: