Hi Doug,
On 12/16/2015 03:09 PM, Doug Lea wrote:
On 12/02/2015 07:51 PM, Martin Buchholz wrote:
We're stuck. Peter wants to make Throwables cloneable, I want to
reverse the order of throwables and add the past Throwable as a
suppressed exception to the exception of a dependent action, and Doug
wants the current webrev behavior of adding the dependent action as a
suppressed exception of the source.
After letting this sit for a week or so, I think the main issue
here is that we do not properly explain how and when to use
whenComplete() vs handle(). We should explain that handle()
should be used in the same way as "catch", and whenComplete as
"finally"/try-with-resources. See below.
Given this, the most appropriate response in whenComplete upon
a secondary exception encountered when handling the primary exception
is the same as the finally-based scheme used in try-with-resources:
primary.addSuppressed(secondary). Which we should have done
originally (vs dropping secondary), and is the current proposed
webrev.
(In contrast, if someone would like to translate one exception
into another by throwing the new one, they could use handle().)
Peter raised the issue that addSuppressed could visibly (but safely)
alter the primary while it is visible to some concurrent action.
Which is true, but also no different than is possible with
try-with-resources (consider that the primary exception there could
be already somehow accessible).
It could be, but in try-with-resources there is usually just a sequence
of actions in a single thread that is always deterministic, while in
whenCompleteAsync for example, a thread requesting the outcome of
previous stage can observe it's outcome with suppressed exception added
or not, depending on timing.
If exceptions were cloneable,
it might be less surprising to users in both cases to create a new
cloned exception with the suppressed secondary. But I think we now
agree that pseudo-cloning exceptions has even more opportunity for
surprising users, so should be done only when there aren't any
alternatives.
Agreed?
Agreed. Pseudo cloning (using reflection to invoke constructor) is not
reliable.
Here's a clarification for CompletionStage (un-reformatted).
Improvements welcome. Someday we might consider adding some examples.
*
* <li>Two method forms support processing whether the triggering
* stage completed normally or exceptionally: Method {@link
! * #whenComplete whenComplete} allows injection of an action
* regardless of outcome, otherwise preserving the outcome in its
! * completion. Method {@link #handle handle} additionally allows the
* stage to compute a replacement result that may enable further
* processing by other dependent stages. In all other cases, if a
* stage's computation terminates abruptly with an (unchecked)
--- 58,66 ----
*
* <li>Two method forms support processing whether the triggering
* stage completed normally or exceptionally: Method {@link
! * #whenComplete whenComplete} is similar to a {@code finally}
clause, allowing injection of an action
* regardless of outcome, otherwise preserving the outcome in its
The finally block (in classical try/finally or in
try-with-resources/finally) preserves the outcome of try block only if
the finally block completes normally. Otherwise the outcome is replaced
with the exception thrown or value returned in finally block. For example:
// IllegalStateException propagates out of:
try {
throw new IllegalArgumentException();
} finally {
throw new IllegalStateException();
}
// as well as out of:
try (AutoCloseable r = () -> {}){
throw new IllegalArgumentException();
} finally {
throw new IllegalStateException();
}
// this method returns 2:
public static int x() {
try {
return 1;
} finally {
return 2;
}
}
If we are talking about the automatic clean-up action
(AutoCloseable::close()) and not of the finally block, then the
exceptional outcome of try block is always preserved and the exception
thrown from AutoCloseable::close() is just added as suppressed to the
exception from try block:
// IllegalArgumentException propagates out of:
try (AutoCloseable r = () -> { throw new
IllegalStateException(); }){
throw new IllegalArgumentException();
}
...but non-exceptional outcome of try block is replaced with the
exception from AutoCloseable::close():
// this method throws IllegalStateException:
public static int x() throws Exception {
try (AutoCloseable r = () -> { throw new
IllegalStateException(); }){
return 12;
}
}
If we are talking about auto-closing, then the text should not speak
about "finally", as whenComplete is not equivalent to finally block then.
Since whenComplete takes a BiConsumer, it can not "replace" the
non-exceptional outcome of previous stage. In this respect it is more
similar to void method AutoCloseable::close() than it is to finally
block which can return a value from the method. So perhaps we should
model it to the AutoCloseable::close() in try-with-resources then. Which
is what is proposed. It's just that non-determinism when multiple
threads are involved that might cause surprises.
Regards, Peter
! * completion. Method {@link #handle handle} is similar to a {@code
catch} clause, allowing the
* stage to compute a replacement result that may enable further
* processing by other dependent stages. In all other cases, if a
* stage's computation terminates abruptly with an (unchecked)
