Add error-handling methods to `EitherT` #2237

alexandru · 2018-04-23T08:55:57Z

Addresses #2161 by adding redeem and redeemWith to MonadError.

These are derived by attempt.map and attempt.flatMap respectively:

trait MonadError[F[_], E] {
  // ...

  def redeem[A, B](fa: F[A])(recover: E => B, map: A => B): F[B] =
    redeemWith(fa)(recover.andThen(pure), map.andThen(pure))

  def redeemWith[A, B](fa: F[A])(recover: E => F[B], bind: A => F[B]): F[B] =
    flatMap(attempt(fa))(_.fold(recover, bind))
}

As mentioned in #2161, this represents an optimization on usage of attempt because for certain datatypes (e.g. IO, Task, Future):

it avoids the Either boxing
and even more importantly, it avoids a second map / flatMap operation

The benchmark results in that issue speak for themselves and note that I've measured IO / Task, which have fairly efficient map and flatMap operations. For a data type like Future working with attempt for error recovery is a disaster.

My use-case for redeem / redeemWith is in monix.tail.Iterant that now makes heavy use of attempt for error recovery. This is a streaming abstraction, so attempt.flatMap operations can pile up and add quite significant overhead.

Given that we have Future instances provided by Cats, it would have been a shame to not also provide optimized redeemWith and attempt operations — because a new, abstract Future#transformWith has been introduced in Scala 2.12 as the base function for all other operations (e.g.flatMap, recoverWith).

The difficulty here is to keep compatibility with Scala 2.11- so this commit introduces FutureShims provided with different implementations for different Scala versions. Compatibility is thus maintained even with Scala 2.10.

I also went ahead and enhanced EitherT with the following operations, straight on the class definition itself:

widen
leftWiden
attempt
attemptF
handleError
handleErrorF
handleErrorWith
handleErrorWithF
recoverF
recoverWith
recoverWithF
redeem
redeemF
redeemWith
redeemWithF

So given EitherT[F, A, B], the functions suffixed with F operate on any F: MonadError[F, E] in scope, thus recovering from E errors thrown in the F[_] context.

Two reasons for it:

