Nifty implicit conversions to Scala's Future to IO

[hilios]

I've been lifting the in Scala's Future to Async for a while now and came up with a simple implicit conversion for this:

final implicit class FutureOps[+T](f: Future[T]) {
  final def toIO: IO[T] = IO.async { callback ⇒
    f.onComplete {
      case Success(v) ⇒ callback(Right(v))
      case Failure(e) ⇒ callback(Left(e))
    }
  }
}

I was wondering if this is probably such common pattern in our daily work – that requires us to interact with libraries that expose Futures – so would make sense to add to the cats.effect.implicits package.

Do you guys have anything against adding this conversion?

If not so, let me know what the guidelines I should follow to submit a PR and contribute to this awesome project.

Thanks in advance!

[SystemFw]

we already have that conversion IO.fromFuture(IO(yourFuture)) :)

[hilios]

I am aware of that but looked a little bit of cumbersome IMO and also remain the proposal to add a implicit conversion.

final implicit class FutureOps[+T](f: Future[T]) {
  final def toIO: IO[T] = final def toIO: IO[T] = IO.fromFuture(IO(f))
}

[LukaJCB]

I don't think implicit conversions are a good feature especially in a library as general as cats-effect.

The problem with this conversion is basically that it looks like you're delaying all effects untill you actually run an IO, but with your method once you have a Future it's already running that effect giving you a false sense of security, so I'm 👎 on this, if you need to convert your futures use the one provided that's guaranteed to be safe :)

[alexandru]

@hilios note that your implementation does not reflect the reality, which should be:

IO.fromFuture(IO.pure(f))

IO.pure because f is an already known Future value and so there's no point to delay anything via IO.apply. Which can catch users by surprise. Note that I'm not opposed to a function signature like this, which is what I actually proposed back in the early days of the project:

def fromFuture[A](fa: Future[A]): IO[A]

This is because the signature makes the evaluation model pretty clear imo and the IDE shows you the type of that parameter as soon as you have to supply it. And also in terms of naming, this is pretty clear:

IO.fromFuture(launchMissiles())

But then such a function is much more explicit than the extension method that you're proposing here.
Consider how it looks in code:

launchMissiles().toIO

Usability is not good, because the signature of toIO gets noticed after the fact if at all. Readability isn't good either.

If this will ever happen, it will happen part of a type-class like the one proposed at #73; when I wrote that issue, I was thinking of Future, but the same argument can be used against that one as well. The @typeclass mechanism that we have (via Simulacrum) would provide a .toIO extension method for things implementing this ToIO type class. But that's a bit of a turn-off.

Also in the case of cats-effect we are wary of adding stuff, because it's a foundational library that other libraries depend on (aka middleware). So more useful utilities like converting from Java's Future type might get rejected as well: #160

[hilios]

Guys, thanks so much for so elucidative and prompt feedback.

I agree that the implicit might cause the wrong impression because of the Future’s default behavior and a type class would be the preferred way to go in this matter.

Cheers.

[monzonj]

Just wanted to drop here that I was using IO.fromFuture(IO(future)) with http4s and was having deadlocks (during gatling tests with hundreds of concurrent requests, the fromFuture would hang and block any incoming request)

Using the solution here by @hilios instead of IO.fromFuture fixed it for me.

Not saying that IO.fromFuture has any problems, it is probably due to some wrong implicit. But i was unable to identify it.

[vasilmkd]

@monzonj Are you running blocking code in the futures?

[monzonj]

Hi @vasilmkd thanks for replying

Inside the future there is some IO, but even without any blocking the IO.fromFuture hangs. For instance:

IO.fromFuture(IO(Future.successful(object)))

I will try to create an isolated http4s app with this code to see if it's a real issue, or just the setup of my project.

[vasilmkd]

If you can, please create a small app, note the versions of the libraries used (preferably a small sbt project) and we'll take a look. A reason why IO.fromFuture might lead to a deadlock is as I said blocking I/O or blocking actions inside the futures. IO.fromFuture brings the code that would be executed in the Future and executes it on the bounded thread pool of IO, and it might just happen that all of the available threads get blocked on some condition.

The reason why using the code from this post "works" is because the Futures that you create start execution immediately upon their declaration (I'm guessing you had to add the scala.concurrent.ExecutionContext.Implicits.global import or something similar, please correct me if I'm wrong) and what this essentially does is offload the blocking I/O or blocking code to the global execution context provided by Scala, which has support for adding extra threads when necessary, to handle the blocking load.

[alexandru]

@monzonj this indicates to me that there's a concurrency issue — the difference between @hilios's solution and the official IO.fromFuture(IO(future)) is that the later may not execute the future-producing task before hand. You might have an ordering issue.

What version of Cats-Effect are you using?

[monzonj]

Hi

@vasilmkd Indeed I'm using scala.concurrent.ExecutionContext.Implicits.global

@alexandru using http4s 0.21.26 which comes with cats-effects 2.5.1

[djspiewak]

@monzonj This definitely seems concerning! Would you mind opening a new issue and adding whatever details you can? It's particularly helpful if you can get anything approaching a shareable reproducer. My guess based on very little other than spidey sense is the same as Vasil's: it's probably some blocking operation within the Future, though Alex's hunch about a race condition is also possible.