Make dispatcher safer

[Baccata]

The current Dispatcher suffers from the following problems :

• algebraic product components of the effect are lost at the point of dispatch (WriterT's log)
• algebraic coproduct components (EitherT, OptionT) of the effect put the dispatch call at risk of never returning.
• self cancellation of dispatched computations prevent the dispatch call from ever returning

This last two points are pretty bad : even if Dispatcher can in theory work against any effect type, in practice it makes it really easy for the user to shoot themselves in the foot.

The problem could be mitigated by requiring the dispatch methods (unsafeRunSomething) to take a callback construct, that'd allow to witness these three above points :

trait DispatchObserver[F[_]] {

  def onCancel: F[Unit]

  // When the computation returns an algebraic failure and a value
  // cannot be produced (for instance OptionT.none)
  def onAlgebraicFailure(failed: F[Unit]): F[Unit]

  // When the computation returns an algebraic success.
  def onAlgebraicSuccess(success: F[Unit]): F[Unit]

}

See a POC here

The incorporation of such a callback would facilitate writing constructs like this one, allowing for communicating cancellation and algebraic information through unsafe regions, in a way that is similar to what cats-effect-cps does today.

A concrete benefit would be a better integration with libraries like https://github.com/sangria-graphql/sangria, which force an unsafe region onto the user.

[djspiewak]

Isn't this isomorphic to unsafeRunOutcome[A](fa: F[A]): Outcome[F, E, A]?

[Baccata]

@djspiewak it is not, because what do you do with the effect once when you get a successful outcome ? You still have to run it through the dispatcher to get the value, and if the F[A] is OptionT.none, you're still f***d.

The outcome does let you detect cancellation though, but then what do you do with it ? You're in unsafe land with that information, so you need to make another dispatcher call to send it over to safe land, or pass some exception to signal cancellation. The latter is fair enough I guess, but the former is not great due to the having to repeat the dispatch.

Also, Outcome doesn't distinguish between algebraic success and failure, and that distinction is super important in any context that requires dispatcher. The trick of using flatTap along with a mutable var to detect algebraic failure is brilliant, I give you that, but I wouldn't want for users to have to implement it on their side.

Tbh, unsafeRunOutcome would be great if Outcome.Success was split into something isomorphic to Either[F[Unit], (F[Unit], A)], allowing to distinguish between algebraic failure and success.

[iRevive]

I hit the dispatcher issue today.

The future created by the dispatcher hangs forever if EitherT has a left value (EitherT.leftT(error)).

Simplified example:

object TransformerTest extends IOApp.Simple {

  final case class Error(reason: String)

  def program[F[_]: Async, A](name: String, fa: F[A]): F[A] =
    Dispatcher[F].use { dispatcher =>
      implicit val ec: ExecutionContext = runtime.compute
      val future: Future[A] = dispatcher.unsafeToFuture(fa)

      future.onComplete(_.fold[Unit](e => println(s"[$name] error $e"), r => println(s"[$name] result $r")))

      Async[F].delay(Await.result(future, Duration.Inf))
    }

  def run: IO[Unit] = {
    type Effect[A] = EitherT[IO, Error, A]

    val error = toIO(program[Effect, String]("EitherT error", EitherT.leftT(Error("boom"))))
    val success = toIO(program[Effect, String]("EitherT success", EitherT.rightT("success")))
    val kleisli = program[Kleisli[IO, Int, *], String]("Kleisli", Kleisli.pure[IO, Int, String]("test")).apply(42)

    kleisli >> success >> error >> IO.unit
  }

  private def toIO[A](fa: EitherT[IO, Error, A]): IO[A] =
    fa.leftSemiflatMap(e => IO.raiseError(new RuntimeException(s"Something went wrong: $e"))).merge
}

The output:

[Kleisli] result test
[EitherT success] result success

[armanbilge]

@djspiewak isn't this basically your example from https://github.com/typelevel/feral/pull/33#issuecomment-949795887?

[djspiewak]

I'm going to take this out of 3.4.0 since it really requires a lot more thought.

The problem is that the OP is describing a mechanism for algebraically splitting effect layers. This isn't always possible, and even when it is possible, it creates some subtle commutativity issues. It's definitely not something we can solve in short order. We do have a more robust API for Dispatcher though in this release, which should at least give us some room to play with handlers and such to try to make this situation slightly less footgun.