Cats-effect: Provide way to create an `IO[T]` from a `CompletableFuture[T]`

[danielkarch]

It is sometimes necessary to work with Java libraries that return CompletableFutures. It would be nice to have an easy way to turn them into IO values.
This is adapted from Monix:

 def fromJavaFuture[A](cf: CompletableFuture[A]): IO[A] =
    IO.cancelable(cb => {
      cf.handle[Unit](new BiFunction[A, Throwable, Unit] {
        override def apply(result: A, err: Throwable): Unit = {
          err match {
            case null =>
              cb(Right(result))
            case _: CancellationException =>
              ()
            case ex: CompletionException if ex.getCause ne null =>
              cb(Left(ex.getCause))
            case ex =>
              cb(Left(ex))
          }
        }
      })
      IO(cf.cancel(true))
    })

Do you think it would make sense to have this in cats-effect itself?

[alexandru]

I encouraged @danielkarch to add this suggestion, as I wanted to hear other people's opinions.

I don't like the idea of adding lots of utilities to cats-effect, if they aren't used much, because it increases the API surface, becoming harder to maintain and with more potential for API breakage in the future.

Leaning towards no right now, as personally I haven't interacted much with CompletableFuture and I don't mind dumping my own utilities in my own utils package either. But if other people need this, then it might be worth adding.

[rossabaker]

I've implemented this as a util twice: once pre-cancellation, and once in scalaz-concurrent. But the points about surface area are well taken. I think I want this if and only if a few other people speak up for it.

[oleg-py]

I believe it does not belong in cats-effect, whose purpose is to provide a common protocol on which Scala effect types cooperate. But I would love to see something like cats-effect-contrib which would provide the set of such utilities (including the one for regular java Future with polling we recently talked about on Gitter). I believe there's no use having people to reinvent those utils every time.

[danielkarch]

I agree with @oleg-py. It would be nice to have an "official solution" for things like this, but cats-effect itself is not the right place, so perhaps a contrib project would be a good place for it.

[alexandru]

Another project or a sub-project was what I thought of in https://github.com/typelevel/cats-effect/issues/117

I don't know how to set the boundaries though. When I made that proposal I was thinking of Timer, which made it in cats-effect proper and it's a good thing imo that it did.

We can initiate a contrib sub-project, I'm in favor.

[alexandru]

Or maybe it should be a completely separate project?

[oleg-py]

We can start with putting here everything platform-dependent, e.g. Java only or JS only (F: Async from ES6 Promise, anyone?).

[alexandru]

@LukaJCB any reason for why this was closed?

[LukaJCB]

Huh? I have no clue what happened there, I've been traveling the past day, maybe left my phone on in my pocket, sorry about that! 🤦

[rossabaker]

Let's leave this open as a request for the helper and resume the contrib talk on #117. Never mind, that issue isn't what I remembered.

Separate contrib modules are rarely successful, and burdensome to synchronize when successful. I think alleycats is a successful model for stigmatized code. I'm in favor of a submodule here if we do this.

[mpilquist]

FWIW, I'm against a separate contrib module and in support of adding this directly to cats-effect.

[djspiewak]

I agree with @mpilquist. I’m as sensitive to API surface area as anyone but this one seems worthwhile to me.

[jatcwang]

I have a situation where I have an Async[F] constraint, but I have some API where it returns Future. Can we generalize this to be an extension method on Async[F] where you can do:

import cats.syntax.effect.futureConversions._
def doStuff[F]()(implicit m: Async[F]): F[Unit] = {
  m.deferFuture(functionReturningAFuture())
}

where deferFuture is an extension method on Async[F] instance, and you can have deferCompletableFuture from that as well.

[alexandru]

@jatcwang I'm leaning towards no.

In Monix's monix-catnap sub-project we've got a new FutureLift type class that is better, but can't be in Cats-Effect: https://github.com/monix/monix/blob/master/monix-catnap/shared/src/main/scala/monix/catnap/FutureLift.scala

The 3.0.0-RC2 release is scheduled for tomorrow, but you can use 3.0.0-RC2-840c090 to play with it.

[jatcwang]

