Proposal: provide Sync-like class without MonadError about cats-effect HOT 24 CLOSED

Heads up, @johnynek. Continuation of this: typelevel/cats#1639 (comment)

I have a couple counter-arguments here.

Undefined Behavior

If Sync does not extend MonadError, then the behavior of the following code is entirely undefined:

Sync[F].delay(throw t)

This is deeply problematic, especially since exceptions can arise innocently from otherwise-pure sources. For example, require is often used in pure APIs to dictate API invariants at function boundaries, and it will raise an IllegalArgumentException eagerly if it fails. DivideByZeroException is another one that will arise from seemingly-benign code, and when the evaluation of that code is deferred, it's not clear where the exception should "appear" unless we form a lawful MonadError.

Encouraging Unsound Usage

Even setting aside questions about the suitability of Streaming as an abstraction for streaming computation (as opposed to just a lazy list), defining Sync such that Eval is a valid inhabitant would be deeply concerning from a conceptual standpoint. If Eval can represent an effect, then copoint is by definition an unsafe function. However, Eval forms a Comonad, meaning that code can and will use copoint as if it is pure, if only in abstracted position. Even if we exclude exceptions, it should be obvious that this is a dangerous state of affairs.

It simply isn't sound to use Eval to encapsulate effects, even effects which do not throw. Eval[A] should be thought of as a better => A with a stack-safe flatMap, and absolutely nothing more.

Non-Singleton Abstraction?

Edit: Added this section.

Another problem with this idea is simply the question of how many different non-error-handling Sync-ish things are there? We have Eval, for one, and then beyond that we have… things that delegate to Eval? An abstraction with only one non-inductive instance isn't a very useful abstraction.

Questioning the Motivation

As I said on Gitter, I would actually turn this whole question on its head: why do you need Sync for Streaming (and similar use-cases)? The Streaming example is a particularly strong one, because it's literally just scalaz.StreamT, which only imposes a Monad constraint. Granted, scalaz's point function takes a by-name parameter, but that's a relatively recent change. StreamT predates it by quite some time.

The only function that Sync offers above MonadError is suspend. If your code (i.e. not client code, but rather abstraction code) is not attempting to suspend effects, then why would you need this function? Beyond this, Sync does offer the nominal guarantee that its constituents define a stack-safe flatMap, but Streaming doesn't technically need this any more than StreamT does. Simply stipulate that Streaming is stack-safe iff the constituent monad is stack-safe, assuming that Streaming cannot be written in terms of tailRecM.

Workarounds

As pointed out on Gitter, EitherT[Eval, Throwable, ?] forms a Sync, and with a type alias and some implicit syntax, you can make this nearly invisible. So if you really, really want Streaming to be used with effects, and you really want Eval to be the core of those effects, this is your way out.

from cats-effect.

You're right about Sync[F].delay(throw t), just mentioning that this can also happen with F.unit.map(_ => throw t), which will have the same effect.

I actually used F.unit.map and F.unit.flatMap in the past, in absence of a Sync class, until @mpilquist told me that they made those ops strict for F.unit in FS2, for optimisation purposes.

from cats-effect.

from cats-effect.

@mpilquist roger, I modified the title and the description; the alternative would be to have a MonadSuspend as the top type.

from cats-effect.

I'm +1 on something like this.

I don't find the throw arguments super compelling since throw causes havoc with basically every typeclass we have in cats.

I want to be able to model lazy mutable variables. I don't see why delay should be singled out to have to deal with throw. I get that the difference is that the caller shifts pain, but most code that is a caller in one place is a callee in another. So, I don't find the distinction especially meaningful in practice.

from cats-effect.

@johnynek If you want to model lazy mutable variables, then you probably want people to be careful with extract, but that's exactly the opposite of what any Comonad will do. Eval is the clear exemplar of a typeclass like what is proposed in the OP, but Eval cannot soundly model lazy mutable variables because the mutation can be re-run arbitrarily by code which abstracts over Comonad.

Also, I point back to my "Eval is the only non-inductive instance anyone has put forward" objection.

Rebutting directly on the throw point…

I don't find the throw arguments super compelling since throw causes havoc with basically every typeclass we have in cats.

Yes, but every typeclass we have in cats is eager, so that havoc is going to pop up in a reasonable and consistent part of the call chain. The moment you introduce laziness, you end up in crazy land. Eval is very clearly susceptible to this problem, but the difference is that it is a concrete, nominal type. You know when you use Eval that throw is wonky. Abstracting over this concept is another question entirely. Some instances of a MonadDefer might catch exceptions (i.e. those that also form a Sync), some might not, and you have no idea which it is going to be!

