Clojure

Data Conveying Exception

Details

  • Type: Enhancement Enhancement
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: Approved 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

Attachments

  1. Text File
    0733-data-conveying-exception.patch
    28/Jan/11 2:14 PM
    4 kB
    Stuart Halloway
  2. Text File
    0733-data-conveying-exception-2.patch
    28/Jan/11 2:47 PM
    4 kB
    Alan Dipert
  3. Text File
    0733-dec-2-edition.patch
    02/Dec/11 10:07 AM
    4 kB
    Stuart Halloway
  4. Text File
    0733-ExceptionInfo.patch
    14/Oct/11 12:52 PM
    4 kB
    Stuart Halloway
  5. Text File
    0733-with-ex-data-and-ex-info.patch
    16/Oct/11 6:06 AM
    4 kB
    Stuart Halloway
  6. Text File
    0733-with-map-check.patch
    21/Oct/11 10:21 AM
    4 kB
    Alan Dipert

Activity

Ascending order - Click to sort in descending order
Stuart Halloway made changes -
Field Original Value New Value
Attachment 0733-data-conveying-exception.patch [ 10095 ]
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 {{DataConveyingException}}. Needs to tell the user that it carries data, without suggesting that there was a problem with data. Still don't love the name.
** think we have to include Exception in the name, esp. for interop consumers.
* Class of data member. Maps are generic (same arguments apply here as for metadata). Using Java map interface (minus mutation) for signature friendliness to non-Clojure clients.
* Class of map input. Use immutable Clojure interface here, because we want to be strict about what we let people produce.
* 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).
* Composition instead of is-a. No need to reinvent anything here.
* private modifier on data
** normally would make immutable final things public
** since we can't force immutability, this seemed the best compromise
* Exposing the data already in an exception
** names {{cause}} and {{message}} conform to names in clj-stacktrace (possible future addition)
** *not* data-ifying the stack trace
*** this can be expensive and should be opt-in (separate ticket)

Possible extensions:

* Clojure convenience form for throwing {{DataConveyingException}}
* recast the other clojure.lang exceptions as derivees? 		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 {{DataConveyingException}}. Needs to tell the user that it carries data, without suggesting that there was a problem with data. Still don't love the name.
** think we have to include Exception in the name, esp. for interop consumers.
* Class of data member. Maps are generic (same arguments apply here as for metadata). Using Java map interface (minus mutation) for signature friendliness to non-Clojure clients.
* Class of map input. Use immutable Clojure interface here, because we want to be strict about what we let people produce.
* 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).
* Composition instead of is-a. No need to reinvent anything here.
* private modifier on data
** normally would make immutable final things public
** since we can't force immutability, this seemed the best compromise
* Exposing the data already in an exception
** names {{cause}} and {{message}} conform to names in clj-stacktrace (possible future addition)
** *not* data-ifying the stack trace
*** this can be expensive and should be opt-in (separate ticket)
* print like defecords do
* equality by identity (pretty much dictated by having exception as base class)

Possible extensions:

* Clojure convenience form for throwing {{DataConveyingException}}
* recast the other clojure.lang exceptions as derivees?
Approval Vetted Test
Waiting On alan.dipert
Patch Code
Hide
Permalink
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.
Alan Dipert made changes -
Attachment 0733-data-conveying-exception-2.patch [ 10096 ]
Alan Dipert made changes -
Waiting On alan.dipert stu
Hide
Permalink
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?
Stuart Halloway made changes -
Approval Test Incomplete
Christopher Redinger made changes -
Assignee Stuart Halloway [ stu ]
Hide
Permalink
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
Christopher Redinger made changes -
Fix Version/s Approved Backlog [ 10034 ]
Fix Version/s Release.Next [ 10038 ]
Waiting On stu
Hide
Permalink
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.
Stuart Halloway made changes -
Assignee Stuart Halloway [ stu ]
Fix Version/s Release 1.4 [ 10040 ]
Fix Version/s Approved Backlog [ 10034 ]
Attachment 0733-ExceptionInfo.patch [ 10393 ]
Approval Incomplete [ 10006 ] Screened [ 10004 ]
Hide
Permalink
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
Permalink
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
Stuart Halloway made changes -
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 {{DataConveyingException}}. Needs to tell the user that it carries data, without suggesting that there was a problem with data. Still don't love the name.
** think we have to include Exception in the name, esp. for interop consumers.
* Class of data member. Maps are generic (same arguments apply here as for metadata). Using Java map interface (minus mutation) for signature friendliness to non-Clojure clients.
* Class of map input. Use immutable Clojure interface here, because we want to be strict about what we let people produce.
* 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).
* Composition instead of is-a. No need to reinvent anything here.
* private modifier on data
** normally would make immutable final things public
** since we can't force immutability, this seemed the best compromise
* Exposing the data already in an exception
** names {{cause}} and {{message}} conform to names in clj-stacktrace (possible future addition)
** *not* data-ifying the stack trace
*** this can be expensive and should be opt-in (separate ticket)
* print like defecords do
* equality by identity (pretty much dictated by having exception as base class)

Possible extensions:

* Clojure convenience form for throwing {{DataConveyingException}}
* recast the other clojure.lang exceptions as derivees? 		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 first
** it is what the exception is about
Approval Screened [ 10004 ] Incomplete [ 10006 ]
Hide
Permalink
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.
Stuart Halloway made changes -
Attachment 0733-with-ex-data-and-ex-info.patch [ 10403 ]
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 first
** it is what the exception is about 		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
Approval Incomplete [ 10006 ] Screened [ 10004 ]
Patch Code [ 10001 ] Code and Test [ 10002 ]
Hide
Permalink
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
Permalink
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
Permalink
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.
Alan Dipert made changes -
Attachment 0733-with-map-check.patch [ 10493 ]
Hide
Permalink
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
Permalink
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
Permalink
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.
Stuart Sierra made changes -
Approval Screened [ 10004 ] Incomplete [ 10006 ]
Hide
Permalink
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.
Stuart Halloway made changes -
Attachment 0733-dec-2-edition.patch [ 10729 ]
Hide
Permalink
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.
Stuart Halloway made changes -
Fix Version/s Release 1.4 [ 10040 ]
Fix Version/s Backlog [ 10035 ]
Hide
Permalink
Stuart Sierra added a comment -

Screened & moved to Approved Backlog.

Show
Stuart Sierra added a comment - Screened & moved to Approved Backlog.
Stuart Sierra made changes -
Approval Incomplete [ 10006 ] Screened [ 10004 ]
Fix Version/s Backlog [ 10035 ]
Fix Version/s Approved Backlog [ 10034 ]
Hide
Permalink
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.
Rich Hickey made changes -
Approval Screened [ 10004 ] Ok [ 10007 ]
Stuart Halloway made changes -
Resolution Completed [ 1 ]
Status Open [ 1 ] Closed [ 6 ]

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4#649-r158309) | Report a problem
Powered by a free Atlassian JIRA open source license for Clojure. Try JIRA - bug tracking software for your team.