the F suffixed methods are provided because working with the provided MonadError[EitherT[F, L, ?], E] instance sucks (see make sure that EitherT MonadError syntax works the old way #2029)
the call sites are too heavy for such common operations, so relying on implicit conversions to provide the necessary syntax creates short-term junk
discovery of features for new users is better this way

On performance, the JVM is able to do some limited escape analysis to avoid short-term allocations in certain situations and the garbage collector is usually good at dealing with short-term junk, but irregardless, the JVM has a limited budget for optimizations and the way to think about it is that obvious optimizations at some call sites detract from doing obvious optimizations at other call-sites.

In order to optimize memory allocations and the byte-code of those functions, I've rewritten some of them to do more explicit pattern matching, with a default cast _ => branch.

Benchmarks are a marketing scheme and an implementation that's easier on memory and possibly performance beats an implementation that's elegant. Libraries don't have to be elegant, libraries have to be efficient such that the user's code can be elegant.

Have also written ScalaDocs — which triggered the Scalastyle check for "maximum file size", which I had to disable completely, since it makes no sense to have a restriction that discourages the writing of ScalaDocs.

Unfortunately using a provided MonadError[EitherT[F, L, ?], E] that's explicitly imported in scope no longer works for those functions.

E.g. this sample will trigger an error, hence that particular test is now gone:

test("#2022 EitherT syntax no long works the old way") {
  import data._

  EitherT.right[String](Option(1)).handleErrorWith((_: String) => EitherT.pure(2))
  {
    implicit val me = MonadError[EitherT[Option, String, ?], Unit]
    // Triggers compile-time error!
    EitherT.right[String](Option(1)).handleErrorWith((_: Unit) => EitherT.pure(2))
  }
}

This is one reason for why I don't like type-class driven syntax. Such overloads should not be possible or desirable, being much worse than Java-driven overloads which I also prefer to avoid.

Another argument for doing this is that the previous implementation had inconsistent definitions for recover and recoverWith straight on the case class definition, but not for handleError and handleErrorWith. So you could do the above for handleError, but not for recover.

This means that this PR has a source incompatibility with 1.0.0, but this is not a binary incompatibility 😉 and is in my opinion a valid fix.

We've got the following incompatibilities:

// Internal classes, moved due to refactoring, not a problem
exclude[MissingClassProblem]("cats.syntax.EitherUtil$"),
exclude[MissingClassProblem]("cats.syntax.EitherUtil"),
// Scala 2.11 only — internal classes, not a problem
exclude[ReversedMissingMethodProblem]("cats.data.EitherTMonadErrorF.redeem"),
exclude[ReversedMissingMethodProblem]("cats.data.EitherTMonadErrorF.redeemWith"),
exclude[ReversedMissingMethodProblem]("cats.data.EitherTMonadErrorF.handleError"),
exclude[ReversedMissingMethodProblem]("cats.data.EitherTMonadErrorF.attempt"),
exclude[ReversedMissingMethodProblem]("cats.data.EitherTMonadError.redeem"),
exclude[ReversedMissingMethodProblem]("cats.data.EitherTMonadError.redeemWith"),
// BREAKAGE — Scala 2.11 only!
exclude[ReversedMissingMethodProblem]("cats.MonadError.redeem"),
exclude[ReversedMissingMethodProblem]("cats.MonadError.redeemWith")

Don't know what the current binary compatibility policy is, but due to adding methods with default implementations on traits, these are not backward compatible on Scala 2.11, because it targets Java 7, which doesn't support interfaces with "default implementations".

Scala 2.12 doesn't have such problems though.

But I'm guessing that due to this change, such a PR can only target Cats 2.0, is that correct?

codecov-io · 2018-04-23T09:37:41Z

Codecov Report

Merging #2237 into master will decrease coverage by 0.07%.
The diff coverage is 90.69%.

@@            Coverage Diff             @@
##           master    #2237      +/-   ##
==========================================
- Coverage   95.05%   94.98%   -0.08%     
==========================================
  Files         333      336       +3     
  Lines        5789     5846      +57     
  Branches      211      218       +7     
==========================================
+ Hits         5503     5553      +50     
- Misses        286      293       +7

Impacted Files	Coverage Δ
core/src/main/scala/cats/syntax/either.scala	`99.15% <ø> (-0.02%)`	⬇️
.../main/scala-2.12+/cats/internals/FutureShims.scala	`0% <0%> (ø)`
.../main/scala-2.11-/cats/internals/FutureShims.scala	`0% <0%> (ø)`
...ore/src/main/scala/cats/internals/EitherUtil.scala	`100% <100%> (ø)`
core/src/main/scala/cats/instances/option.scala	`100% <100%> (ø)`	⬆️
core/src/main/scala/cats/MonadError.scala	`100% <100%> (ø)`	⬆️
core/src/main/scala/cats/data/Ior.scala	`98.48% <100%> (ø)`	⬆️
core/src/main/scala/cats/ApplicativeError.scala	`100% <100%> (ø)`	⬆️
core/src/main/scala/cats/instances/either.scala	`100% <100%> (ø)`	⬆️
...n/scala/cats/laws/discipline/MonadErrorTests.scala	`100% <100%> (ø)`	⬆️
... and 8 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...ecdc544. Read the comment docs.

LukaJCB · 2018-04-23T10:00:16Z

We could add redeem and redeemWith add syntax on the SyntaxBinCompat trait :)

johnynek · 2018-04-23T18:33:05Z

core/src/main/scala/cats/data/EitherT.scala

+   *
+   * @see [[upcastR]]
+   */
+  @inline def upcastL[AA >: A]: EitherT[F, AA, B] =


can we call this leftWiden to mirror widen which we normally use here?
https://github.com/typelevel/cats/blob/master/core/src/main/scala/cats/Bifunctor.scala#L54

johnynek · 2018-04-23T18:33:53Z

core/src/main/scala/cats/data/EitherT.scala

+   *
+   * @see [[upcastL]]
+   */
+  @inline def upcastR[BB >: B]: EitherT[F, A, BB] =


rightWiden?

alexandru · 2018-04-24T04:15:33Z

@LukaJCB

We could add redeem and redeemWith add syntax on the SyntaxBinCompat trait :)

Not sure what you mean, I see there is a AllSyntaxBinCompat0, don't know what the purpose is.

@johnynek

I renamed upcastL to leftWiden and upcastR to widen. That naming is unfortunate tbh, because the technical term is upcasting. It explains for example why I had no idea they existed. But being consistent is more important.

alexandru · 2018-04-24T05:56:52Z

@johnynek actually I removed widen / leftWiden, as I don't want to defend adding those on the class itself 🙂 Their existence on Functor / Bifunctor is good enough.

LukaJCB · 2018-04-24T08:28:19Z

Sorry about the lack of depth on my last comment, was on mobile and on a train. We can create some MonadErrorExtraOps trait put the two functions there and then create another AllSyntaxBinCompat1 trait just like the one here. Then we can mix those into the AllSyntaxBinCompat trait as well as the object implicits found here. That should give everyone the ability to use these methods whenever they import all syntax.

When cats 2.0 comes around, we can then move them into their respective type classes :)

alexandru · 2018-04-25T08:21:05Z

@LukaJCB that's cool, but the syntax isn't the problem solved here — redeemWith isn't much of an improvement over .attempt.flatMap.

The real win is the ability to override redeemWith.