from cats-effect.

from cats-effect.

I don't have time to lay out the code in question at the moment.

Given that any code could be called inside an Eval transitively, and any JVM code can throw, I don't see why you find Eval ever acceptable.

from cats-effect.

No, because they're abstracting over evaluation, they aren't encapsulating effects. Like I said, throw is wonky with Eval, but it's wonky in a specific and deterministic way. Everyone understands that. You lose the specificity and determinism once you start abstracting over this stuff, since instances are free to do whatever they want.

I like Eval a lot. I think it's quite good exactly the way it is. But part of its value is in the nominal type. I don't see a need to abstract over it, nor do I see the need (or the soundness!) to use it to encapsulate effects.

Here's a more concrete example of my point (re: throw):

def foo: Eval[Int] = Eval.always(sys.error("yoink"))

def bar[F[_]: Sync]: F[Int] = Sync[F].delay(sys.error("yoink"))

// hypothetical
def bar[F[_]: MonadDefer]: F[Int] = MonadDefer[F].delay(sys.error("yoink"))

What are the semantics of these three functions? With the first two, we can say for absolute certain what is going to happen. With the third, we really have no idea, because the proposed MonadDefer allows any behavior.

from cats-effect.

Some instances of a MonadDefer might catch exceptions (i.e. those that also form a Sync), some might not, and you have no idea which it is going to be!

This is just an argument for using a typeclass protecting you against exceptions (like Sync) which is not the MonadDefer contract. I don't see why a given typeclass should have a deterministic behaviour in all cases. Why not allow to have some behaviours unspecified?

from cats-effect.

Agreed with @etorreborre here. Addressing the original concerns in order:

1. Undefined Behavior
   I don't care about what F.delay(throw ex) does really, in almost every case; the single outlier is anything which does IO. If I'm not mistaken you used the same argument as I'm using here to make the case that Sync needs to extend MonadError and not the other way around, because the exception catching is an extra behavior which MonadError does not need to be concerned by. Why doesn't this apply to F.delay(throw ex)?
2. Encouraging Unsound Usage
   Sync extends MonadError already encourages unsound usage of MonadError, because some MonadError's catch errors thrown during map and flatMap and raise them. And some don't. Your argument that "every typeclass we have in cats is eager" is immaterial because many typeclasses contain higher-order functions, with the same result. This is to my mind the largest issue with conflating pure errors and exceptions; to my mind, Sync shouldn't extend MonadError in the first place, because it should have entirely different control flow semantics. Exception as a pure error type is not the same abstraction as actually throwing exceptions.
3. Non-Singleton Abstraction?
   What about transformers over Eval? This is a really common case afaik.
4. Questioning the Motivation
   I would question the other way: why provide a stack-safety law at all for Sync, if it's entirely a concern of choosing the individual Sync instance in question? Why is it that a simple thunk-based, non-stack-safe Function0-based IO relative cannot be a lawful inhabitant of Sync? If users should be able to make the choice between stack-safe and not stack-safe implementations, as you mention with StreamingT, why is Sync excluded from this? Does this not imply that there's a need for a "stack-safe monad" type class, which you're satisfying with Sync?

from cats-effect.

@edmundnoble I understand what you're saying, but in my mind if an F[_] data type has MonadError[F, Throwable] implemented, then that MonadError instance should catch exceptions in map and flatMap.

How we should specify that as a law, I don't know, but if MonadError does not have this behavior, its usability is broken since users will expect this.

Why? Because by having a MonadError[Throwable, ?] users will assume that it's sufficient for catching and handling errors. And if it's not, then it's kind of useless.

The problem with exceptions and why I personally don't like them is that throwing an exception represents an alternative exit point for your procedure. A routine throwing an exception is actually not following the rules of structured programming. And having procedures throwing exceptions synchronously is semi-OK, since your current stack gets blown so something in your program will notice it. But if that happens on some thread, on some thread-pool somewhere, then nothing will catch it. And depending on the thread-pool implementation, you won't get a STDERR log either.

from cats-effect.

Alright, then it sounds like you'd be in favor of removing MonadError[F, Throwable] instances in all cases in favor of a MonadException type class. Otherwise you don't actually know where the exceptions are caught.

from cats-effect.

Well, not really, a MonadException would basically be MonadError[?, Throwable] + some extra laws, so why not add those laws directly on MonadErrorLaws somehow?

