mbeckerle commented on code in PR #1196:
URL: https://github.com/apache/daffodil/pull/1196#discussion_r1544614047
##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/api/DFDLParserUnparser.scala:
##########
@@ -236,8 +236,12 @@ object DFDL {
* Thrown by the DaffodilUnparseConentHandler when an unexpected error
* occurs, this usually represents a bug in Daffodil
*/
- class DaffodilUnhandledSAXException(description: String, cause: Exception)
- extends SAXException(description, cause)
+ class DaffodilUnhandledSAXException(description: String, cause: Throwable)
+ extends SAXException(description, new Exception(cause)) {
+ def this(msg: String) = this(msg, null)
+
+ def this(cause: Throwable) = this(null, cause)
Review Comment:
I think Java exceptions with messages and causes do not have an agreed/ideal
usage pattern.
I've looked and not found it.
The encapsulation pattern: where you wrap your own exception class around an
underlying library's thrown exception - is a useful pattern, but not one that
is necessarily fully supported by other software. In particular, if you use
this pattern, and don't provide any message text, because well, the class name
itself says it all, then you find some things, like SLF4J, make no effort to
create any useful message from your classes.
Some frameworks/systems are used in a way where exception messages are
needed, other situations causes are used, but there seem to be many contexts
where things assume messages are present when they are not.
There are also goofy things that happen with cause chains. I've seen
circular cause chains, for example, where the cause of an exception points back
to itself. Anything that chases the cause chain needs to cope with this. Loops
in this chain are clearly meaningless mistakes. It's possible for the same
exception class to appear in the chain as two different instances, but not a
cycle.
To me it's very desirable to have a reliable single call which always
creates a explanatory string from any throwable regardless of whether any given
encapsulation or exception class in the chain provides a message, has a cycle,
etc. Often the class names of the exceptions in the chain are the important
information .
Unfortunately, that isn't what getMessage() does. Many things that handle
exceptions appear to believe getMessage has that contract, when it really is
just a getter on a message if provided at construction time.
And ... you override getMessage() at your peril. Voice of experience as you
can imagine. It is very easy to get stack overflows if you override
getMessage() and anything goes wrong. E.g., overriding get message to call
super.getMessage, and if null look at the cause.... turns out to be a very bad
idea.
I think our Misc.getSomeMessage(e) is supposed to be the thing that always
gives you *some* message from an exception - even if it's just the class name.
I don't think it deals with chained exceptions quite right though.
But SLF4J doesn't call our Misc.getSomeMessage(e) of course. It seems to
just call the getMessage() getter and ends up ignoring the cause object.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
