-2

I am making parallel call using completablefuture like below,

public Response getResponse() {
    Response resultClass = new Response();
   try {
    CompletableFuture<Optional<ClassA>> classAFuture
        = CompletableFuture.supplyAsync(() -> service.getClassA() );
    CompletableFuture<ClassB> classBFuture
        = CompletableFuture.supplyAsync(() -> {
             try {
                   return service.getClassB(); 
              }
              catch (Exception e) {
                   throw new CompletionException(e);
              }
     });

   CompletableFuture<Response> responseFuture =
    CompletableFuture.allOf(classAFuture, classBFuture)
         .thenApplyAsync(dummy -> {
            if (classAFuture.join().isPresent() {
               ClassA classA = classAFuture.join();
               classA.setClassB(classBFuture.join());
               response.setClassA(classA)
             }
            return response;
         });
   responseFuture.join();
  } catch (CompletionExecution e) {
    throw e;
  }
  return response;
}

I need to add try catch for return service.getClassB() as it throwing an exception inside the method getClassB.

Now problem I am facing is if service.getClassB() throws error it always comes wrapped inside java.util.concurrent.ExecutionException. There is a scenario where this method throws UserNameNotFoundException but this is wrapped inside ExecutionException and it is not getting caught in the right place @ControllerAdvice Exception handler class. I tried different option using throwable but it didn't help.

Is there a good way to handle the exception and unwrap and send it to @ControllerAdvice class?

halfer
  • 19,824
  • 17
  • 99
  • 186
Lolly
  • 34,250
  • 42
  • 115
  • 150
  • can't you create a `@ControllerAdvice` for `ExecutionException` and inside it check what it the cause of it? via `Exception::getCause` and delegate both the usual `UserNameNotFoundException` and `ExecutionException` controller advices to the same common logic? – Eugene Jun 18 '21 at 00:39
  • 1
    What’s the point of `catch (CompletionExecution e) { throw e; }`? The effect of catching and throwing again is like not catching it in the first place. So you end up with throwing a `CompletionExecution`, not an `ExecutionException`. And why is the result of the operation a variable (`response`) that is not in scope? Is it supposed to be `resultClass`, the variable you declare and initialize at the beginning of the method? – Holger Jun 18 '21 at 07:20
  • 1
    (Note: I have downvoted here, for the several spelling errors introduced into your code when writing it out here. Code should always be pasted in order to not make these avoidable mistakes. I go easy on beginners here, but not 28K users). – halfer Jun 19 '21 at 13:57

1 Answers1

2

Your code has several errors, like referring to a variable response that is not declared in this code and most probably supposed to be the resultClass declared at the beginning. The line
ClassA classA = classAFuture.join(); suddenly ignores that this future encapsulates an Optional and there are missing ) and ; separators.

Further, you should avoid accessing variables from the surrounding code when there’s a clean alternative. Also, using allOf to combine two futures is an unnecessary complication.

If I understood your intention correctly, you want to do something like

public Response getResponse() {
    return CompletableFuture.supplyAsync(() -> service.getClassA())
        .thenCombine(CompletableFuture.supplyAsync(() -> {
            try {
                return service.getClassB();
            }
            catch(ExecutionException e) {
                throw new CompletionException(e.getCause());
            }
        }), (optA, b) -> {
            Response response = new Response();
            optA.ifPresent(a -> {
                a.setClassB(b);
                response.setClassA(a);
            });
            return response;
        })
        .join();
}

The key point to solve your described problem is to catch the most specific exception type. When you catch ExecutionException, you know that it will wrap the actual exception and can extract it unconditionally. When the getClassB() declares other checked exceptions which you have to catch, add another catch clause, but be specific instead of catching Exception, e.g.

try {
    return service.getClassB();
}
catch(ExecutionException e) {
    throw new CompletionException(e.getCause());
}
catch(IOException | SQLException e) { // all of getClassB()’s declared exceptions
    throw new CompletionException(e); //     except ExecutionException, of course
}
Holger
  • 285,553
  • 42
  • 434
  • 765
  • But that would throw a `CompletionException`? The OP is looking to throw whatever is in `e.getCause` so that his/her `@ControllerAdvice` is triggered – Eugene Jun 18 '21 at 10:31
  • 2
    @Eugene the OP only talked about an unintended `ExecutionException`. Besides that, the answer shows how to catch such an exception and extract the cause, so any adaptation to other wrapper exception type should be a no-brainer. – Holger Jun 18 '21 at 10:52
  • youre right, but the no brainer part was also to slightly refactor the initial advice in spring. I think a slight edit would be appropriate, imho. I perfectly get your point, though. – Eugene Jun 18 '21 at 11:06