scala / scala-java8-compat

A Java 8 (and up) compatibility kit for Scala.
Apache License 2.0
436 stars 102 forks source link

Future.recover does not catch exceptions that are thrown inside a CompletableFuture #169

Open mresposito opened 4 years ago

mresposito commented 4 years ago

I've been using the aws dynamodb async sdk, but I find that I cannot catch exceptions after I convert a CompletableFuture to Scala. When I run

    client
      .createTable(tableSchema.createTableRequest(tableName))
      .thenApply[Unit](_ => Unit)
      .exceptionally {
        case _: Throwable => // exception is now property handled
      }

I can successfully catch the exception.

However when I run

     import scala.compat.java8.FutureConverters.toScala

    val cf = client
      .createTable(tableSchema.createTableRequest(tableName))
    toScala(cf).recover {
      case _: Throwable => // never gets executed
    }

I cannot actually catch the exception. Any clues on why?

mberchon commented 2 years ago

FYI, our team is troubleshooting a similar weird behavior between scala-java8-compat and AWS SDK V2 Context:

For some unexplained reasons, our code works fine if keeping a CompletableFuture but doesn't work when turning it into a Future with the scala-java8-compat. /!\ I am not saying there is a bug in scala-java8-compat ... but this issue has been routed to me by the team ... so I am trying to give a bit of reflexion to this issue /!\

1) We did the following tests which confirm exceptions are properly catched

  test("test future compat recover") {

    import scala.compat.java8.FutureConverters.toScala

    toScala(CompletableFuture.supplyAsync(() => "Hello")).futureValue shouldBe "Hello"

    assertThrows[Exception] {
      toScala(CompletableFuture.supplyAsync(() => {
        throw new IllegalStateException("Boom")
        "Hello" }))
        .futureValue
    }

    //Throwing nonFatal exception
    toScala(CompletableFuture.supplyAsync(() => {
      throw new IllegalStateException("Boom")
      "Hello" }))
      .recover { case _:Throwable => "Bim"}
      .futureValue shouldBe "Bim"

    //Throwing fatal exception (https://stackoverflow.com/questions/29744462/the-difference-between-nonfatal-and-exception-in-scala)
    toScala(CompletableFuture.supplyAsync(() => {
      throw new LinkageError("Mega Boom")
      "Hello" }))
      .recover { case _:Throwable => "Bim ?"}
      .futureValue shouldBe "Bim ?"
  }

2) Will update this comment once we flush out the djinn

UPDATE:

Final status in our context:

So definitely not a scala-java8-compat bug but a bug on our side (in our context)