Sorry this slipped through the cracks.
Looks good to me. (Don't know if you already pushed it :) )
David
On 25/04/2013 5:16 PM, Joe Darcy wrote:
Hello,
Responding to David's comment and some comments from Alan off-list, here
is a variant which doesn't use suppressed exceptions in initCause, but
still passes along some information:
http://cr.openjdk.java.net/~darcy/8012044.4
Patch to Throwable:
--- a/src/share/classes/java/lang/Throwable.java Wed Apr 24 21:27:52
2013 +0000
+++ b/src/share/classes/java/lang/Throwable.java Thu Apr 25 00:15:32
2013 -0700
@@ -453,9 +453,10 @@
*/
public synchronized Throwable initCause(Throwable cause) {
if (this.cause != this)
- throw new IllegalStateException("Can't overwrite cause");
+ throw new IllegalStateException("Can't overwrite cause with
" +
+ Objects.toString(cause, "a null"), this);
if (cause == this)
- throw new IllegalArgumentException("Self-causation not
permitted");
+ throw new IllegalArgumentException("Self-causation not
permitted", this);
this.cause = cause;
return this;
}
@@ -1039,7 +1040,7 @@
*/
public final synchronized void addSuppressed(Throwable exception) {
if (exception == this)
- throw new IllegalArgumentException(SELF_SUPPRESSION_MESSAGE);
+ throw new
IllegalArgumentException(SELF_SUPPRESSION_MESSAGE, exception);
if (exception == null)
throw new NullPointerException(NULL_CAUSE_MESSAGE);
Thanks,
-Joe
On 04/22/2013 10:51 PM, Joe Darcy wrote:
Hi David,
On 04/22/2013 10:25 PM, David Holmes wrote:
Hi Joe,
On 23/04/2013 9:05 AM, Joseph Darcy wrote:
Hello,
Just reasserting the request for a review of the latest version of this
patch:
http://cr.openjdk.java.net/~darcy/8012044.2
I believe this version does an appropriate job of propagating exception
information when there is misuse of the methods on Throwable.
I still find the use of addSuppressed in initCause to be
questionable. Given:
catch(SomeException s) {
sharedException.initCause(s); // oops already has a cause
throw sharedException;
}
then the ISE isn't suppressing 's', but replacing/suppressing
sharedException in my view, as it is what would have been thrown had
the ISE not occurred.
I understand the desire to not lose sight of the fact that 's' was
thrown, but this is really no different to a finally block losing the
original exception info. (And fixing that was rejected when the
suppression mechanism was added.)
Project Coin discussions did note try-catch-finally and
try-with-resources were inconsistent on this point. While the
try-with-resources behavior is regarded as preferable, we thought it
would be too large a change to redefine the long-standing semantics of
try-catch-finally.
Anyway this isn't a "block" (not that such a thing exists), just a
comment. The change isn't harmful and may be useful.
Cheers,
David
Yes, I would describe the intention of this change as provding
programmers more information to debug when the methods are Throwable
and used improperly.
Thanks,
-Joe
