1

I have a method that returns a CompletionStage<> at various points, but it appears that doing so from within a try-catch block causes an "Incompatible types" error:

Incompatible types

Required: CompletionStage<com.foo.Response>

Found: CompletableFuture<? extends com.foo.Response>

Here's the code. I've simplified it as much as possible without potentially removing the culprit:

  public CompletionStage<Response> example(final Optional<String> idMaybe) {

    return idMaybe.map(id -> {

      if (true) { // Simplified example.

        if (!NumberUtils.isNumber(id)) {
          return CompletableFuture.completedFuture(Response.forStatus(Status.BAD_REQUEST));
        }

        final SomeServiceInterface service;
        try {
          service = someClient.getServiceInterface(SomeServiceInterface.class);
        } catch (SomeException e) {
          LOG.error("Error retrieving SomeServiceInterface.", e);
          return CompletableFuture.completedFuture(Response.forStatus(Status.INTERNAL_SERVER_ERROR));
        }

        final Page page;
        try {
          page = someService.getSomethingByStatement(statementBuilder.toStatement());
        } catch (RemoteException e) {
          LOG.error("Error retrieving a thing by statement", e);
          return CompletableFuture.completedFuture(Response.forStatus(Status.INTERNAL_SERVER_ERROR));
        }

        if (page.getResults() == null || page.getTotalResultSetSize() == 0) {
          return CompletableFuture.completedFuture(Response.forStatus(Status.NOT_FOUND));
        }

        if (page.getTotalResultSetSize() != 1) {
          // There should be only one result.
          return CompletableFuture.completedFuture(Response.forStatus(Status.INTERNAL_SERVER_ERROR));
        }
      }

      return CompletableFuture.completedFuture(Response.ok());

    }).orElse(CompletableFuture.completedFuture(Response.forStatus(Status.BAD_REQUEST)));

  }

To narrow the issue down, I removed the failure-case returns one by one, backwards. So first, removed this:

        if (page.getTotalResultSetSize() != 1) {
          // There should be only one result.
          return CompletableFuture.completedFuture(Response.forStatus(Status.INTERNAL_SERVER_ERROR));
        }

Still the same error. So then, removed this:

        if (page.getResults() == null || page.getTotalResultSetSize() == 0) {
          return CompletableFuture.completedFuture(Response.forStatus(Status.NOT_FOUND));
        }

Still the same error. So then, removed this:

        final Page page;
        try {
          page = someService.getSomethingByStatement(statementBuilder.toStatement());
        } catch (RemoteException e) {
          LOG.error("Error retrieving a thing by statement", e);
          return CompletableFuture.completedFuture(Response.forStatus(Status.INTERNAL_SERVER_ERROR));
        }

Still the same error. So then, removed this:

        final SomeServiceInterface service;
        try {
          service = someClient.getServiceInterface(SomeServiceInterface.class);
        } catch (SomeException e) {
          LOG.error("Error retrieving SomeServiceInterface.", e);
          return CompletableFuture.completedFuture(Response.forStatus(Status.INTERNAL_SERVER_ERROR));
        }

That removed the error. (I double-checked that it wasn't because it caused an error anywhere else; it compiles.)

What is different about returning a completable future from within a try-catch block? Or, what's happening that gives it the appearance of being so? And how can I fix the original code?

Community
  • 1
  • 1
Andrew Cheong
  • 29,362
  • 15
  • 90
  • 145
  • look at the code, if you try catch around `Page` does not cause compile error then try catch around `SomServiceInterface` must also not cause compile error. Either your dummy code does not represent correctly your source or you're missing somewhere – Mạnh Quyết Nguyễn May 18 '18 at 17:16
  • @MạnhQuyếtNguyễn - No, I'm not saying _only_ the first one causes an error. The second one probably does too. But just for reasons of variable dependency I "debugged" by removing things backwards. – Andrew Cheong May 18 '18 at 17:25
  • Does `Response` has generic type? – Mạnh Quyết Nguyễn May 18 '18 at 17:28
  • I think so. Here is the actual open-sourced class: https://github.com/spotify/apollo/blob/master/apollo-api/src/main/java/com/spotify/apollo/Response.java – Andrew Cheong May 18 '18 at 17:32
  • I think you should try to give CompletionStage a concrete type. – Mạnh Quyết Nguyễn May 18 '18 at 17:47
  • Hm, thanks but, didn't work... tried a few things. – Andrew Cheong May 18 '18 at 17:50
  • I tried on my local. https://pastebin.com/r4dGGSNZ. If I remove the concrete type, i got compile error. So that's your problem too – Mạnh Quyết Nguyễn May 18 '18 at 17:51
  • Thank you for going so far! I see in your example you were using `new Response<>(...)` but I'm not sure I can do that with `Response.forStatus(...)`... but you gave me an idea. I had seen this kind of generic specification before, so I tried it and it worked. (See answer.) – Andrew Cheong May 18 '18 at 17:55

1 Answers1

0

I think @MạnhQuyếtNguyễn had the right idea in the comments, but as I couldn't easily structure my code his way, I found another solution:

return creativeIdMaybe.<CompletionStage<Response>>map(creativeId -> {

So after all, I think it was the map that was having an issue resolving the type. (I think. I don't know.)

Andrew Cheong
  • 29,362
  • 15
  • 90
  • 145
  • So finally it works? Nice – Mạnh Quyết Nguyễn May 18 '18 at 18:06
  • Yes. I wish I understood exactly what was happening but, oh well. I'll come back and update this answer in the future when I know more. Thanks for your help! – Andrew Cheong May 18 '18 at 18:23
  • 1
    The compiler is not able to infer the right return type of the `map()` call because it is followed by an `orElse()`. You thus have to hint it with the actual type. – Didier L May 19 '18 at 19:50
  • @DidierL - Feel free to post as answer. I see that that was indeed why now. – Andrew Cheong May 19 '18 at 19:55
  • 1
    I was actually looking for a good canonical duplicate question. I think I found one – hope this helps :) – Didier L May 20 '18 at 11:17
  • Thanks @DidierL! Though, still not sure what the try-catch has to do with it all. – Andrew Cheong May 20 '18 at 20:45
  • I don't think it is the try-catch itself, more likely it is its content. For example, methods that declare a generic type only used for the return type and not the arguments are more likely to cause this, like `Response.forStatus()` and `Response.ok()`. – Didier L May 21 '18 at 11:01