Related: typelevel/cats-effect#191

alexandru · 2018-04-25T11:40:26Z

Actually I want to think about this more, given Cats 2.0's timeline I think that's OK, we've got time. Closing it to prevent merging or PR creep, will reopen if the case.

alexandru · 2018-04-27T06:25:13Z

OK, sorry for reopening it — I've carefully considered possibilities, doing benchmarks with alternatives, like a variant that takes a function with an Either[Throwable, A] parameter, plus I modified some Monix code to see how it feels like and I think this approach is the best.

The signature is nice too, it's basically a fold (catamorphism?) that happens in the F[_] context.

joan38 · 2018-10-19T07:28:02Z

Is this PR scheduled to go in for 2.0? If yes can we add the label?

alexandru · 2018-10-19T08:30:33Z

Is this PR scheduled to go in for 2.0?

I hope so 🙂

joan38 · 2018-10-19T09:18:26Z

Great!
@kailuowang could you add the 2.0 label please ? 😄

joan38 · 2019-07-04T16:15:24Z

Hey @kailuowang @alexandru, Did we forget to merge this for the 2.0 release? Is it still time?

kailuowang · 2019-07-04T17:15:46Z

We didn't forget. Our release plan changed. The original 2.0 release is going to be 2.1, the 2.0 release we are preparing now is different and doesn't allow PRs that break binary compat on Scala 2.11 like this one.

travisbrown · 2019-11-13T17:24:46Z

Removing this from the 2.1.0-RC1 milestone in favor of the subset in #3146. I do think we should aim to get the EitherT enhancements merged at some point, so I don't think we should close it yet.

johnynek · 2022-06-27T04:16:42Z

core/src/main/scala/cats/data/EitherT.scala

+   *
+   * @see [[handleErrorF]] for recovering errors thrown in `F[_]`
+   */
+  def handleError(f: A => B)(implicit F: Functor[F]): EitherT[F, A, B] =


I kind of think this should take a new type parameter for A:

def handleError[A1](f: A => B)... : EitherT[F, A1, B]

Since we know that there is no A left in the result.

johnynek · 2022-06-27T04:17:22Z

core/src/main/scala/cats/data/EitherT.scala

+   *
+   * @see [[handleError]] for recovering errors expressed via `EitherT`
+   */
+  def handleErrorF[E](f: E => B)(implicit F: ApplicativeError[F, E]): EitherT[F, A, B] =


Same comment about A

I don't think we can change this one. How do you make an EitherT(F.pure(Left(a)) into a EitherT(F.pure(Left(a1))) ?

I guess we can do A1 >: A or something.

johnynek · 2022-06-27T04:18:25Z

core/src/main/scala/cats/data/EitherT.scala

+   *
+   * @see [[handleErrorWithF]] for recovering errors thrown in `F[_]`
+   */
+  def handleErrorWith(f: A => EitherT[F, A, B])(implicit F: Monad[F]): EitherT[F, A, B] =


Seems this could also benefit from a type parameter A1 in f result type.

johnynek · 2022-06-27T04:18:41Z

core/src/main/scala/cats/data/EitherT.scala

+   *
+   * @see [[handleErrorWith]] for recovering errors expressed via `EitherT`
+   */
+  def handleErrorWithF[E](f: E => EitherT[F, A, B])(implicit F: ApplicativeError[F, E]): EitherT[F, A, B] =


Some A comment

armanbilge

Still need to add tests.

armanbilge · 2022-06-27T13:19:49Z

tests/shared/src/test/scala/cats/tests/RegressionSuite.scala

-    EitherT.right[String](Option(1)).handleErrorWith((_: String) => EitherT.pure(2))
+    EitherT.right[String](Option(1)).handleErrorWith((_: String) => EitherT.pure[Option, String](2))


@johnynek note that adding an A1 to handleErrorWith seems to break inference. So this would be source-breaking wrt the old implicit syntax way.

armanbilge · 2022-06-27T13:21:23Z

tests/shared/src/test/scala/cats/tests/RegressionSuite.scala

-    {
-      implicit val me: MonadError[EitherT[Option, String, *], Unit] =
-        EitherT.catsDataMonadErrorFForEitherT[Option, Unit, String]
-      EitherT.right[String](Option(1)).handleErrorWith((_: Unit) => EitherT.pure(2))
-    }


Now that EitherT implements handleErrorWith this would no longer work. However, for these sorts of use cases there is the better alternative handleErrorWithF.

Add Mima filters

a2e8854

johnynek reviewed Apr 23, 2018

View reviewed changes

alexandru added 2 commits April 24, 2018 07:03

Rename upcastL to leftWiden, upcastR to rightWiden

663bca3

Rename rightWiden to widen

1197f39

alexandru added 2 commits April 24, 2018 08:18

Modify attemptF

f917f4e

Remove widen/leftWiden

ecdc544

alexandru mentioned this pull request Apr 25, 2018

Add IO#redeem and IO#redeemWith typelevel/cats-effect#191

Merged

alexandru closed this Apr 25, 2018

alexandru reopened this Apr 27, 2018

alexandru mentioned this pull request May 1, 2018

Cats Effect 1.0.0-RC monix/monix#659

Merged

diesalbla mentioned this pull request Oct 18, 2018

Add ApplicativeError.redeem and MonadError.redeemWith #2560

Closed

kailuowang added this to the 2.0 milestone Oct 19, 2018

rossabaker mentioned this pull request Apr 14, 2019

Client: avoid throwing outside of effect system http4s/http4s#2502

Merged

joan38 mentioned this pull request Jul 6, 2019

Proposal: MonadError.transformWith #2161

Closed

travisbrown added the enhancement label Nov 6, 2019

travisbrown mentioned this pull request Nov 13, 2019

Add redeem and redeemWith #3146

Merged

travisbrown removed this from the 2.1.0-RC1 milestone Nov 13, 2019

Base automatically changed from master to main March 20, 2021 10:41

Merge remote-tracking branch 'upstream/main' into issue-2161

f5cc3da

armanbilge changed the title ~~Fix #2161, adding MonadError.redeemWith, enhance EitherT~~ Add error-handling methods to EitherT Jun 26, 2022

armanbilge added this to the 2.9.0 milestone Jun 26, 2022

johnynek requested changes Jun 27, 2022

View reviewed changes

armanbilge added 2 commits June 27, 2022 12:36

Fix compile

3104046

Add A1 type param

c5cf43e

armanbilge requested changes Jun 27, 2022

View reviewed changes

armanbilge mentioned this pull request Oct 9, 2022

Behaviour of MonadCancelThrow[EitherT[IO, Throwable, *]] #4308

Open

armanbilge modified the milestones: 2.9.0, 2.10.0 Oct 30, 2022

armanbilge modified the milestones: 2.10.0, 2.11.0 Aug 14, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add error-handling methods to `EitherT` #2237

Add error-handling methods to `EitherT` #2237

alexandru commented Apr 23, 2018 •

edited

Loading

codecov-io commented Apr 23, 2018 •

edited

Loading

LukaJCB commented Apr 23, 2018

johnynek Apr 23, 2018

johnynek Apr 23, 2018

alexandru commented Apr 24, 2018

alexandru commented Apr 24, 2018 •

edited

Loading

LukaJCB commented Apr 24, 2018

alexandru commented Apr 25, 2018

alexandru commented Apr 25, 2018

alexandru commented Apr 27, 2018

joan38 commented Oct 19, 2018

alexandru commented Oct 19, 2018

joan38 commented Oct 19, 2018 •

edited

Loading

joan38 commented Jul 4, 2019 •

edited

Loading

kailuowang commented Jul 4, 2019

travisbrown commented Nov 13, 2019

johnynek Jun 27, 2022

johnynek Jun 27, 2022

armanbilge Jun 27, 2022

armanbilge Jun 27, 2022

johnynek Jun 27, 2022

johnynek Jun 27, 2022

armanbilge left a comment

armanbilge Jun 27, 2022

armanbilge Jun 27, 2022

		EitherT.right[String](Option(1)).handleErrorWith((_: String) => EitherT.pure(2))
		EitherT.right[String](Option(1)).handleErrorWith((_: String) => EitherT.pure[Option, String](2))

Add error-handling methods to EitherT #2237

Are you sure you want to change the base?

Add error-handling methods to EitherT #2237

Conversation

alexandru commented Apr 23, 2018 • edited Loading

codecov-io commented Apr 23, 2018 • edited Loading

Codecov Report

LukaJCB commented Apr 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexandru commented Apr 24, 2018

alexandru commented Apr 24, 2018 • edited Loading

LukaJCB commented Apr 24, 2018

alexandru commented Apr 25, 2018

alexandru commented Apr 25, 2018

alexandru commented Apr 27, 2018

joan38 commented Oct 19, 2018

alexandru commented Oct 19, 2018

joan38 commented Oct 19, 2018 • edited Loading

joan38 commented Jul 4, 2019 • edited Loading

kailuowang commented Jul 4, 2019

travisbrown commented Nov 13, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

armanbilge left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Add error-handling methods to `EitherT` #2237

Add error-handling methods to `EitherT` #2237

alexandru commented Apr 23, 2018 •

edited

Loading

codecov-io commented Apr 23, 2018 •

edited

Loading

alexandru commented Apr 24, 2018 •

edited

Loading

joan38 commented Oct 19, 2018 •

edited

Loading

joan38 commented Jul 4, 2019 •

edited

Loading