search

typelevel / cats-effect

The pure asynchronous runtime for Scala
https://typelevel.org/cats-effect/
Apache License 2.0
2.03k stars 520 forks source link

IO.onError is inconsistent with ApplicativeError #4058

Closed Jasper-M closed 2 months ago

Jasper-M commented 7 months ago

The onError that you get from ApplicativeError has this signature:

def onError[A](fa: F[A])(pf: PartialFunction[E, F[Unit]]): F[A]

While the "native" one in IO doesn't take a PartialFunction:

def onError(f: Throwable => IO[Unit]): IO[A]

That could lead to some surprises if you would rewrite a piece of code from tagless final to IO, or if you are just used to using the onError extension method in other code.

armanbilge commented 7 months ago

I think we can fix this one fairly source-compatibly.

borgben commented 6 months ago

Hey @armanbilge @Jasper-M I'd love to take this on, if you'd assign it to me.

djspiewak commented 6 months ago

Assigned!

lenguyenthanh commented 3 months ago

@armanbilge I'd like to take on this one. As I mentioned in discord, I believe We can implement the onError function like this:

  def onError(pf: PartialFunction[Throwable, IO[Unit]]): IO[A] =
    // ~~handleErrorWith(t => pf.applyOrElse(t, IO.raiseError[A]) *> IO.raiseError(t))~~
    handleErrorWith(t => pf.applyOrElse(t, (_: Throwable) => IO.unit) *> IO.raiseError(t))

But I'm not very familiar with source compatible thingy, could you provide some pointer for me?

armanbilge commented 3 months ago

@lenguyenthanh great, open up a PR!

But I'm not very familiar with source compatible thingy, could you provide some pointer for me?

By source-compatible, what I mean is that we don't break compilation of existing code that's using the old, inconsistent method definition. I think if we add this additional definition and deprecate the old one, it will be okay.