That looks like a good solution. Will be great if we can pull that up into contrib to provide a standardized way of converting from Futures (both Scala and Java's).

Any particular reason why the signature require a F[Future[A]] instead of => Future[A] (like monix's Task.deferFuture). I see the rationale (so it's harder to accidentally pass in a running Future) but it makes the usage quite noisy. (IO.fromFuture(IO(somethingFuture))) vs (IO.deferFuture(somethingFuture)).

[Daenyth]

@jatcwang I can't speak for the authors but in my opinion (mostly as a user of the library), it's extremely critical. Without forcing F[Future[A]], the potential for crippling footcannons is absolutely massive, because all it takes is a def f: Future[A] changing to a val f: Future[A] to blow out all your referential transparency.

Future's impurity is especially insidious because it's the kind that will compile with both versions, and have behavior that is almost-but-not-quite identical... until it becomes extremely different, and you have a production bug.

[jatcwang]

If you change the definition of a Future expression from def to val, I don't think F[Future[A]] will save you either. I've been a user of Monix for years and I have never had an issue with deferFuture (or a situation where I have a seen a def Future turn into a val in an interface).

[alexandru]

@jatcwang Monix's philosophy is slightly different and for a library it's important to be consistent in the chosen conventions.

I do agree that F[Future[A]] doesn't protect much. At the end of the day, Scala is a strict language and people have to pay attention to how things get evaluated, so people should look for Future[A] versus => Future[A] and should know the difference, even if that's painful. Scala isn't Haskell and that won't change irregardless of what we do in these libraries.

But in Cats-Effect the general agreement is to work with F[Future[A]] because it makes the signature more clear, plus Function1 references are supposed to be pure, unless the method is known to be FFI (e.g. IO.delay). And again, it's important to be consistent.

[DuncanHills]

What was the resolution here?

[jatcwang]

scala def fromJavaFuture[A](makeCf: => CompletableFuture[A]): IO[A] = IO.cancelable(cb => { val cf = makeCf cf.handle[Unit](new BiFunction[A, Throwable, Unit] { override def apply(result: A, err: Throwable): Unit = { err match { case null => cb(Right(result)) case _: CancellationException => () case ex: CompletionException if ex.getCause ne null => cb(Left(ex.getCause)) case ex => cb(Left(ex)) } } }) IO(cf.cancel(true)) })

^ Improved version from the first post. Made completableFuture parameter by-name instead of eagerly evaluating it

[tovbinm]

How about this?

import scala.compat.java8.FutureConverters._

implicit def completableFutureToIO[T](cf: CompletableFuture[T]): IO[T] = 
    IO.fromFuture(IO(toScala(cf)))

[joroKr21]

This is really useful when working with AWS SDK 2 (which I'm sure is very common).

[alexandru]

@tovbinm you're losing the cancelation token when you're converting like that, because Future from Scala isn't cancelable and CompletableFuture from Java is.

@jatcwang's version is better.

Note that monix-catnap, a sub-project of Monix meant for providing utilities for Cats Effect, does have FutureLift, a type class meant for lifting Future-like data types into any F[_] : Async.

import monix.catnap.FutureLift
//...

FutureLift.from(IO {
  myCompletableFuture
})

See API documentation

[djspiewak]

What was the resolution here?

Oh, I never replied to this… Sorry.

The resolution is: use monix-catnip. It really does have excellent solutions to this problem. Like there's definitely a lot to be said for a "batteries included" library, and I appreciate the desire to have more of these kinds of utilities inside of Cats Effect, but we do have to draw the line somewhere, and things like this that are very close to that line end up getting unfortunately left out.

I think this is also a bit of a broader philosophical discussion (out of scope for this issue, but very very welcome in Gitter and/or other issues!) about exactly how many batteries we should be including in this library. CE is, first and foremost, a foundational framework. But at the same time, people very reasonably don't want to go hunting for microlibraries for everything. So is the solution more discoverability? Better lists of "recommended ancillary libraries" for specific tasks? More coordinated release/versioning cycles? More functionality in the core library (CE)? A mixture of all of the above?

These are excellent topics for discussion and I don't know the answer. :-) If you have an opinion on these things though, I would definitely love to hear it in Gitter, or feel free to open a new discussion-labeled issue raising the point.

[jatcwang]

I have changed my opinion on cats.effect.IO.fromFuture vs monix's delayFuture - I don't mind needing to write IO.fromFuture(IO { // make the future happen } ) anymore as it does help to point out that this block of code inside () is side-effecting and need extra care.

Will look at adopting monix-catnip.

And thanks for all your work :) Appreciate the thoughtful replies and I think you're doing great!