REASSERT Code review request for 8012044: Give more information about self-suppression from Throwable.addSuppressed

[Remi Forax]

On 04/23/2013 07:51 AM, 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

Like David,
I think that the use of addSuppressed is a bit too much,
suppressed exception are exceptions that were thrown and there is no 
guarantee that a cause was thrown before
(it's sometime the case, but sometimes the cause is used as a 
'tunelling' mechanism,
If I want to throw ThatException but the method only throw ThisException 
so I will create a ThisException
that used a newly created ThatException as cause. In that case, the 
cause was never thrown
and register it has a suppressed exception is weird IMO.

cheers,
Rémi