java.sql2 Connection.connect(Function<Throwable, Void>) should be connect(Consumer<Throwable>)

[Douglas Surber]

Lukas,

1. It is an open question whether Operation.onError should take a Consumer or a Function. The difference is whether it can change the Throwable to a different Throwable. As is, onError takes a Consumer and the Operation is completed exceptionally with the original Throwable. If onError takes a Function then the Operation would be completed exceptionally with the result of calling the Function with the original Throwable. Either makes sense.

That said, I agree that Connection.connect should take a Consumer not a Function. There is no way to get access to the internal CompletableFuture so how it is completed is irrelevant. It’s simpler for the user to provide a Consumer. Accepting a Function would mislead the user into thinking there was some way to use the result of the Function which there is not.

2. The JavaDoc for Submission.toCompletableFuture requires that the implementation guarantee that if the Operation completes exceptionally any CompletableFuture created by the Submission be completed exceptionally with the same Throwable. I don’t see any problem implementing this.

Douglas

> On Oct 4, 2017, at 3:47 AM, Lukas Eder <lukas.eder at gmail.com> wrote:
> 
> Hello,
> 
> I'm referring to this version:
> http://hg.openjdk.java.net/jdk10/sandbox/jdk/file/a31057bda7c5/src/java.sql/share/classes/java/sql2
> 
> Which I believe to be the latest version on that branch. There's a method
> Connection.connect(Function<Throwable, Void>) which I think has two issues:
> 
> 1. It should accept a Consumer<Throwable>, just like Operation.onError(). I
> don't see the point of having a Function<Throwable, Void> here. It looks
> like this is just because of an implementation detail leaking into the API.
> The implementation should be:
> 
>  public default Connection connect(Consumer<Throwable> onError) {
>    this.holdForMoreMembers()
>            .submit();
>    this.connectOperation()
>            .submit()
>            .toCompletableFuture()
>            .exceptionally(t -> { onError.accept(t); return null; });
>    return this;
>  }
> 
> Instead of:
> 
>  public default Connection connect(Function<Throwable, Void> onError) {
>    this.holdForMoreMembers()
>            .submit();
>    this.connectOperation()
>            .submit()
>            .toCompletableFuture()
>            .exceptionally(onError);
>    return this;
>  }
> 
> 2. This may be my incomplete understanding of the API in general, but from
> what I understand, there might be a race condition between the submit()
> operation, and the transformation to a CompletableFuture and the error
> interception. Can this API really *guarantee* that all errors will be
> passed to the "onError" handler even if submit() terminates super fast?
> 
> If not, this might be a flaw in the Submission.toCompletableFuture() method.
> 
> Thanks,
> Lukas