Throwable.addSuppressed error conditions -- use the exception as the cause?

[Zhong Yu]

On Mon, Apr 8, 2013 at 6:54 PM, Steven Schlansker <
stevenschlansker at gmail.com> wrote:

> Today I encountered "java.lang.IllegalArgumentException: Self-suppression
> not permitted" from Throwable.addSuppressed.
>
> My first surprise is that the try-with-resources block can throw this
> exception; it is very confusing to have auto-generated code throw
> exceptions.  But beyond that, it is impossible to figure out *which*
> exception caused the problem.  If you have multiple resources acquired in
> the try-with-resources, it could be any of them.
>
> Would it be reasonable to change
>
>     public final synchronized void addSuppressed(Throwable exception) {
>         if (exception == this)
>             throw new IllegalArgumentException(SELF_SUPPRESSION_MESSAGE);
>
> to
>
>     public final synchronized void addSuppressed(Throwable exception) {
>         if (exception == this)
>             throw new IllegalArgumentException(SELF_SUPPRESSION_MESSAGE,
> this);
>
> so that when you get this exception it at least points to one of the throw
> sites that actually caused the problem?  Otherwise the only stack trace you
> have is in auto-generated code and you are left scratching your head,
> wondering where it came from.  I believe this would increase the
> debuggability of the try-with-resources construct and there's no
> immediately obvious downside.
>
> I'm not the only one who was confused by this behavior:
>
> http://stackoverflow.com/questions/12103126/what-on-earth-is-self-suppression-not-permitted-and-why-is-javac-generating-co
>
>
> Reusing exception is probably not a good idea - unless the exception is
immutable, i.e. enableSuppression=writableStackTrace=true.

Interestingly, even for an immutable exception, `e.addSuppressed(e)` isn't
allowed. I think that it should be allowed.

Zhong Yu