New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bracket implementations for all transformers ignore aux effects in release #371

Closed
catostrophe opened this Issue Sep 23, 2018 · 5 comments

Comments

3 participants
@catostrophe
Contributor

catostrophe commented Sep 23, 2018

import cats.data.{Chain, WriterT}
import cats.effect.{ExitCode, IO, IOApp, Sync}
import cats.effect.implicits._
import cats.implicits._
import cats.mtl.FunctorTell
import cats.mtl.implicits._

object Test extends IOApp {

  def bracketWithTell[F[_], A](a: A)(implicit F: Sync[F], W: FunctorTell[F, Chain[String]]): F[ExitCode] =
    a.pure[F].bracket { x =>
      F.delay(println(s"used $x")) *> W.tell(Chain.one(s"used $x")) as ExitCode.Success
    } { x =>
      F.delay(println(s"released $x")) *> W.tell(Chain.one(s"released $x"))
    }

  override def run(args: List[String]): IO[ExitCode] =
    bracketWithTell[WriterT[IO, Chain[String], ?], Int](1).run.flatMap {
      case (log, code) => IO(println(s"writer log: $log")) as code
    }
}

Output:

used 1
released 1
writer log: Chain(used 1)

Expected:

used 1
released 1
writer log: Chain(used 1, released 1)

The same holds for all implementations in Sync.

Is it wrong? Do we need laws for release?

UPD. The same happens in Async.asyncF.

@LukaJCB

This comment has been minimized.

Collaborator

LukaJCB commented Sep 23, 2018

Check out some of the history here: #113 (comment)

@catostrophe

This comment has been minimized.

Contributor

catostrophe commented Oct 13, 2018

bracketCase is fixed in #374, #365

@catostrophe

This comment has been minimized.

Contributor

catostrophe commented Oct 13, 2018

Can we think of a more reasonable implementation of Async.asyncF for transformers?
IO itself propagates effects occurring in asyncF, but handles exceptions in a special way (just prints error backtrace to stderr).

  1. What behavior can we expect from StateT, WriterT? I suppose, to propagate changes in state and log.
  2. From EitherT, OptionT? Should they ignore failures just as IO does or should they propagate?
  3. From IorT, which mixes behavior of both WriterT and EitherT? Depends on the previous decisions.

I would like to have them consistent.

@alexandru

This comment has been minimized.

Member

alexandru commented Oct 15, 2018

IO itself propagates effects occurring in asyncF, but handles exceptions in a special way (just prints error backtrace to stderr).

Throwing exceptions in the functions passed to async and asyncF is in the "undefined behavior" territory.

"Fixing it" implies that the callback injected is thread-safe and protected against being called multiple times. As it is, we cannot guarantee such a behavior, even if IO does it. It's a contract violation to call that callback multiple times (JavaScript style). And so we cannot guarantee correct behavior without expensive synchronization, which is why we cannot demand it by law.

@alexandru

This comment has been minimized.

Member

alexandru commented Oct 15, 2018

Or to put it in other words — the only way to signal errors in async and asyncF is via the provided callback.

Signaling errors in any other way is a contract violation. Some implementations may choose to synchronize on that callback and if possible invoke it and some implementations will simply log the error. And I don't think we should do anything more than that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment