Add ErrorControl

[LukaJCB]

Should fix #2229
Bikeshedding welcome :)

[LukaJCB]

For some reason Scala can't seem to find an implicit ErrorControl[Kleisli[Option, Int, ?], Kleisli[Id, Int, ?], Unit] even though it is defined in the companion object of Kleisli. Anyone know a way to help scalac here?

[kailuowang]

what if you call Kleisli.catsErrorControlForKleisli[Option, Id, Int, Unit] directly? does it complain that couldn't find ErrorControl[Option, Id, Unit]?

Edit : oh you mean that implicit val is required to help scalac there?

[LukaJCB]

Yeah it won't compile without it :/

[kailuowang]

hmm, since StateT has exactly the same shape but passes with an implicit Monad[G], I wonder what if you add Monad[G] to Kleisli might help?
Generally, when I get into this type of error (the implicit method is available and legit but scalac refuses to find it) I'll just throw in a Lazy and see if it helps. but that's not an option here. And I suspect that it's not the same scalac bug.

[LukaJCB]

Nope, I needed to add the extra implicit to WriterT and StateT as well, so it's the same problem.

[kailuowang]

(╯°□°)╯︵ ┻━┻ time!
Just to experiment, what if add them to the companion of ErrorControl? (maybe it's easier to find if they are togther? )

[LukaJCB]

Good idea, I'll try that :)

[kailuowang]

M: Monad[G] no longer required since it's provided by ErrorControl right?

[LukaJCB]

Yep, will fix, this was needed when G was an Applicative so that StateT[G, S, A] could be an Applicative

[oleg-py]

Ok, I'm gonna bikeshed here :) This is my opinion, but without proposed resolution.

I'm not convinced yet it's the best way to handle errors, so would at least want some discussion before it becomes part of cats.

First, let's point out that it seems possible to write a "@jdegoes transformer" to give precise exception type on top of stuff that has MonadError[F, Throwable] instance (as shown here). This leaves us with a single MonadError instance that cannot be given Bifunctor-y type: MonadError[Option, Unit]. IO and even Future might be given precise error type parameter.

This might have very negligible impact on performance for existing typess, so it's alone is worth pursuing IMO, as right now our only option for typed errors is EitherT.

The proposed abstraction seems to capture the relationship between exceptional and unexceptional types, but it doesn't do it (at least I don't see it straight away) for e.g. "restricted" exceptional IO and unexceptional one (IO[DomainError, A] <-> IO[Throwable, A] or for transforming between error types inside), so it's not powerful enough on its own to be used in generic context, which will reduce the nicety of its usage (e.g. not being able to use context bound syntax and having a lot of type parameters; Parallel suffers from the same problem).

[codecov-io]

Codecov Report

Merging #2231 into master will increase coverage by 0.02%.
The diff coverage is 96.26%.

@@            Coverage Diff             @@
##           master    #2231      +/-   ##
==========================================
+ Coverage   95.05%   95.08%   +0.02%     
==========================================
  Files         333      337       +4     
  Lines        5789     5896     +107     
  Branches      211      226      +15     
==========================================
+ Hits         5503     5606     +103     
- Misses        286      290       +4

Impacted Files	Coverage Δ
core/src/main/scala/cats/data/IndexedStateT.scala	89.24% <ø> (ø)	⬆️
testkit/src/main/scala/cats/tests/CatsSuite.scala	70% <ø> (ø)	⬆️
core/src/main/scala/cats/data/Kleisli.scala	97.93% <ø> (ø)	⬆️
core/src/main/scala/cats/instances/either.scala	100% <100%> (ø)	⬆️
...ws/src/main/scala/cats/laws/ErrorControlLaws.scala	100% <100%> (ø)
...scala/cats/laws/discipline/ErrorControlTests.scala	100% <100%> (ø)
core/src/main/scala/cats/data/EitherT.scala	97.77% <100%> (+0.12%)	⬆️
core/src/main/scala/cats/data/OptionT.scala	97.8% <100%> (+0.33%)	⬆️
core/src/main/scala/cats/instances/option.scala	100% <100%> (ø)	⬆️
core/src/main/scala/cats/syntax/errorControl.scala	72.72% <72.72%> (ø)
... and 5 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ef05c76...86ff72a. Read the comment docs.

[LukaJCB]

The EitherEff "transformer" would require all existing code using IO or Task to completely change all of their code, whereas this will be a strictly backwards compatible change. Nothing stops us from adding additional bifunctorial error handling abstractions or bifunctorial IO types.

I hear your point about context bounds, but that just comes with the territory, we can't use them with any of the mtl classes, so it's some necessary boilerplate that's to be expected in some parts.
F[_], G[_] is just 3 characters longer than F[_, _] 😁

[oleg-py]

The EitherEff "transformer" would require all existing code using IO or Task to completely change all of their code, whereas this will be a strictly backwards compatible change.

Anybody who wants to take advantage of new abstraction would have to change their code. I don't see how bifunctor version would break anything (except if we remove MonadError too, which I didn't propose to do).

Nothing stops us from adding additional bifunctorial error handling abstractions or bifunctorial IO types.

Yes. It's a question whether we should do stuff, not whether we could. Adding something to cats might be easy, but removing it is likely to be nearly impossible. And I do quite like the minimalism in cats API. So I'd rather not add stuff if it will become obsolete (like I said, I'm not sure what approach is the best, so that is a possibility).

F[_], G[_] is just 3 characters longer than F[_, _]

It's more like [F[_, _] : ErrorControl] vs [F[_], G[_]](implicit F: ErrorControl[F, G, _])

[johnynek]

I don't like using abstract vals on traits since it invites the val init order bug. Can we make these def?

[johnynek]

I like to use abstract types as "output types". It seems to me we might want G[_] to be an abstract type here, since we might often want to write algorithms to work in F go through G, but return an F. Then we could accept an implicit ErrorControl[F, E] to do that, which might be nice.

[LukaJCB]

I like that as well, see my recent issue in parallel, though I'm not 100% sure if it applies here, have to think about it more.

[LukaJCB]

I don't think we'd be able to express these https://github.com/typelevel/cats/pull/2231/files#diff-aadaecd5f11de32b7cdacd702c9a5393R38 maybe you know a way?

[johnynek]

should this be catsErrorControlForOptionT?

[johnynek]

seems like a def control[A,B](fa: F[A])(f: Either[E, A] => G[B]): G[B] might be a nice function to have. It is more general than controlError and can also be used to implement trial. I think users might often want this variant personally.

The name might not be great, but I would love to see this and have syntax for it.

[jdegoes]

Given @LukaJCB's cool adapter ("@jdegoes transformer" 😆), I personally prefer the much simpler:

trait Fallible[B[_, _]] extends Bifunctor[B] {
  def applicative[E]: Applicative[B[E, ?]]
  
  def attempt[E1, E2, A](b: B[E1, A]): B[E2, Either[E1, A]]
  
  def fail[E, A](e: E): B[E, A]
}

This is a sufficient basis to implement everything else, and has very precise types and laws.

The only disadvantage, if you consider it such, is that it requires you use an adapter in order to use the type class.

[LukaJCB]

Anybody who wants to take advantage of new abstraction would have to change their code. I don't see how bifunctor version would break anything (except if we remove MonadError too, which I didn't propose to do).

Right, I don't disagree here, though with ErrorControl you can keep using IO[A] and make use of more principled error handling by making use of the type class to get to UIO[A]. I've been using this for some time and it works well for me. People can keep using their IOs and Tasks, without changing anything. Whereas once you go to IO[E, A] as your base type you have to change a LOT. Also bifunctor IO seems super promising and I really like the idea, but no one has actually had any real experience with it in practice yet AFAIK.

For existing applications I think this is the better way forward. Cats-1.0.0-RC1 was just released and we aren't going to change the nature of IO anytime soon.

Yes. It's a question whether we should do stuff, not whether we could. Adding something to cats might be easy, but removing it is likely to be nearly impossible. And I do quite like the minimalism in cats API. So I'd rather not add stuff if it will become obsolete (like I said, I'm not sure what approach is the best, so that is a possibility).

Again, I agree in principal, but I see these two as rather orthogonal (kind of like how Bifunctor and Functor are orthogonal). I think there's room in the future to add a type class like you're suggesting, without either becoming obsolete.

[oleg-py]

Whereas once you go to IO[E, A]

I'm not proposing to change all existing effect types, it is probably not feasible with current ecosystem state and b/c the best approach is not yet set in stone. Newtyping would be the way to go here, so we will have e.g. IO[A] and StrictIO[E, A] versus IO[A] and EitherT[UIO, E, A].

We will also be able to provide e.g. a StrictFuture[E, A] using the same machinery for impure folks :)

It's also more general than unexceptional types. To represent lack of errors, you can use StrictIO[Nothing, A] (well, you probably need an alias for Nothing for scalac to not complain). To represent possibility of some errors with UIO, you need Either.

no one has actually had any real experience with it in practice yet AFAIK.

Similar can be said about three-parameter version of ErrorControl :) It might be worth just to back off a bit and experiment with different options we have.

[LukaJCB]

I'm not proposing to change all existing effect types, it is probably not feasible with current ecosystem state and b/c the best approach is not yet set in stone. Newtyping would be the way to go here, so we will have e.g. IO[A] and StrictIO[E, A] versus IO[A] and EitherT[UIO, E, A].

👍 Fully agree here.

It's also more general than unexceptional types. To represent lack of errors, you can use StrictIO[Nothing, A] (well, you probably need an alias for Nothing for scalac to not complain). To represent possibility of some errors with UIO, you need Either.

I agree here as well, however I'd like to argue UIO is a special case because it's completely error free. I see it very similar to how NonEmptyList is a special case of List, both are special because they give very specific guarantees, never halting execution for UIO, and having atleast one element for NonEmptyList. Both have more general solutions in BIO[Nothing, A] and shapeless.Sized[L <: Nat], but they're still very useful because they encode very specific invariants.
I certainly believe that even if we had full dependent types, something like the Reducible or NonEmptyTraverse type classes or a SizedList[> 0] would be still be very valuable, because of their guaranteed invariants.

Similar can be said about three-parameter version of ErrorControl :) It might be worth just to back off a bit and experiment with different options we have.

That's fair, I've been experimenting with both approaches for some time now, and concluded for myself that existing code bases are probably better of leveraging unexceptional types, which provide more interoperatability with existing code.

Of course, that's only my opinion and my experimentation has only been going on for a couple of months, so my conclusion might still be a bit premature. :)

I would certainly love to hear more input from others.

Btw, implementing ErrorControl in terms of Fallible is totally possible:

implicit def errorControlForFallible[F[_, _]: Fallible, E]: ErrorControl[F[E, ?], F[Nothing, ?], E] =
  new ErrorControl[F[E, ?], F[Nothing, ?], E] {
    def controlError[A](fa: F[E, A])(f: E => F[Void, A]): F[Void, A] =
      Fallible[F].attempt(fa).flatMap {
        case Right(a) => pure(a)
        case Left(e) => f(e)
      }

    def accept[A](fa: F[Nothing, A]): F[E, A] = fa.leftMap(_.absurd[E])
  }

[wedens]

There was a typeclass TransLift in cats with an abstract member. It required some utterly horrible machinery for syntax: https://github.com/typelevel/cats/blob/v0.9.0/core/src/main/scala/cats/syntax/transLift.scala. Perhaps there is a better way when abstract type in not a typeclass.

[LukaJCB]

Thanks for that link @wedens!

Another problem is that unlike Parallel, you'll often want to access the G[_] value:

def foo[F[_]](fi: F[Int])(implicit EC: ErrorControl[F, String]): EC.G[Int] =
    F.intercept(fi)(_ => 0)

Now we have to go into the type class instance and grab the type from there, not really pretty unfortunately.

[kailuowang]

If we do make G an abstract, then we probably want to provide the ErrorControl.Aux as well.

object ErrorControl {
   type Aux[F[_], G0[_], E] = ErrorControl[F, E] { type G = G0 } 
}

so that user can write

def foo[F[_], G[_]](fi: F[Int])(implicit EC: ErrorControl.Aux[F, G, String]): G[Int] =

Then user can basically pick to use this Aux or E.G whichever fits in the situation.
If G is uniquely determined by F then abstract type with Aux makes sense. You can get some benefits as @johnynek pointed out.

The syntax machinery in TransLift is a bit horrifying but that's a different kind. ((* - > *) -> * ) -> ((* - > *) -> * ) -> * -> *, v.s. ( * -> * ) -> (* -> *) -> * -> * . I speculate that ErrorControl.Aux would be easier to provide syntax to G[_].

[LukaJCB]

Right that makes sense, thank you @kailuowang!

Though now I'm more leaning towards keeping the two type constructors, as it doesn't seem to provide a large benefit and adds complexity for people not familiar with the Aux pattern or path dependent types.

[LukaJCB]

Anyone have any additional comments? I don't want to rush this, but interested in more feedback :)

[kailuowang]

TBH, I'm still not sure how I feel about adding this to cats.core. More discussion would help. The abstraction itself is sound and meaningful, what I don't know is whether its practical benefits justify adding it to Cats.core.
The common strategy in FP I've seen so far is to have the effects being handled at the edge. This abstraction allows users to completely remove the error effect in the middle of an application. What are the scenarios in which can we handle ALL errors in the middle of an application? It seems to me that it must satisfy the following conditions:

1. the result data is absolutely optional for further computation, either an optional side effect or Option of something, i.e. F[Option[A]] or F[Unit].
2. More importantly, we know for sure that whatever error occurs in this scenario, it's exactly the same domain meaning as getting an empty result.

And as soon as the computation involves a sub-computation that does not satisfy the conditions, the whole computation has to maintain the error effect. So it seems to me that the scope of this abstraction would be rather limited within an application.

A real-world application example would really shed more light on this. Without it, IMO, it would be hard to convince people that this is actually more practical than BIO

[kubukoz]

I, for one, would love to revive this PR. A useful scenario is having F=EitherT[IO, NEL[E], ?] and G=IO. You define your http4s router for G, handling errors from F, e.g. by doing controlError(errors => BadRequest(errors.asJson)).

This would make it much easier to work with sealed error ADTs that aren't also Throwables (while working with single-functor IO). While IO can still fail, Throwables can be handled on another layer.

@kailuowang do you have any thoughts after this time? @LukaJCB do you still consider ErrorControl worth having?

I'd also love to read what @SystemFw thinks about this.

[LukaJCB]

I'm still quite fond of this little type class, though I admit it's not that strong an abstraction. But from my time with using UIO and IO, it came in handy :)

[kubukoz]

WDYT about publishing it as a small separate lib for now? Just so we don't have to commit to having this in cats in the long term.

[kailuowang]

I am +1 on experimenting it with a separate lib or module.

[LukaJCB]

An extra module sounds good 👍 though I probably won't have any time to do it anytime soon :(

[kubukoz]

I created a repo for this, we can transfer to you later @LukaJCB, hope you don't mind :) I'll try to publish some time this week. error-control

[LukaJCB]

Not at all, thanks for running with it :)