<< Back to previous view

[CLJ-733] Data Conveying Exception Created: 28/Jan/11  Updated: 26/Jul/13  Resolved: 09/Dec/11

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: None
Fix Version/s: Backlog

Type: Enhancement Priority: Minor
Reporter: Stuart Halloway Assignee: Stuart Halloway
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0733-data-conveying-exception-2.patch     Text File 0733-data-conveying-exception.patch     Text File 0733-dec-2-edition.patch     Text File 0733-ExceptionInfo.patch     Text File 0733-with-ex-data-and-ex-info.patch     Text File 0733-with-map-check.patch    
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


 Comments   
Comment by Alan Dipert [ 28/Jan/11 2:47 PM ]

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.

Comment by Rich Hickey [ 31/Jan/11 12:42 PM ]

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?

Comment by Kevin Downey [ 19/Apr/11 3:41 PM ]

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

Comment by Stuart Halloway [ 14/Oct/11 12:52 PM ]

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.

Comment by Rich Hickey [ 14/Oct/11 1:34 PM ]

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

Comment by Rich Hickey [ 14/Oct/11 1:51 PM ]

Also the description no longer describes the approach

Comment by Stuart Halloway [ 16/Oct/11 6:06 AM ]

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

Comment by Rich Hickey [ 16/Oct/11 6:32 AM ]

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.

Comment by Rich Hickey [ 16/Oct/11 6:40 AM ]

Also please mark as alpha, subject to change.

Comment by Alan Dipert [ 21/Oct/11 10:21 AM ]

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.

Comment by Rich Hickey [ 21/Oct/11 2:15 PM ]

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

Comment by Stuart Sierra [ 02/Dec/11 9:20 AM ]

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.

Comment by Stuart Sierra [ 02/Dec/11 9:21 AM ]

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

Comment by Stuart Halloway [ 02/Dec/11 10:07 AM ]

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

Comment by Stuart Halloway [ 02/Dec/11 10:32 AM ]

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

Comment by Stuart Sierra [ 03/Dec/11 12:06 PM ]

Screened & moved to Approved Backlog.

Comment by Hugo Duncan [ 05/Dec/11 4:53 PM ]

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.

Generated at Tue Jul 29 22:17:05 CDT 2014 using JIRA 4.4#649-r158309.