My problem is that adding an extra type class won't make MonadError[?, Throwable] any less confusing.

from cats-effect.

You can't. It's impossible. MonadError[F, E] works for any E. MonadError[?, Throwable] should just not exist.

Edit: You could go two ways here. Make MonadException[F] extend MonadError[F, Throwable], or completely separate it from the MonadError hierarchy. My vote is to go the second way. I want to know exactly where exceptions will be caught, for the same reason I want to know exactly where Some(null) gets flattened to None.

from cats-effect.

If MonadError[?, Throwable] shouldn't exist, then the number of useful MonadError implementations in the wild goes down dramatically.

I mean, I know there are some useful types out there, like Validated, but I can't think of a single use in the past year where I needed E != Throwable in MonadError.

Which means that its presence in cats-core is now questionable.

from cats-effect.

Validated and Either as well as transformers over them are the only useful cases for MonadError and ApplicativeError, thus they should be in cats-mtl, and MonadException should be in cats core.

If you only use MonadError with E =:= Throwable then you certainly need MonadException instead. MonadError is for MTL programming. If you don't actually want to abstract over how you can raise an arbitrary error type in an arbitrary monad then you don't care about MonadError, and you shouldn't. Especially because right now, MonadError and ApplicativeError remaining in cats-core makes it impossible to have multiple instances of either in scope for the same F[_] without ambiguity over the base Monad[F]. This makes it impossible to use a monad transformer stack with multiple Either or EitherT layers for example.

Just so you understand where I'm coming from, I never use E =:= Throwable in tandem with MonadError or ApplicativeError except for with external libraries. It's always a concrete domain-specific error type. I understand that in the concurrency sphere you likely deal with the other case, and I think that we both win with this change.

from cats-effect.

OK, I might like that.

I hope we come up with a better name than MonadException though, but I can't think of anything good right now.

from cats-effect.

Also, this would be another big change 😔

from cats-effect.

Yeah, it would. Pinging @peterneyens @kailuowang @tpolecat for some more opinions.

@alexandru Other than MonadException you might prefer in MonadTry or MonadCatch. MonadException is a somewhat common variation from Haskell.

from cats-effect.

@edmundnoble

I don't care about what F.delay(throw ex) does really, in almost every case; the single outlier is anything which does IO. If I'm not mistaken you used the same argument as I'm using here to make the case that Sync needs to extend MonadError and not the other way around, because the exception catching is an extra behavior which MonadError does not need to be concerned by. Why doesn't this apply to F.delay(throw ex)?

Wait, you care about what F.map(throw ex) does but not F.delay(throw ex)? That seems a bit weird. My contention is that F.delay is not a meaningful function at all if you aren't specifying what happens with exceptions.

Sync extends MonadError already encourages unsound usage of MonadError, because some MonadError's catch errors thrown during map and flatMap and raise them.

I disagree. It only encourages unsound usage if people assume things about map/flatMap which may or may not hold. In fact, your contention is really that MonadError[F, Throwable] encourages unsound usage, and your argument transcends purely Sync, but I don't see anyone advocating for implicit negation evidence preventing people from defining such instances.

This is to my mind the largest issue with conflating pure errors and exceptions; to my mind, Sync shouldn't extend MonadError in the first place, because it should have entirely different control flow semantics. Exception as a pure error type is not the same abstraction as actually throwing exceptions.

Except Exception is tied to the semantics of throw due to fillInStackTrace(). It's really a very inappropriate type to be using for pure errors, even above and beyond the nominative semantics that people generally attach to it.

What about transformers over Eval? This is a really common case afaik.

