Clojure

Data Conveying Exception

Details

  • Type: Enhancement Enhancement
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: Backlog
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code and Test
  • Approval:
    Ok

Description

Problem: need to convey diagnostic information in an exception, and Java exceptions are poor for this because they don't have any place to put data. Having many apps write their own exceptions is unnecessary code bloat, and sometimes has compilation implications.

Choices made:

  • Naming it ExceptionInfo. Needs to tell the user that it carries data, without suggesting that there was a problem with data.
    • think we have to include Exception in the name, esp. for interop consumers.
  • Class of data member. IPersistentMap
  • Class of map input. IPersistentMap
  • Do not want to build anything to support conditional logic. (Use bindings for this).
  • Base class. RuntimeException is consistent with a dynamic language (don't want to impose checked exceptions).
  • print with data just after msg on first line
  • equality by identity (pretty much dictated by having exception as base class)
  • ex-info factory function
  • ex-data fn to get the data out
  • data and msg arguments required
  • data argument comes second
    • matches Java expectation of msg first, cause last
    • matches print order
  1. 0733-with-map-check.patch
    21/Oct/11 10:21 AM
    4 kB
    Alan Dipert
  2. 0733-with-ex-data-and-ex-info.patch
    16/Oct/11 6:06 AM
    4 kB
    Stuart Halloway
  3. 0733-ExceptionInfo.patch
    14/Oct/11 12:52 PM
    4 kB
    Stuart Halloway
  4. 0733-dec-2-edition.patch
    02/Dec/11 10:07 AM
    4 kB
    Stuart Halloway
  5. 0733-data-conveying-exception-2.patch
    28/Jan/11 2:47 PM
    4 kB
    Alan Dipert
  6. 0733-data-conveying-exception.patch
    28/Jan/11 2:14 PM
    4 kB
    Stuart Halloway

Activity

Hide
Alan Dipert added a comment -

Looks good; Stu's patch was missing a forward declaration to print-defrecord. 0733-data-conveying-exception-2.patch adds one, and all tests pass.

Show
Alan Dipert added a comment - Looks good; Stu's patch was missing a forward declaration to print-defrecord. 0733-data-conveying-exception-2.patch adds one, and all tests pass.
Hide
Rich Hickey added a comment -

Looks like is-a to me. Why is exception a map?

Also, bad practice to merge non-namespace-qualified keys with something you don't originate - should have ns. Actually, why merge at all? Why not let higher-level exception reporting handle cause and message?

Is there a convention for nesting these, merging data?

Show
Rich Hickey added a comment - Looks like is-a to me. Why is exception a map? Also, bad practice to merge non-namespace-qualified keys with something you don't originate - should have ns. Actually, why merge at all? Why not let higher-level exception reporting handle cause and message? Is there a convention for nesting these, merging data?
Hide
Kevin Downey added a comment - - edited

type checks for :type in metadata, might be nice if that worked on DCE's, maybe they should support metadata

Show
Kevin Downey added a comment - - edited type checks for :type in metadata, might be nice if that worked on DCE's, maybe they should support metadata
Hide
Stuart Halloway added a comment -

Latest patch per discussion on IRC http://clojure-log.n01se.net/#11:12b . Note that I had to touch string representation in three places (Java, main, and repl). I think main is only used for repl anyway, so a future cleanup could have main and repl share printing code.

Show
Stuart Halloway added a comment - Latest patch per discussion on IRC http://clojure-log.n01se.net/#11:12b . Note that I had to touch string representation in three places (Java, main, and repl). I think main is only used for repl anyway, so a future cleanup could have main and repl share printing code.
Hide
Rich Hickey added a comment -

Ctor arg order is a question.

Also, do we want to encourage making an exception w/o a message?

I think it should have been ex-data, in my last line on IRC, not ex-info

Will we have a fn to make these rather than encode the type via ctor call everywhere? Maybe that should be called ex-info. Same fn could be useful in cljs

Show
Rich Hickey added a comment - Ctor arg order is a question. Also, do we want to encourage making an exception w/o a message? I think it should have been ex-data, in my last line on IRC, not ex-info Will we have a fn to make these rather than encode the type via ctor call everywhere? Maybe that should be called ex-info. Same fn could be useful in cljs
Hide
Rich Hickey added a comment -

Also the description no longer describes the approach

Show
Rich Hickey added a comment - Also the description no longer describes the approach
Hide
Stuart Halloway added a comment -

Been back and forth on argument order, but I think msg/data/cause is best.

Show
Stuart Halloway added a comment - Been back and forth on argument order, but I think msg/data/cause is best.
Hide
Rich Hickey added a comment -

Only issue I think is whether you are going to enforce a map (and not nil) be passed. Currently you don't, making your doc string for ex-data wrong - it returns .getData of ExceptionInfo objects, which may be nil. If we enforce non-nil map on the way in, then ex-data works as documented and people can catch the exception and branch on ex-data. That seems most useful.

Show
Rich Hickey added a comment - Only issue I think is whether you are going to enforce a map (and not nil) be passed. Currently you don't, making your doc string for ex-data wrong - it returns .getData of ExceptionInfo objects, which may be nil. If we enforce non-nil map on the way in, then ex-data works as documented and people can catch the exception and branch on ex-data. That seems most useful.
Hide
Rich Hickey added a comment -

Also please mark as alpha, subject to change.

Show
Rich Hickey added a comment - Also please mark as alpha, subject to change.
Hide
Alan Dipert added a comment - - edited

Attached 0733-with-map-check.patch. Same as 0733-with-ex-data-and-ex-info.patch but added Alpha docstring and map check to ex-data per Rich's latest comment.

Show
Alan Dipert added a comment - - edited Attached 0733-with-map-check.patch. Same as 0733-with-ex-data-and-ex-info.patch but added Alpha docstring and map check to ex-data per Rich's latest comment.
Hide
Rich Hickey added a comment -

Thanks. The nil check should probably go in the ExceptionInfo ctor, just to have the same enforcement should they be created from Java.

Show
Rich Hickey added a comment - Thanks. The nil check should probably go in the ExceptionInfo ctor, just to have the same enforcement should they be created from Java.
Hide
Stuart Sierra added a comment -

I think this line in clojure.repl/pst,

(when-let [info (ex-data e)] (str " " info))

should be using pr-str instead of str, although str on a map already quotes string keys/values, so maybe it's good enough.

Show
Stuart Sierra added a comment - I think this line in clojure.repl/pst,
(when-let [info (ex-data e)] (str " " info))
should be using pr-str instead of str, although str on a map already quotes string keys/values, so maybe it's good enough.
Hide
Stuart Sierra added a comment -

Setting to 'Incomplete' pending response to 2 previous comments.

Show
Stuart Sierra added a comment - Setting to 'Incomplete' pending response to 2 previous comments.
Hide
Stuart Halloway added a comment -

The dec 2 edition does the arg check in the constructor and uses pr-str, addressing the last two comments raised.

Show
Stuart Halloway added a comment - The dec 2 edition does the arg check in the constructor and uses pr-str, addressing the last two comments raised.
Hide
Stuart Halloway added a comment -

The dec 2 edition does the arg check in the constructor and uses pr-str, addressing the last two comments raised.

Show
Stuart Halloway added a comment - The dec 2 edition does the arg check in the constructor and uses pr-str, addressing the last two comments raised.
Hide
Stuart Sierra added a comment -

Screened & moved to Approved Backlog.

Show
Stuart Sierra added a comment - Screened & moved to Approved Backlog.
Hide
Hugo Duncan added a comment -

ex-info and ex-data seem rather terse names to me (exception-info and exception-data would be clearer, imho). Clojure seems to have a mixture of abbreviated names and full names, but it was my vague impression that most of the abbreviations stemmed from traditional lisp or java usage.

Show
Hugo Duncan added a comment - ex-info and ex-data seem rather terse names to me (exception-info and exception-data would be clearer, imho). Clojure seems to have a mixture of abbreviated names and full names, but it was my vague impression that most of the abbreviations stemmed from traditional lisp or java usage.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: