[External] : Re: Changes to JEP 453

[forax at univ-mlv.fr]

----- Original Message -----
> From: "Ron Pressler" <ron.pressler at oracle.com>
> To: "Remi Forax" <forax at univ-mlv.fr>
> Cc: "loom-dev" <loom-dev at openjdk.org>
> Sent: Wednesday, May 24, 2023 11:00:11 AM
> Subject: Re: [External] : Re: Changes to JEP 453

> Okay, so we should try to avoid ever directly dealing with a particular
> subtask’s exceptions; in fact we try to avoid ever calling any method on
> Subtask other than get. If you find yourself calling any other Subtask method
> (outside handleComplete) — rethink.
> 
> So here’s how I might do it (assuming you really don’t want to wrap the forks’
> exceptions; also, sorry, but I couldn’t bring myself to wrapping
> InterruptedException with a different type):

As i said, the trick to use an Optional instead of an Exception is nice until you want to centralize the logging (log the potential exceptions and the returned result) without emitting a flly of log record you then have to correlate.

> 
> static MailData getMailData(long customerId, long productId) throws
> ApplicationException, InterruptedException {
>    var customerService = new CustomerService();
>    var productService = new ProductService();
> 
>    try (var sts = new StructuredTaskScope.ShutdownOnFailure()) {
>        var customerTask = sts.fork(() ->
>        customerService.findCustomerFromId(customerId));
>        var productDescriptionTask = sts.fork(() -> {
>            try { return Optional.of(productService.findProductDescription(productId)); }
>            catch (ApplicationException e) { return Optional.<ProductDescription>empty(); }
>        });
>    
>        sts.join().throwIfFailed(e -> e);
>    
>        var customer = customerTask.get();
>        var productDescription = productDescriptionTask.get()
>             .orElse(getDefaultDescription(customer.locale());
>    
>        return new MailData(customer.name(), productDescription);
>    } catch (ApplicationException|InterruptedException|Error e) {
>        throw e;
>    } catch (Throwable e) {
>        throw new RuntimeException(e);
>    }
> }

Here we are discussing about two things, the first one is to have a method result() in Subtask, the second is exception transparency, i think even if you no not want a method result() in Subtask (let see how people use the preview), having exception transparency also simplify the code.

Here is your code if we implement exception transparency, you have to parametrized ShutdownOnFailure by the exception but you are rewarded because you can remove all the try/catch.
 

static MailData getMailData(long customerId, long productId) throws ApplicationException, InterruptedException {
   var customerService = new CustomerService();
   var productService = new ProductService();

   try (var sts = new StructuredTaskScope.ShutdownOnFailure<ApplicationException>()) {
       var customerTask = sts.fork(() -> customerService.findCustomerFromId(customerId));
       var productDescriptionTask = sts.fork(() -> {
           try {
             return Optional.of(productService.findProductDescription(productId));
           } catch (ApplicationException e) {
             return Optional.<ProductDescription>empty();
           }
       });
   
       sts.join().throwIfFailed(e -> e);
   
       var customer = customerTask.get();
       var productDescription = productDescriptionTask.get()
            .orElseGet(() -> getDefaultDescription(customer.locale());
   
       return new MailData(customer.name(), productDescription);
   }
}

> 
> Note that it also shuts down correctly if CustomerService throws.

yes, your implementation fails faster.

> 
> — Ron

Rémi

BTW: I believe the first catch in your code is missing RuntimeException.

> 
>> On 23 May 2023, at 22:00, forax at univ-mlv.fr wrote:
>> 
>> 
>> Let say I've two services, one that give me a Customer from a customer id and
>> another that give me the product description from a product id,
>> and i want to send a mail containing the customer name and a description.
>> Alternative scenario, if the product service reports an error, a default
>> description is provided depending on the language of the customer.
>> 
>> record Customer(long id, String name, Locale locale) {}
>> record ProductDescription(long id, String description) {}
>> 
>> static sealed class ApplicationException extends Exception { ... }
>> static final class ServiceErrorException extends ApplicationException { ... }
>> static final class CustomerNotFoundException extends ApplicationException { ...
>> }
>> 
>> record CustomerService() {
>> Customer findCustomerFromId(long id) throws ApplicationException {
>> try {
>> Thread.sleep(1_000);
>> } catch (InterruptedException e) {
>> throw new ServiceErrorException(e);
>> }
>> if (id == 0) {
>> throw new CustomerNotFoundException();
>> }
>> return new Customer(id, "Bob", Locale.of("fr")); // fake data
>> }
>> }
>> 
>> record ProductService() {
>> ProductDescription findProductDescription(long id) throws ApplicationException {
>> try {
>> Thread.sleep(1_000);
>> } catch (InterruptedException e) {
>> throw new ServiceErrorException(e);
>> }
>> return new ProductDescription(id, "description"); // fake data
>> }
>> }
>> 
>> private static String getDefaultDescription(Locale locale) {
>> return "fake data";
>> }
>> 
>> record MailData(String customerName, String productDescription) {}
>> 
>> static MailData getMailData(long customerId, long productId) throws
>> ApplicationException {
>> var customerService = new CustomerService();
>> var productService = new ProductService();
>> 
>> try(var sts = new StructuredTaskScope<Object, ApplicationException>()) {
>> // both calls can be done in parallel
>> var customerTask = sts.fork(() ->
>> customerService.findCustomerFromId(customerId));
>> var productDescriptionTask = sts.fork(() ->
>> productService.findProductDescription(productId));
>> 
>> sts.join();
>> 
>> // the business logic is implemented here
>> var customer = customerTask.result(); // should propagate the exception
>> transparently
>> String productDescription;
>> try {
>> productDescription = productDescriptionTask.result().description();
>> } catch(ApplicationException e) { // the exception type is not erased
>> productDescription = getDefaultDescription(customer.locale());
>> }
>> return new MailData(customer.name(), productDescription);
>> } catch (InterruptedException e) {
>> throw new ServiceErrorException(e);
>> }
>> }
>> 
>> 
>> findCustomerfromId, findProductDescription and getMailData() all throw an
>> ApplicationException.
>> getMailData() can itself be called in another STS, composition is easy !
>> 
>> Exceptions naturally flows with no wrapping by default.
>> 
>> [...]
>> 
>>> 
>>> — Ron
>> 
> > Rémi