Re: [DISCUSS][txn] Move checked exception into builder when newTransaction.

[Enrico Olivelli]

I totally agree.
There has been little API Design error to add that "throws" clause.

Thanks for the initiative

Enrico

Il giorno lun 30 gen 2023 alle ore 09:16 <mattisonc...@gmail.com> ha scritto:
>
> Hello, everyone
>
> I submitted this PR https://github.com/apache/pulsar/pull/19356 to discuss if 
> we can accept moving this checked exception into the builder to avoid adding 
> more useless try-catch blocks.
>
> In practice, we should create the new transaction like this:
>
> ```java
> final TransactionBuilder transactionBuilder;
> try {
>     transactionBuilder = client.newTransaction();
> } catch (PulsarClientException e) {
>     // handle this exception.
>     return;
> }
> CompletableFuture<Transaction> txnFuture = transactionBuilder
>         .withTransactionTimeout(1, TimeUnit.MINUTES)
>         .build();
> ```
> But this exception only throws by
>
> ```java
>  public TransactionBuilder newTransaction() throws PulsarClientException {
>     if (!conf.isEnableTransaction()) {
>         throw new 
> PulsarClientException.InvalidConfigurationException("Transactions are not 
> enabled");
>     }
>     return new TransactionBuilderImpl(this, tcClient);
> }
> ```
> Therefore, even if we enabled the transaction, we still need to handle this 
> checked exception. Maybe we can move this checked exception into the builder 
> or make this exception to be runtime to avoid writing the more useless 
> try-catch blocks.
>
> ```java
> client.newTransaction()
>       .withTransactionTimeout(1, TimeUnit.MINUTES)
>       .build()
>       .thenCompose(transaction -> {
>           // do somethings
>       }).exceptionally(ex -> {
>           // handle exception
>       });
> ```
> It is better, isn't it?
>
> Best,
> Mattison