I mean non-inductive implementations. Give me a single base type other than Eval (or equivalent; Function0 doesn't count as being separate here) that satisfies a "MonadDefer".

There's another huge, huge problem here which everyone seems to be overlooking: Eval (and all similar types) are copointed. What you're proposing is adding an effect-characterizing typeclass which will almost exclusively contain copointed inhabitants (and inhabitants which are inductive transformers over copointed types). Even pretending that exceptions don't exist, it is entirely inappropriate for something which encapsulates effects to define extract. That basically defeats the whole purpose of such an abstraction.

I would question the other way: why provide a stack-safety law at all for Sync, if it's entirely a concern of choosing the individual Sync instance in question? Why is it that a simple thunk-based, non-stack-safe Function0-based IO relative cannot be a lawful inhabitant of Sync?

Because nearly all programs come down to some sort of IO (e.g. Task, etc) as their base and "final" monad. Since nearly all useful programs are coterminating, this forces the base monad to have a stack-safe interpreter when "run to completion". I consider this to be a sufficiently common use-case (as in, nearly every non-trivial program that uses IO-like things) as to merit a law.

If users should be able to make the choice between stack-safe and not stack-safe implementations, as you mention with StreamingT, why is Sync excluded from this? Does this not imply that there's a need for a "stack-safe monad" type class, which you're satisfying with Sync?

I'm not sure I understand what you mean?

@alexandru

Why? Because by having a MonadError[Throwable, ?] users will assume that it's sufficient for catching and handling errors. And if it's not, then it's kind of useless.

Except MonadError doesn't characterize exception or error catching at all, it simply defines a means for suspending and unsuspending errors within the effect. It doesn't imply that the effect has any special semantics regarding those errors, save for the fact that it is able to suspend them and short-circuit computation. Sync is the only type which implies catching of exceptions in any form, and then it is only defined around suspend.

I can see an argument that users might expect Sync types to catch exceptions in map and flatMap, since as a practical matter, many of them do! But I don't see a practical way to act on that expectation (see my comment on #71). And I certainly don't see why users would have that expectation about MonadError.

from cats-effect.

"Wait, you care about what F.map(throw ex) does but not F.delay(throw ex)? That seems a bit weird. My contention is that F.delay is not a meaningful function at all if you aren't specifying what happens with exceptions."

Why? If I care about one I almost certainly care about the other. F.delay is as much a meaningful function as F.map whether both of them have or don't have laws regarding exceptions; I can throw exceptions from either function value. I think I understand your reasoning for treating the first specially, but people don't just perform potentially-throwing operations in delay calls in IO monads. I think it's a reasonable expectation that code written for Future and Task and IO all works completely fine when converted verbatim to F[_]: Effect, and that isn't the case here because I can pick F[_]'s which will utterly break the code.

"It only encourages unsound usage if people assume things about map/flatMap which may or may not hold. In fact, your contention is really that MonadError[F, Throwable] encourages unsound usage"

Indeed. I'm not going to prevent people providing them themselves, but I don't want to provide them from cats or any non-alleycats subprojects. The connection between using a Throwable as my error type and having concrete semantics for dealing with throw is totally unavoidable, as you're about to say.

"Except Exception is tied to the semantics of throw due to fillInStackTrace(). It's really a very inappropriate type to be using for pure errors, even above and beyond the nominative semantics that people generally attach to it."

Fair point. I don't actually do this myself, I was more speculating. However this is consistent with my position that MonadError[F, Throwable] encourages misuse; it's inappropriate, as you stated, as a pure error type, and if you don't think people would sanely use it for pure errors then what are they using it for? Catching thrown exceptions inside map and flatMap.

"I mean non-inductive implementations."

I don't see why this restriction is necessary because people really need these, just look at etorreborre/origami for an example.

"There's another huge, huge problem here which everyone seems to be overlooking: Eval (and all similar types) are copointed."

What exactly does this relate to? Transformers over Eval are not necessarily copointed, and MonadDefer has nothing to do with capturing side effects, but guarded recursion.

"Because nearly all programs come down to some sort of IO (e.g. Task, etc) as their base and "final" monad. Since nearly all useful programs are coterminating, this forces the base monad to have a stack-safe interpreter when "run to completion". I consider this to be a sufficiently common use-case (as in, nearly every non-trivial program that uses IO-like things) as to merit a law."

Why is it that because the base monad of my entire app is stack-safe every single IO monad I use anywhere in my code has to be as well? Thunk-based IO is really fast and convertible to and from other IO variations; if this wasn't intended what is the purpose of Effect.to?

"Except MonadError doesn't characterize exception or error catching at all, it simply defines a means for suspending and unsuspending errors within the effect. It doesn't imply that the effect has any special semantics regarding those errors, save for the fact that it is able to suspend them and short-circuit computation. Sync is the only type which implies catching of exceptions in any form, and then it is only defined around suspend."

You stated that Throwable is linked to throw, and this is the exact reason why users will assume this. My point is that people use MonadError[F, Throwable] in this way. You care exactly where your exceptions are caught and thrown especially in async contexts, I know you do, you and @alexandru talk about it all the time. If I don't know that delay is coherent with map in this instance then I have no idea where the exceptions thrown during map will end up, so I'll just assume it all works and then when I use the inductive instances for Sync my house of cards will eventually fall (to say nothing of using inductive instances for Async, or refactoring code to use map instead of delay). If I don't care about map and flatMap catching exceptions then why am I using Throwable?

I can see an argument that users might expect Sync types to catch exceptions in map and flatMap, since as a practical matter, many of them do! But I don't see a practical way to act on that expectation (see my comment on #71). And I certainly don't see why users would have that expectation about MonadError.

In my opinion, "I don't see why users would have that expectation about MonadError[F[_], Throwable]" is inconsistent with "Exception is tied to the semantics of throw". People don't just expect this behavior, they rely on it in their programs and without it the control flow is radically different. This is the same reason why using Future as just another Monad in ostensibly pure code is a bad idea.

Not only do people rely on this behavior when using MonadError with Throwable but not providing it removes a number of other derived laws that I would argue are very intuitive. For example: F.delay(u) <-> F.pure(()).map(_ => u) is an obvious law which is ruled out by having delay inconsistent with map.

I took a look at your comment on #71 which states that you can't actually construct a derived Sync instance which catches in map and flatMap, because it would result in incoherence; so people who rely on that property will be screwed anyway if they swap Sync instances. I understand that you want to keep a lot of instances but it looks like everyone who actually uses these instances has bugs hiding in their program because they are relying on behavior which is not necessarily there. Leaving these behaviors unspecified means app-breaking, totally hidden bugs in user code. I don't consider the utility of extra instances to outweigh this.

I can't see how MonadDefer is related to exceptions, so why provide them together? Right now we're stuck in the middle of two different abstractions with different properties and usecases. It looks at first like the Sync laws are all needed by users for an "IO", but they don't actually form a full-fledged IO abstraction, because users are forced to rely on individual properties of the underlying monads.

In the end, the user has five individual requirements of everything which can be considered "IO-like":

1. Stack-safe flatMap and map/guarded monadic recursion (each can be implemented with the other).
2. Side effects.
3. Full exception safety.
4. Asynchronous side effects (which means you also need 2 and 3).
5. runAsync (Effect, requires 4).

I think it's reasonable to expect each one of these to form its own type class, and I can imagine scenarios where a user would want every possible combination of these five except for the ones explicitly disallowed in the parenthesis. Inductive instances remain possible for 1 and 2, and not possible with good reason for 3, 4 or 5.

There is also a possibility of introducing exception-safe map and flatMap functions with different names as an alternate interface for 3, maybe mapCatch and flatMapCatch, which would regain the safety of transformers over 3 and 4 but also basically regain all of the danger of user misuse.

from cats-effect.

Just to be concrete, and sorry for the long delay, what is the problem below?

We can write things like this using only delay. Why are we required to have MonadError to do things like this?

Logging (without passing back errors), caching and maybe other applications seem like they can benefit from Sync extending Monad not MonadError.

import java.util.concurrent.atomic.AtomicReference
import cats.Eval
import cats.effect.Sync

object EvalRef {

  sealed trait Ref[A] {
    def get: Eval[A]
    def set(a: A): Eval[Unit]
    def compareAndSet(expect: A, update: A): Eval[Boolean]
  }

  private class RefImpl[A](ref: AtomicReference[A]) extends Ref[A] {
    def get = Eval.always(ref.get)
    def set(a: A) = Eval.always(ref.set(a))
    def compareAndSet(ex: A, up: A) = Eval.always(ref.compareAndSet(ex, up))
  }

  def apply[A](a: A): Eval[Ref[A]] =
    Eval.always(new RefImpl(new AtomicReference(a)))
}

object HKRef {

  sealed trait Ref[F[_], A] {
    def get: F[A]
    def set(a: A): F[Unit]
    def compareAndSet(expect: A, update: A): F[Boolean]
  }

  private class RefImpl[F[_], A](ref: AtomicReference[A])(implicit s: Sync[F]) extends Ref[F, A] {
    def get = s.delay(ref.get)
    def set(a: A) = s.delay(ref.set(a))
    def compareAndSet(ex: A, up: A) = s.delay(ref.compareAndSet(ex, up))
  }

  def apply[F[_], A](a: A)(implicit s: Sync[F]): F[Ref[F, A]] =
    s.delay(new RefImpl[F, A](new AtomicReference(a)))
}

// Why can't we have the second one and use it with Eval?

from cats-effect.

Closing as this has been overcome by events. Sync[F] extends MonadError[F, Throwable] in cats-effect 1.x. This could change in a future version of the library of course, but I'd like to see such a proposal cover things like support for UIO/BIO as well. Feel free to reopen if you disagree with closing.

from cats-effect.