CompletableFuture.whenComplete survey

classic Classic list List threaded Threaded
31 messages Options
12
Reply | Threaded
Open this post in threaded view
|

CompletableFuture.whenComplete survey

Doug Lea

Ever since Chris Purcell helpfully pointed out last summer that we
should not drop nested exceptions in whenComplete, we've stalled
incorporating changes. There are some good arguments for each of
two options, that affects only a single line of code.
(We've ruled out some other options.)

Even though this is a corner case issue with low impact, we'd like
to choose the best option we can. So please help us out by
responding to this SurveyMonkey survey before Saturday
morning (19 December).

   https://www.surveymonkey.com/r/3D2L9DR

Thanks!

-Doug
_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: CompletableFuture.whenComplete survey

tpeierls
I'm confused: whenComplete takes a BiConsumer, not a BiFunction, so it can't return a value. I can't get the survey question code to compile.

On Wed, Dec 16, 2015 at 8:00 PM, Doug Lea <[hidden email]> wrote:

Ever since Chris Purcell helpfully pointed out last summer that we
should not drop nested exceptions in whenComplete, we've stalled
incorporating changes. There are some good arguments for each of
two options, that affects only a single line of code.
(We've ruled out some other options.)

Even though this is a corner case issue with low impact, we'd like
to choose the best option we can. So please help us out by
responding to this SurveyMonkey survey before Saturday
morning (19 December).

  https://www.surveymonkey.com/r/3D2L9DR

Thanks!

-Doug
_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest


_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: CompletableFuture.whenComplete survey

Doug Lea
On 12/18/2015 10:57 AM, Tim Peierls wrote:
> I'm confused: whenComplete takes a BiConsumer, not a BiFunction, so it can't
> return a value. I can't get the survey question code to compile.

Sorry. The "return" with values should just say "return". They
don't get executed in the case in question.
Non-compilability is an accidental feature of the survey.

-Doug

>
> On Wed, Dec 16, 2015 at 8:00 PM, Doug Lea <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>
>     Ever since Chris Purcell helpfully pointed out last summer that we
>     should not drop nested exceptions in whenComplete, we've stalled
>     incorporating changes. There are some good arguments for each of
>     two options, that affects only a single line of code.
>     (We've ruled out some other options.)
>
>     Even though this is a corner case issue with low impact, we'd like
>     to choose the best option we can. So please help us out by
>     responding to this SurveyMonkey survey before Saturday
>     morning (19 December).
>
>     https://www.surveymonkey.com/r/3D2L9DR
>
>     Thanks!
>
>     -Doug
>     _______________________________________________
>     Concurrency-interest mailing list
>     [hidden email] <mailto:[hidden email]>
>     http://cs.oswego.edu/mailman/listinfo/concurrency-interest
>
>

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: CompletableFuture.whenComplete survey

Doug Lea
In reply to this post by Doug Lea

Thanks to the 71 people who answered the survey. The majority (52)
voted for the second option. To recap, here's the (fixed) question:

   CompletableFuture<String> f1 = CompletableFuture.supplyAsync(() -> {
      if (true)
         throw new FirstException();
      else
         return "A"; });

   CompletableFuture<String> f2 = f1.whenComplete((result, exception) -> {
      if (exception != null) {
         if (true)
            throw new SecondException();
          else
            return;
      }
      else
         return; });

(where "true" stands in for some error condition).

What should the completed value of f2 be?

19: The FirstException, with the SecondException as suppressed exception
52: The SecondException, with the FirstException as suppressed exception

The vote was closer among the 29 people who filled out the optional
rationale, but still favoring the second.

One consideration differentiating votes is whether SecondException was
viewed as intentional versus the result of a programming error or
cleanup artifact. Regardless of vote outcome, throwing an exception in an
exception handler (in CompletableFutures or elsewhere) is likely to
surprise some users.

The most detailed rationale, by Tim Peierls, was
long enough that he had to place it elsewhere:
   https://gist.github.com/Tembrel/68d5670bf37824d55078

Others are below.

...

to me, it's as if the lambda is wrapped in a try-catch, with the `if (exception
!= null)` block as the catch. As this doesn't rethrow the FirstException, f2
should contain the thrown SecondException.

...

the most recent exception should be first, with prior ones nested

...

whenComplete shouldn't allow overwriting a normal result with a different normal
result despite what the example code implies. Nor should it allow replacing an
exceptional result with a different exceptional result, but going with the
second option would be same as specifying just that, with an added wrinkle that
the replacement exception is modified to suppress the original if it doesn't do
that already.

...

the first exception was "handled" by the whenComplete, but not successfully

...

By using whenComplete, the user is declaring an expectation that FirstException
may occur and are saying "I'll handle it." They haven't declared they will
handle SecondException, so that one seems more likely (than the first) to be a
programming error, and more important to call out.

...

per "then the returned stage exceptionally completes with this exception unless
this stage also completed exceptionally." unless is the key word indicating f1
exception has primacy.

...

This matches the current behavior (aside from the additional suppressed
exception, of course). It also matches try-with-resources, the other main case
for suppressed exceptions. (It consequently doesn't match try-finally, but we
can match only one of the two, and try-with-resources, besides being the one
that uses suppressed exceptions, is newer and (debatably) a replacement for most
existing try-finally blocks.) But I admit that the try-with-resources and
try-finally analogies aren't perfect for whenComplete(): whenComplete() has
access to the result, but the try-with-resources close() implementation and the
try-finally's finally do not. If whenComplete() is intended to handle other use
cases, then those use cases may have different requirements.

...

As long as application code has the opportunity to be aware of the first
exception and take action, it's not suppressed. Suppressing the second exception
would mean no application code has the opportunity to even be aware of it.

...

since I specifically handle FirstException and decide to throw SecondException
that would be the one that I expect at the end

...

f2 was supposed to handle exception from f1, but failed, so we throw
SecondException. To not lose data, we suppress FirstException

...

First exception is likely to be the cause of a whole issue.

...

1. The contract of F2 is throw SecondException, not FirstException. 2. The
implementor is knowingly suppressing FirstException, similar supplying the
caused by exception when wrapping a thrown exception.

...

It should be treated the same way, when FirstException occurred in a try-block
and SecondException occurred in finally block. I think the try-exception is the
one which is thrown and the finally-exception is added to it as suppressed.

...

It is the least change to the current semantics; it matches the behaviour of
try-with-resources; it lets you write a stage that releases resources without
worrying about the exceptional path of the main code.

...
It should be isomorphic to throwing an exception in a catch block.

...

The second exception is further down the line and one can think of it as the
transformation of the first exception, i.e., closest failure first (similar to
how printStackTrace prints the closest method first, not the Thread's starting
method). The third option would be to introduce some CompositeException that
will get both exceptions suppressed, in timely order. Besides, a fluent
java.util.concurrent.Flow library does this when lambdas crash on the onError path.

...

SecondException is the one that is last thrown.

...

For f2, SecondException is the primary exception (at the top of the ladder), and
everything else must trail behind in the context of historicity

...

I can see reasons for both options, but in the end, I think that the caller of
oncomplete passes in a lambda which may have a known and well defined set of
throws, thus they are entitled to expect that f2 will only have values that can
be thrown by the lambda

...

For consistency as an async version of a try-catch-finally block. Here "then" ==
try; "exceptionally" == catch; "when"/"handle" == finally.

...

[Option 1] mirrors try-with-resources and (arguably) is how try-finally should
have been specified when both the try and finally blocks throw.

...

At first glance, I was viewing exception2 as the suppressed one, but then I
wondered what happens in a chain of exceptional completions. Do suppressed
exceptions chain? That is, what happens in a chain of exceptional completions?
Is there a way to draw an analogy to a nesting of ARM statements? On second or
third thought, I don't see the need for suppressed exceptions here, and worry a
little bit about creating one automatically. I say there is not a real
suppressed exception in this case because the first exception is "explicit" in
the completion arguments. However, there is a chance that the earlier exception
won't be handled by the completion code, and will be lost. And so, as a nicety
(B) the first exception could be added as a suppressed exception of exception2.

...

Order of execution makes me feel like an exception from f1 should be the root,
and f2 be the suppressed exception.

...

The implementor of f2 is explicitly handling the exception from f1 and throwing
a new one. That one is the one that matters to the client of f2, at least from
f2's perspective.

...

No time travelling to change history!

...

When ever an in-flight exception gets replaced it should become a suppressed
exception of the new exception.

...

try-with-resources will add exceptions from close() as suppressed exceptions.






_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: CompletableFuture.whenComplete survey

Joe Bowbeer
I'm still holding out for option C: none of the above.  (Also I want access to the voter database!)

The concerns that I didn't express very well in my survey response are:

1. That exception1 is not "suppressed" because it has already been delivered to user code.

I think the comparison with ARM try-with-resources breaks down because in try-with-resources the exception *is* suppressed by the framework, and would otherwise go unreported.

2. The completion handler receiving exception1 may not want it to reach the next stage, even via getSuppressedExceptions.

--Joe

On Sat, Dec 19, 2015 at 5:08 AM, Doug Lea <[hidden email]> wrote:

Thanks to the 71 people who answered the survey. The majority (52)
voted for the second option. To recap, here's the (fixed) question:

  CompletableFuture<String> f1 = CompletableFuture.supplyAsync(() -> {
     if (true)
        throw new FirstException();
     else
        return "A"; });

  CompletableFuture<String> f2 = f1.whenComplete((result, exception) -> {
     if (exception != null) {
        if (true)
           throw new SecondException();
         else
           return;
     }
     else
        return; });

(where "true" stands in for some error condition).

What should the completed value of f2 be?

19: The FirstException, with the SecondException as suppressed exception
52: The SecondException, with the FirstException as suppressed exception

The vote was closer among the 29 people who filled out the optional
rationale, but still favoring the second.

One consideration differentiating votes is whether SecondException was
viewed as intentional versus the result of a programming error or
cleanup artifact. Regardless of vote outcome, throwing an exception in an
exception handler (in CompletableFutures or elsewhere) is likely to
surprise some users.

The most detailed rationale, by Tim Peierls, was
long enough that he had to place it elsewhere:
  https://gist.github.com/Tembrel/68d5670bf37824d55078

Others are below.

...

to me, it's as if the lambda is wrapped in a try-catch, with the `if (exception != null)` block as the catch. As this doesn't rethrow the FirstException, f2 should contain the thrown SecondException.

...

the most recent exception should be first, with prior ones nested

...

whenComplete shouldn't allow overwriting a normal result with a different normal result despite what the example code implies. Nor should it allow replacing an exceptional result with a different exceptional result, but going with the second option would be same as specifying just that, with an added wrinkle that the replacement exception is modified to suppress the original if it doesn't do that already.

...

the first exception was "handled" by the whenComplete, but not successfully

...

By using whenComplete, the user is declaring an expectation that FirstException may occur and are saying "I'll handle it." They haven't declared they will handle SecondException, so that one seems more likely (than the first) to be a programming error, and more important to call out.

...

per "then the returned stage exceptionally completes with this exception unless this stage also completed exceptionally." unless is the key word indicating f1 exception has primacy.

...

This matches the current behavior (aside from the additional suppressed exception, of course). It also matches try-with-resources, the other main case for suppressed exceptions. (It consequently doesn't match try-finally, but we can match only one of the two, and try-with-resources, besides being the one that uses suppressed exceptions, is newer and (debatably) a replacement for most existing try-finally blocks.) But I admit that the try-with-resources and try-finally analogies aren't perfect for whenComplete(): whenComplete() has access to the result, but the try-with-resources close() implementation and the try-finally's finally do not. If whenComplete() is intended to handle other use cases, then those use cases may have different requirements.

...

As long as application code has the opportunity to be aware of the first exception and take action, it's not suppressed. Suppressing the second exception would mean no application code has the opportunity to even be aware of it.

...

since I specifically handle FirstException and decide to throw SecondException that would be the one that I expect at the end

...

f2 was supposed to handle exception from f1, but failed, so we throw SecondException. To not lose data, we suppress FirstException

...

First exception is likely to be the cause of a whole issue.

...

1. The contract of F2 is throw SecondException, not FirstException. 2. The implementor is knowingly suppressing FirstException, similar supplying the caused by exception when wrapping a thrown exception.

...

It should be treated the same way, when FirstException occurred in a try-block and SecondException occurred in finally block. I think the try-exception is the one which is thrown and the finally-exception is added to it as suppressed.

...

It is the least change to the current semantics; it matches the behaviour of try-with-resources; it lets you write a stage that releases resources without worrying about the exceptional path of the main code.

...
It should be isomorphic to throwing an exception in a catch block.

...

The second exception is further down the line and one can think of it as the transformation of the first exception, i.e., closest failure first (similar to how printStackTrace prints the closest method first, not the Thread's starting method). The third option would be to introduce some CompositeException that will get both exceptions suppressed, in timely order. Besides, a fluent java.util.concurrent.Flow library does this when lambdas crash on the onError path.

...

SecondException is the one that is last thrown.

...

For f2, SecondException is the primary exception (at the top of the ladder), and everything else must trail behind in the context of historicity

...

I can see reasons for both options, but in the end, I think that the caller of oncomplete passes in a lambda which may have a known and well defined set of throws, thus they are entitled to expect that f2 will only have values that can be thrown by the lambda

...

For consistency as an async version of a try-catch-finally block. Here "then" == try; "exceptionally" == catch; "when"/"handle" == finally.

...

[Option 1] mirrors try-with-resources and (arguably) is how try-finally should have been specified when both the try and finally blocks throw.

...

At first glance, I was viewing exception2 as the suppressed one, but then I wondered what happens in a chain of exceptional completions. Do suppressed exceptions chain? That is, what happens in a chain of exceptional completions? Is there a way to draw an analogy to a nesting of ARM statements? On second or third thought, I don't see the need for suppressed exceptions here, and worry a little bit about creating one automatically. I say there is not a real suppressed exception in this case because the first exception is "explicit" in the completion arguments. However, there is a chance that the earlier exception won't be handled by the completion code, and will be lost. And so, as a nicety (B) the first exception could be added as a suppressed exception of exception2.

...

Order of execution makes me feel like an exception from f1 should be the root, and f2 be the suppressed exception.

...

The implementor of f2 is explicitly handling the exception from f1 and throwing a new one. That one is the one that matters to the client of f2, at least from f2's perspective.

...

No time travelling to change history!

...

When ever an in-flight exception gets replaced it should become a suppressed exception of the new exception.

...

try-with-resources will add exceptions from close() as suppressed exceptions.







_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest


_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: CompletableFuture.whenComplete survey

Doug Lea
On 12/19/2015 08:40 AM, Joe Bowbeer wrote:
> I'm still holding out for option C: none of the above.  (Also I want access
> to the voter database!)

I set it up as anonymous.

> 2. The completion handler receiving exception1 may not want it to reach the
> next stage, even via getSuppressedExceptions.
>

In which case, why not use CF.handle to perform arbitrary outcome translation?

> 1. That exception1 is not "suppressed" because it has already been delivered
> to user code.

This always holds for a suppressed exception -- some code catches it
but then does addSuppressed. The term "suppressed" is not perfect here:
   "e1 with suppressed e2"
is treated in nearly the same way as
   "e2 with cause e1",
but with roles swapped.

Martin suggested (on core-libs-dev) an alternative that might reduce
confusion. Or might not. Thoughts welcome.

On 12/18/2015 02:08 PM, Martin Buchholz wrote:

> Instead of calling addSuppressed on the source exception, call
> addSuppressed on the wrapping CompletionException.  This has the
> upside that the integrity of both original exceptions is maintained,
> but the downside that the suppressed exception is likely to be
> discarded since the CompletionException is "just a wrapper".  In fact,
> get() will do that.  But we can modify reportGet to transfer any
> suppressed exceptions from the CompletionException to the
> ExecutionException, somewhat mitigating this problem, at the cost of a
> small tax on all users of get() who encounter abnormal completion.  As
> to which exception gets to be the cause and which the suppressed, I
> still think it makes sense for the action exception to be the cause
> and the source exception to be suppressed, by analogy with "finally".


_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: CompletableFuture.whenComplete survey

Chris Purcell

The second option surprised me, as we now have a fourth approach to suppressed exceptions in Java: try/finally (throw away A), try-with-resources (suppress B), j8 whenComplete (throw away B) and j9+ whenComplete (suppress A). Isn't this unnecessarily complicated? Why not behave like a finally block, and let the user add A as a suppressed exception if they want it?

WhenComplete seems like the only way to reliably close resources in async code, so I expected it to work like try-with-resources. That seems to be the point of contention for others too. Perhaps there should be another method for this use-case instead? One which takes a simple runnable and suppresses any exception it throws.

Thanks!
Chris


On Sat, 19 Dec 2015 14:38 Doug Lea <[hidden email]> wrote:
On 12/19/2015 08:40 AM, Joe Bowbeer wrote:
> I'm still holding out for option C: none of the above.  (Also I want access
> to the voter database!)

I set it up as anonymous.

> 2. The completion handler receiving exception1 may not want it to reach the
> next stage, even via getSuppressedExceptions.
>

In which case, why not use CF.handle to perform arbitrary outcome translation?

> 1. That exception1 is not "suppressed" because it has already been delivered
> to user code.

This always holds for a suppressed exception -- some code catches it
but then does addSuppressed. The term "suppressed" is not perfect here:
   "e1 with suppressed e2"
is treated in nearly the same way as
   "e2 with cause e1",
but with roles swapped.

Martin suggested (on core-libs-dev) an alternative that might reduce
confusion. Or might not. Thoughts welcome.

On 12/18/2015 02:08 PM, Martin Buchholz wrote:
> Instead of calling addSuppressed on the source exception, call
> addSuppressed on the wrapping CompletionException.  This has the
> upside that the integrity of both original exceptions is maintained,
> but the downside that the suppressed exception is likely to be
> discarded since the CompletionException is "just a wrapper".  In fact,
> get() will do that.  But we can modify reportGet to transfer any
> suppressed exceptions from the CompletionException to the
> ExecutionException, somewhat mitigating this problem, at the cost of a
> small tax on all users of get() who encounter abnormal completion.  As
> to which exception gets to be the cause and which the suppressed, I
> still think it makes sense for the action exception to be the cause
> and the source exception to be suppressed, by analogy with "finally".


_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: CompletableFuture.whenComplete survey

tpeierls
On Sat, Dec 19, 2015 at 11:16 AM, Chris Purcell <[hidden email]> wrote:

The second option surprised me, as we now have a fourth approach to suppressed exceptions in Java: try/finally (throw away A), try-with-resources (suppress B), j8 whenComplete (throw away B) and j9+ whenComplete (suppress A). Isn't this unnecessarily complicated?

Like Joe, I would have voted against suppression if that had been given as a survey option.
 

Why not behave like a finally block, and let the user add A as a suppressed exception if they want it?

Is this opt-in behavior not achievable with a wrapper? (Assuming survey-option-2-without-suppression semantics.)

    public static <T> BiConsumer<T, Throwable> suppressEarlierException(
            BiConsumer<? super T, ? super Throwable> action) {
                
        return (r1, ex1) -> {
            try {
                action.accept(r1, ex1);
            } catch (Throwable ex2) {
                ex2.addSuppressed(ex1);
                throw ex2;
            }
        };
    }
 

WhenComplete seems like the only way to reliably close resources in async code, so I expected it to work like try-with-resources. That seems to be the point of contention for others too. Perhaps there should be another method for this use-case instead? One which takes a simple runnable and suppresses any exception it throws.

I don't see why CF.handle(BiFunction) isn't already sufficient for providing any behavior you want here.

--tim



_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: CompletableFuture.whenComplete survey

Timo Kinnunen
In reply to this post by Doug Lea

Hi,

 

Interesting and unexpected results, and thanks especially to everyone who provided a rationale.

 

I’d like to point out that the question posed in the survey isn’t the only one that needs to be considered when deciding the whenComplete method contract. A BiConsumer that rethrows the same FirstException, or throws a SecondException which has the FirstException as its cause, or throws a SecondException which suppresses the FirstException are also possibilities that must be taken into account.

 

That said, what I take away from the results is that there’s too much overlap between the whenComplete(BiConsumer) method and the handle(BiFunction) method. As several responses which mentioned handling indicates, it’s not clear where the responsibilities of handle end and whenComplete’s begin. The uncorrected code in the survey itself confused the two methods with each other, too! So my recommendations would then be:

 

-          Specify the whenComplete methods in terms of the handle method.

-          Rewrite the implementations of the whenComplete method to forward the call to the handle methods.

-          And then deprecate whenComplete.

 

With this it becomes readily apparent which method to use when you want to react to the exceptional result as well and it’s clear what happens if your BiFunction throws an exception, regardless of if it was thrown intentionally or due to a bug.

 

 

 

 

 

 



--
Have a nice day,
Timo

Sent from Mail for Windows 10

 


From: [hidden email]
Sent: Saturday, December 19, 2015 14:17
To: [hidden email]
Subject: Re: [concurrency-interest] CompletableFuture.whenComplete survey

 

 

Thanks to the 71 people who answered the survey. The majority (52)

voted for the second option. To recap, here's the (fixed) question:

 

   CompletableFuture<String> f1 = CompletableFuture.supplyAsync(() -> {

      if (true)

         throw new FirstException();

      else

         return "A"; });

 

   CompletableFuture<String> f2 = f1.whenComplete((result, exception) -> {

      if (exception != null) {

         if (true)

            throw new SecondException();

          else

           return;

      }

      else

         return; });

 

(where "true" stands in for some error condition).

 

What should the completed value of f2 be?

 

19: The FirstException, with the SecondException as suppressed exception

52: The SecondException, with the FirstException as suppressed exception

 

The vote was closer among the 29 people who filled out the optional

rationale, but still favoring the second.

 

One consideration differentiating votes is whether SecondException was

viewed as intentional versus the result of a programming error or

cleanup artifact. Regardless of vote outcome, throwing an exception in an

exception handler (in CompletableFutures or elsewhere) is likely to

surprise some users.

 

The most detailed rationale, by Tim Peierls, was

long enough that he had to place it elsewhere:

   https://gist.github.com/Tembrel/68d5670bf37824d55078

 

Others are below.

 

...

 

to me, it's as if the lambda is wrapped in a try-catch, with the `if (exception

!= null)` block as the catch. As this doesn't rethrow the FirstException, f2

should contain the thrown SecondException.

 

...

 

the most recent exception should be first, with prior ones nested

 

...

 

whenComplete shouldn't allow overwriting a normal result with a different normal

result despite what the example code implies. Nor should it allow replacing an

exceptional result with a different exceptional result, but going with the

second option would be same as specifying just that, with an added wrinkle that

the replacement exception is modified to suppress the original if it doesn't do

that already.

 

...

 

the first exception was "handled" by the whenComplete, but not successfully

 

...

 

By using whenComplete, the user is declaring an expectation that FirstException

may occur and are saying "I'll handle it." They haven't declared they will

handle SecondException, so that one seems more likely (than the first) to be a

programming error, and more important to call out.

 

...

 

per "then the returned stage exceptionally completes with this exception unless

this stage also completed exceptionally." unless is the key word indicating f1

exception has primacy.

 

...

 

This matches the current behavior (aside from the additional suppressed

exception, of course). It also matches try-with-resources, the other main case

for suppressed exceptions. (It consequently doesn't match try-finally, but we

can match only one of the two, and try-with-resources, besides being the one

that uses suppressed exceptions, is newer and (debatably) a replacement for most

existing try-finally blocks.) But I admit that the try-with-resources and

try-finally analogies aren't perfect for whenComplete(): whenComplete() has

access to the result, but the try-with-resources close() implementation and the

try-finally's finally do not. If whenComplete() is intended to handle other use

cases, then those use cases may have different requirements.

 

...

 

As long as application code has the opportunity to be aware of the first

exception and take action, it's not suppressed. Suppressing the second exception

would mean no application code has the opportunity to even be aware of it.

 

...

 

since I specifically handle FirstException and decide to throw SecondException

that would be the one that I expect at the end

 

...

 

f2 was supposed to handle exception from f1, but failed, so we throw

SecondException. To not lose data, we suppress FirstException

 

...

 

First exception is likely to be the cause of a whole issue.

 

...

 

1. The contract of F2 is throw SecondException, not FirstException. 2. The

implementor is knowingly suppressing FirstException, similar supplying the

caused by exception when wrapping a thrown exception.

 

...

 

It should be treated the same way, when FirstException occurred in a try-block

and SecondException occurred in finally block. I think the try-exception is the

one which is thrown and the finally-exception is added to it as suppressed.

 

...

 

It is the least change to the current semantics; it matches the behaviour of

try-with-resources; it lets you write a stage that releases resources without

worrying about the exceptional path of the main code.

 

...

It should be isomorphic to throwing an exception in a catch block.

 

...

 

The second exception is further down the line and one can think of it as the

transformation of the first exception, i.e., closest failure first (similar to

how printStackTrace prints the closest method first, not the Thread's starting

method). The third option would be to introduce some CompositeException that

will get both exceptions suppressed, in timely order. Besides, a fluent

java.util.concurrent.Flow library does this when lambdas crash on the onError path.

 

...

 

SecondException is the one that is last thrown.

 

...

 

For f2, SecondException is the primary exception (at the top of the ladder), and

everything else must trail behind in the context of historicity

 

...

 

I can see reasons for both options, but in the end, I think that the caller of

oncomplete passes in a lambda which may have a known and well defined set of

throws, thus they are entitled to expect that f2 will only have values that can

be thrown by the lambda

 

...

 

For consistency as an async version of a try-catch-finally block. Here "then" ==

try; "exceptionally" == catch; "when"/"handle" == finally.

 

...

 

[Option 1] mirrors try-with-resources and (arguably) is how try-finally should

have been specified when both the try and finally blocks throw.

 

...

 

At first glance, I was viewing exception2 as the suppressed one, but then I

wondered what happens in a chain of exceptional completions. Do suppressed

exceptions chain? That is, what happens in a chain of exceptional completions?

Is there a way to draw an analogy to a nesting of ARM statements? On second or

third thought, I don't see the need for suppressed exceptions here, and worry a

little bit about creating one automatically. I say there is not a real

suppressed exception in this case because the first exception is "explicit" in

the completion arguments. However, there is a chance that the earlier exception

won't be handled by the completion code, and will be lost. And so, as a nicety

(B) the first exception could be added as a suppressed exception of exception2.

 

...

 

Order of execution makes me feel like an exception from f1 should be the root,

and f2 be the suppressed exception.

 

...

 

The implementor of f2 is explicitly handling the exception from f1 and throwing

a new one. That one is the one that matters to the client of f2, at least from

f2's perspective.

 

...

 

No time travelling to change history!

 

...

 

When ever an in-flight exception gets replaced it should become a suppressed

exception of the new exception.

 

...

 

try-with-resources will add exceptions from close() as suppressed exceptions.

 

 

 

 

 

 

_______________________________________________

Concurrency-interest mailing list

[hidden email]

http://cs.oswego.edu/mailman/listinfo/concurrency-interest

 


_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: CompletableFuture.whenComplete survey

Chris Purcell
In reply to this post by tpeierls


On Sat, 19 Dec 2015 16:40 Tim Peierls <[hidden email]> wrote:
On Sat, Dec 19, 2015 at 11:16 AM, Chris Purcell <[hidden email]> wrote:

Why not behave like a finally block, and let the user add A as a suppressed exception if they want it?

Is this opt-in behavior not achievable with a wrapper? (Assuming survey-option-2-without-suppression semantics.)

Yep, that's exactly what I meant, thanks for the impl!


WhenComplete seems like the only way to reliably close resources in async code, so I expected it to work like try-with-resources. That seems to be the point of contention for others too. Perhaps there should be another method for this use-case instead? One which takes a simple runnable and suppresses any exception it throws.

I don't see why CF.handle(BiFunction) isn't already sufficient for providing any behavior you want here.

In fact, the non-suppressing-option-2 whenComplete would be sufficient to implement it. However, it's a lot of boilerplate for a common use-case, and easy to get viciously wrong, as modifying the result of a previous stage (even an exception) is a bug: you need to carefully clone the exception with serialisation. I would expect there to be a default implementation that does the obvious (painful) delegation. (And I would expect a lot of effort to go into optimising the CompletableFuture version to avoid the clone if it isn't needed.)

Cheers,
Chris


_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: CompletableFuture.whenComplete survey

Doug Lea
In reply to this post by Chris Purcell
On 12/19/2015 11:16 AM, Chris Purcell wrote:
> The second option surprised me, as we now have a fourth approach to suppressed
> exceptions in Java: try/finally (throw away A), try-with-resources (suppress B),
> j8 whenComplete (throw away B) and j9+ whenComplete (suppress A).

Actually, at the moment, our jsr166 version suppresses B.
Nothing has changed in our codebase since the a.addSuppressed(b)
fix. We were about this to commit to OpenJDK when further questions emerged,
leading to survey. My plans were to check for any new ideas or
over-my-dead-body reactions stemming from survey before changing.

Also, to recap related core-libs-dev discussions: The initial suggestion
to serialize or clone A doesn't work well with some user Throwable
classes. So reflection-based pseudo-cloning should be treated as
a desperation move, avoided when there are other options.
It was also pointed out that with a.addSuppressed(b), the call
might occur while exception a is being processed in another thread.
Which might be surprising, but is thread-safe and not in itself
a compelling reason not to do it.

>
> WhenComplete seems like the only way to reliably close resources in async code,

This was among initial motivations (see list archives mainly in December 2012).

But as it turns out from survey, a lot of people expect or want it to
behave more like handle(). Which is a different issue than the one
you first raised.

-Doug

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: CompletableFuture.whenComplete survey

Martin Buchholz-3
In reply to this post by Doug Lea
On Sat, Dec 19, 2015 at 6:15 AM, Doug Lea <[hidden email]> wrote:
> On 12/19/2015 08:40 AM, Joe Bowbeer wrote:

>> 1. That exception1 is not "suppressed" because it has already been
>> delivered
>> to user code.
>
>
> This always holds for a suppressed exception -- some code catches it
> but then does addSuppressed.

Unsurprisingly, I agree with Joe here.  For try-with-resources it is
hard for real user code to directly handle the failure from an
AutoCloseable

try (FileOutputStream s = ...)

The only way I can think of doing that is by creating another
AutoCloseable that wrapped the FileOutputStream, which is cumbersome.
(I guess users would just fall back to sugar-free try/catch/finally)

BUT with whenComplete, the user code has the exception handed to them
directly as an argument - no machinery required.
_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: CompletableFuture.whenComplete survey

Chris Purcell
In reply to this post by Doug Lea

But as it turns out from survey, a lot of people expect or want it to
behave more like handle(). Which is a different issue than the one
you first raised.

Behaving like handle() makes sense, but wasn't strictly speaking one of the survey options. Will you (a) open a new survey, (b) throw B-with-A-suppressed (survey option 2), or (c) throw B (like handle)? I'd vote for C.

Thanks!
Chris

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: CompletableFuture.whenComplete survey

Vladimir Ozerov
2015-12-19 22:41 GMT+03:00 Chris Purcell <[hidden email]>:
Behaving like handle() makes sense, but wasn't strictly speaking one of the survey options. Will you (a) open a new survey, (b) throw B-with-A-suppressed (survey option 2), or (c) throw B (like handle)? I'd vote for C.

+ 1 for just throwing B without A suppressed. 

When I receive normal result X and then transform it to Y inside my *new* completion stage, it doesn't mean that I "suppressed" X. Likewise, throwing an exception from a new completion stage should not mean that I suppressed previous exception. Instead, most likely it means that user *processed* previous exception and transformed it into some other form.

try-with-resources is not very good analogy here, because it has a single control flow. If the first exception is thrown from try-block and then the control flow is disturbed by the second exception from close() method, we end up with two pending unrelated exceptions. Both of them must be propagated further somehow and this is where suppression comes into play.
To the contrast, "whenComplete" and "invoke" create new control flow. And as there is no ambiguity, it is not clear why A should be suppressed. Looks like we can simply forget about it.

Vladimir.

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

CompletableFuture.whenComplete survey, pass 2

Doug Lea
In reply to this post by Chris Purcell
On 12/19/2015 02:41 PM, Chris Purcell wrote:
> Will you (a) open a new survey,

At this point, I think our only choice is to iterate until
we stop encountering new alternatives that we can't rule out
on other grounds. It's clear that there is no perfect policy,
but we'd still like to minimize long-term discontent.

So, please answer the new survey, before Tuesday (22 December) morning.

   https://www.surveymonkey.com/r/S3CS3LJ

It simplifies question, and includes all viable options that I know of,
with very brief explanations.

Also pasted below, but don't reply to this mail with votes.

Q1. Given

   CompletableFuture<String> f1 = CompletableFuture.supplyAsync(() -> {
      if (true)
         throw new FirstException();
      else
         return "A";
    });

   CompletableFuture<String> f2 = f1.whenComplete((result, exception) -> {
     if (true)
        throw new SecondException();
    });

Where "if (true)" shows the paths of interest in this question. These might
arise from explicit error checks, programming errors, resource failures, and/or
translated rethrows.

What should be the completed value of f2?

A. The FirstException. In other words, preserve the source outcome (only) if
exceptional, ignoring the SecondException.

B. The FirstException, with the SecondException as its suppressed exception.  In
other words, preserve but modify the source exception to record the SecondException.

C. The SecondException. In other words, replace the source outcome (whether it
is exceptional or not).

D. The SecondException, with the FirstException as its suppressed exception.  In
other words, replace the source outcome, but if exceptional, record it as a
suppressedException of the SecondException.

E. A new CompletionException, with the FirstException as its cause and the
SecondException as its suppressed exception. In other words, indicate that
throwing an exception in whenComplete is a different form of error.

Q2. 2. Even if you don't prefer them, which of the above choices are acceptable?

[same options]



_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: CompletableFuture.whenComplete survey

Doug Lea
In reply to this post by Timo Kinnunen
On 12/19/2015 11:40 AM, Timo Kinnunen wrote:
> there’s too much overlap between the whenComplete(BiConsumer) method and the
> handle(BiFunction) method.

Just as a reminder, the initial intent was that handle() translates
source outcomes, but whenComplete() preserves them.

This leaves open the policy about what to do if the action in
whenComplete itself throws an exception, so cannot preserve the
outcome without losing information. Which is the issue at hand:
in jdk8, a new exception encountered in whenComplete is dropped
if the outcome is already exceptional.

As I've said, regardless of survey results, we should improve
documentation to more strongly advise users to use handle()
instead of whenComplete() when intentionally translating outcomes.
It seems that most users know this already. As far as we know,
no user has ever complained about jdk8 behavior. The issue was
noticed while Chris was exploring an alternative implementation of
CompletionStage, suggesting that we could do better.

-Doug





_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: CompletableFuture.whenComplete survey

Joe Bowbeer
Given this discussion, I had assumed that the jdk8 documentation left the door open regarding what happened if the whenComplete action itself threw an exception, but the CompletionStage#whenComplete javadoc clearly specifies what happens:

"If the supplied action itself encounters an exception, then the returned stage exceptionally completes with this exception unless this stage also completed exceptionally."

I agree that better documentation about the intended use is needed. I've learned more from this discussion than from the CompletionStage and CompletableFuture javadoc.



On Mon, Dec 21, 2015 at 5:15 AM, Doug Lea <[hidden email]> wrote:

On 12/19/2015 11:40 AM, Timo Kinnunen wrote:
> there’s too much overlap between the whenComplete(BiConsumer) method and the
> handle(BiFunction) method.

Just as a reminder, the initial intent was that handle() translates
source outcomes, but whenComplete() preserves them.

This leaves open the policy about what to do if the action in
whenComplete itself throws an exception, so cannot preserve the
outcome without losing information. Which is the issue at hand:
in jdk8, a new exception encountered in whenComplete is dropped
if the outcome is already exceptional.

As I've said, regardless of survey results, we should improve
documentation to more strongly advise users to use handle()
instead of whenComplete() when intentionally translating outcomes.
It seems that most users know this already. As far as we know,
no user has ever complained about jdk8 behavior. The issue was
noticed while Chris was exploring an alternative implementation of
CompletionStage, suggesting that we could do better.

-Doug





_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest



_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: CompletableFuture.whenComplete survey

Doug Lea
On 12/21/2015 09:01 AM, [hidden email] wrote:
> Given this discussion, I had assumed that the jdk8 documentation left the door
> open regarding what happened if the whenComplete action itself threw an
> exception,

Given the controversy, the survey includes all known options,
including those that would require an interface-level spec change.
Only the first two options are clearly compatible with current spec.

> I agree that better documentation about the intended use is needed.

Suggestions are always welcome! People out there do complain
a lot about the terse javadocs. We initially decided against
tutorial-style documentation, thinking that the topic was too
big to fit into javadocs. But a few additional explanations
and examples would still help.

A few tutorials have arisen elsewhere, including in some java8 books and:

http://www.jesperdj.com/2015/09/26/the-future-is-completable-in-java-8/
http://www.infoq.com/articles/Functional-Style-Callbacks-Using-CompletableFuture
http://www.nurkiewicz.com/2013/05/java-8-definitive-guide-to.html

and probably more.

-Doug


_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: CompletableFuture.whenComplete survey

Peter Levart
In reply to this post by Doug Lea


On 12/21/2015 02:06 PM, Doug Lea wrote:

> On 12/19/2015 11:40 AM, Timo Kinnunen wrote:
>> there’s too much overlap between the whenComplete(BiConsumer) method
>> and the
>> handle(BiFunction) method.
>
> Just as a reminder, the initial intent was that handle() translates
> source outcomes, but whenComplete() preserves them.
>
> This leaves open the policy about what to do if the action in
> whenComplete itself throws an exception, so cannot preserve the
> outcome without losing information. Which is the issue at hand:
> in jdk8, a new exception encountered in whenComplete is dropped
> if the outcome is already exceptional.
>
> As I've said, regardless of survey results, we should improve
> documentation to more strongly advise users to use handle()
> instead of whenComplete() when intentionally translating outcomes.
> It seems that most users know this already. As far as we know,
> no user has ever complained about jdk8 behavior. The issue was
> noticed while Chris was exploring an alternative implementation of
> CompletionStage, suggesting that we could do better.
>
> -Doug

It should also be noted that the behavior of the suggested fix to
improve the situation (the point B in the 2nd survey, which says: "The
FirstException, with the SecondException as its suppressed exception. In
other words, preserve but modify the source exception to record the
SecondException"), while potentially modifying the suppressed exceptions
list of exceptional outcome of the 1st stage (f1) after it's completion,
is not that bad because:

- the Throwable.addSuppressed() is a thread-safe operation
- Throwable.getSuppressed() returns a snapshot of exceptions accumulated
so-far

...which means that a potential consumer of the exceptional outcome of
the 1st stage (f1) will see all the suppressed exceptions that were
attached to the FirstException before it was thrown in the 1st stage.
Hypothetical logic processing those suppressed exceptions will not miss
any of them. It might or might not see the additional SecondException
attached, and we hope it will not be confused by that.

Spoiler alert: "personal opinion follows". If you intend to vote
uninfluenced, do so before reading further...











I think that code that handles the exceptional outcome will usually be
only interested in the type of the main exception, it might also be
interested in it's cause, but I'm yet to see code that tries to
reconstruct a meaning from suppressed exception(s). They are usually
just a diagnostic tool. It would also be sad to loose the property of
whenComplete() which now guarantees that the exceptional outcome of the
previous stage is preserved regardless of what whenComplete() action
does as it might be an action that is not written by the same programmer
that has written a call to whenComplete().

I've written a thin wrapper for CompletableFuture that presents a
type-safe API for checked exception handling. It relies on whenComplete
not changing the type of exceptional outcome. I'm not using this API for
anything important, but just want to note that any change that makes it
possible for whenComplete() to change the type of exceptional outcome
might brake some code that currently relies on assumption that it can't
be changed.

Regards, Peter

>
>
>
>
>
> _______________________________________________
> Concurrency-interest mailing list
> [hidden email]
> http://cs.oswego.edu/mailman/listinfo/concurrency-interest

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: CompletableFuture.whenComplete survey

Joe Bowbeer
In reply to this post by Doug Lea
In the second survey email, the only options clearly *compatible* with the CompletionStage spec are the third and fourth options, C and D, which throw the second exception. Correct?



On Mon, Dec 21, 2015 at 6:25 AM, Doug Lea <[hidden email]> wrote:

On 12/21/2015 09:01 AM, [hidden email] wrote:
> Given this discussion, I had assumed that the jdk8 documentation left the door
> open regarding what happened if the whenComplete action itself threw an
> exception,

Given the controversy, the survey includes all known options,
including those that would require an interface-level spec change.
Only the first two options are clearly compatible with current spec.

> I agree that better documentation about the intended use is needed.

Suggestions are always welcome! People out there do complain
a lot about the terse javadocs. We initially decided against
tutorial-style documentation, thinking that the topic was too
big to fit into javadocs. But a few additional explanations
and examples would still help.

A few tutorials have arisen elsewhere, including in some java8 books and:

http://www.jesperdj.com/2015/09/26/the-future-is-completable-in-java-8/
http://www.infoq.com/articles/Functional-Style-Callbacks-Using-CompletableFuture
http://www.nurkiewicz.com/2013/05/java-8-definitive-guide-to.html

and probably more.

-Doug


_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest



_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
12