Skip to content
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

Add bridge between ApplicativeError and ApplicativeThrow #4616

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 118 additions & 2 deletions core/src/main/scala/cats/ApplicativeError.scala
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,12 @@

package cats

import cats.ApplicativeError.CatchOnlyPartiallyApplied
import cats.ApplicativeError.{
CatchOnlyAsPartiallyApplied,
CatchOnlyAsUnsafePartiallyApplied,
CatchOnlyPartiallyApplied,
CatchOnlySafePartiallyApplied
}
import cats.data.{EitherT, Validated}
import cats.data.Validated.{Invalid, Valid}

Expand Down Expand Up @@ -264,6 +269,8 @@ trait ApplicativeError[F[_], E] extends Applicative[F] {
/**
* Often E is Throwable. Here we try to call pure or catch
* and raise.
*
* Note: fatal exceptions (as defined by `scala.util.control.NonFatal`) will be propagated
*/
def catchNonFatal[A](a: => A)(implicit ev: Throwable <:< E): F[A] =
try pure(a)
Expand All @@ -274,6 +281,8 @@ trait ApplicativeError[F[_], E] extends Applicative[F] {
/**
* Often E is Throwable. Here we try to call pure or catch
* and raise
*
* Note: fatal exceptions (as defined by `scala.util.control.NonFatal`) will be propagated
*/
def catchNonFatalEval[A](a: Eval[A])(implicit ev: Throwable <:< E): F[A] =
try pure(a.value)
Expand All @@ -282,11 +291,85 @@ trait ApplicativeError[F[_], E] extends Applicative[F] {
}

/**
* Evaluates the specified block, catching exceptions of the specified type. Uncaught exceptions are propagated.
* Evaluates the specified block, catching exceptions of the specified type.
*
* This method is considered unsafe because uncaught exceptions are propagated
* outside of `F`. While the name does not currently reflect this, this may change
* at a future date.
*/
def catchOnly[T >: Null <: Throwable]: CatchOnlyPartiallyApplied[T, F, E] =
new CatchOnlyPartiallyApplied[T, F, E](this)

/**
* Evaluates the specified block, catching exceptions of the specified type.
*
* Exceptions of type `T` are raised in `F`, other non-fatal exceptions are raised in `G`
*
* Note: fatal exceptions (as defined by `scala.util.control.NonFatal`) will be propagated
*/
def catchOnlySafe[G[_], T >: Null <: Throwable]: CatchOnlySafePartiallyApplied[T, G, F, E] =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I find such a naming quite confusing. I feel in Cats everything is supposed to be safe, unless stated and marked otherwise.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No argument, I just needed to name it something so I could put a safe variant of catchOnly in there so we can figure out if it's something worth adding or not, as renaming the existing method to catchOnlyUnsafe would break bincompat

new CatchOnlySafePartiallyApplied[T, G, F, E](this)

/**
* Often E can be created from Throwable. Here we try to call pure or
* catch, adapt into E, and raise.
*
* Exceptions that cannot be adapted to E and raised in `F` will be raised in `G`
*
* Note: fatal exceptions (as defined by `scala.util.control.NonFatal`) will be propagated
*/
def catchNonFatalAs[G[_], A](adaptIfPossible: Throwable => Option[E])(a: => A)(implicit
G: ApplicativeThrow[G]
): G[F[A]] =
try G.pure(pure(a))
catch {
case t if NonFatal(t) =>
adaptIfPossible(t).fold(G.raiseError[F[A]](t))(e => G.pure(raiseError[A](e)))
}

/**
* Often E can be created from Throwable. Here we try to call pure or
* catch, adapt into E, and raise.
*
* This method is considered unsafe because exceptions that cannot be
* adapted to E will be rethrown outside of `F`
*
* Note: fatal exceptions (as defined by `scala.util.control.NonFatal`) will be propagated
*/
def catchNonFatalAsUnsafe[A](adaptIfPossible: Throwable => Option[E])(a: => A): F[A] =
try pure(a)
catch {
case e if NonFatal(e) => adaptIfPossible(e).map(raiseError[A]).getOrElse(throw e)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line concerns me quite a lot, particularly throw e.
It means that the exception will be re-thrown out of the F context, which is not what I would expect from any ApplicativeError instance.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless we require the adaptation function to be total, we're going to need to rethrow these, because there isn't a way to raise a Throwable into an arbitrary ApplicativeError[F, E]

Copy link
Member

@armanbilge armanbilge Jun 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, exactly. This is why we must not use a PartialFunction as you suggested in #4616 (comment). See my comments in #4286 (comment).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@armanbilge , PartialFunction[A, B] and A => Option[B] are isomorphic. Do I understand correctly your point that we must not use either of them?

}

/**
* Evaluates the specified block, catching exceptions of the specified type.
*
* Caught exceptions are mapped to `E` and raised in `F`.
* Uncaught non-fatal exceptions will be raised in `G`.
* Fatal exceptions (as defined by `scala.util.control.NonFatal`) will be propagated
*
* Note: `catchOnlyAs` assumes that, if a specific exception type `T` is expected, there
* exists a mapping from `T` to `E`. If this is not the case, consider either manually
* rethrowing inside the mapping function, or using `catchNonFatalAs`
*/
def catchOnlyAs[G[_], T >: Null <: Throwable]: CatchOnlyAsPartiallyApplied[T, G, F, E] =
new CatchOnlyAsPartiallyApplied[T, G, F, E](this)

/**
* Evaluates the specified block, catching exceptions of the specified type.
*
* Caught exceptions are mapped to `E` and raised in `F`.
*
* This method is considered unsafe because uncaught exceptions will be propagated outside of `F`
*
* Note: `catchOnlyAsUnsafe` assumes that, if a specific exception type `T` is expected, there
* exists a mapping from `T` to `E`. If this is not the case, consider either manually
* rethrowing inside the mapping function, or using `catchNonFatalAsUnsafe`
*/
def catchOnlyAsUnsafe[T >: Null <: Throwable]: CatchOnlyAsUnsafePartiallyApplied[T, F, E] =
new CatchOnlyAsUnsafePartiallyApplied[T, F, E](this)

/**
* If the error type is Throwable, we can convert from a scala.util.Try
*/
Expand Down Expand Up @@ -383,6 +466,39 @@ object ApplicativeError {
}
}

final private[cats] class CatchOnlySafePartiallyApplied[T, G[_], F[_], E](private val F: ApplicativeError[F, E])
extends AnyVal {
def apply[A](
f: => A
)(implicit CT: ClassTag[T], NT: NotNull[T], ev: Throwable <:< E, G: ApplicativeThrow[G]): G[F[A]] =
try G.pure(F.pure(f))
catch {
case t if CT.runtimeClass.isInstance(t) => G.pure(F.raiseError(t))
case t if NonFatal(t) => G.raiseError(t)
}
}

final private[cats] class CatchOnlyAsPartiallyApplied[T >: Null <: Throwable, G[_], F[_], E](
private val F: ApplicativeError[F, E]
) extends AnyVal {
def apply[A](adapt: T => E)(f: => A)(implicit CT: ClassTag[T], NT: NotNull[T], G: ApplicativeThrow[G]): G[F[A]] =
try G.pure(F.pure(f))
catch {
case CT(t) => G.pure(F.raiseError(adapt(t)))
case t if NonFatal(t) => G.raiseError[F[A]](t)
}
}

final private[cats] class CatchOnlyAsUnsafePartiallyApplied[T >: Null <: Throwable, F[_], E](
private val F: ApplicativeError[F, E]
) extends AnyVal {
def apply[A](adapt: T => E)(f: => A)(implicit CT: ClassTag[T], NT: NotNull[T]): F[A] =
try F.pure(f)
catch {
case CT(t) => F.raiseError(adapt(t))
}
}

/**
* lift from scala.Option[A] to a F[A]
*
Expand Down
58 changes: 58 additions & 0 deletions tests/shared/src/test/scala/cats/tests/EitherSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ package cats.tests

import cats._
import cats.data.{EitherT, NonEmptyChain, NonEmptyList, NonEmptySet, NonEmptyVector, Validated}
import cats.syntax.option._
import cats.syntax.bifunctor._
import cats.kernel.laws.discipline.{EqTests, MonoidTests, OrderTests, PartialOrderTests, SemigroupTests}
import cats.laws.discipline._
Expand Down Expand Up @@ -136,6 +137,63 @@ class EitherSuite extends CatsSuite {
assert(Either.catchNonFatal(throw new Throwable("blargh")).isLeft)
}

test("ApplicativeError instance catchNonFatalAs maps exceptions to E") {
val res =
ApplicativeError[Either[String, *], String]
.catchNonFatalAs[Either[Throwable, *], Int](_.getMessage.some)("foo".toInt)
.leftMap(0 -> _.getMessage)
assert(res === Right(Left("For input string: \"foo\"")))
}

test("ApplicativeError instance catchNonFatalAs raises unmappable exceptions in G") {
val res =
ApplicativeError[Either[String, *], String]
.catchNonFatalAs[Either[Throwable, *], Int](_ => none[String])("foo".toInt)
.leftMap(0 -> _.getMessage)
assert(res === Left(0 -> "For input string: \"foo\""))
}

test("ApplicativeError instance catchNonFatalAsUnsafe maps exceptions to E") {
val res = ApplicativeError[Either[String, *], String].catchNonFatalAsUnsafe(_.getMessage.some)("foo".toInt)
assert(res === Left("For input string: \"foo\""))
}

test("ApplicativeError instance catchNonFatalAsUnsafe rethrows unmappable exceptions") {
val _ = intercept[NumberFormatException] {
ApplicativeError[Either[String, *], String].catchNonFatalAsUnsafe(_ => none[String])("foo".toInt)
}
}

test("ApplicativeError instance catchOnlyAs maps exceptions of the specified type to E") {
val res =
ApplicativeError[Either[String, *], String]
.catchOnlyAs[Either[Throwable, *], NumberFormatException](_.getMessage)("foo".toInt)
.leftMap(0 -> _.getMessage)
assert(res === Right(Left("For input string: \"foo\"")))
}

test("ApplicativeError instance catchOnlyAs raises non-matching exceptions in G") {
val res =
ApplicativeError[Either[String, *], String]
.catchOnlyAs[Either[Throwable, *], IndexOutOfBoundsException](_.getMessage)("foo".toInt)
.leftMap(0 -> _.getMessage)
assert(res === Left(0 -> "For input string: \"foo\""))
}

test("ApplicativeError instance catchOnlyAsUnsafe maps exceptions of the specified type to E") {
val res =
ApplicativeError[Either[String, *], String]
.catchOnlyAsUnsafe[NumberFormatException](_.getMessage)("foo".toInt)
assert(res === Left("For input string: \"foo\""))
}

test("ApplicativeError instance catchOnlyAsUnsafe propagates non-matching exceptions") {
val _ = intercept[NumberFormatException] {
ApplicativeError[Either[String, *], String]
.catchOnlyAsUnsafe[IndexOutOfBoundsException](_.getMessage)("foo".toInt)
}
}

test("fromTry is left for failed Try") {
forAll { (t: Try[Int]) =>
assert(t.isFailure === (Either.fromTry(t).isLeft))
Expand Down
14 changes: 14 additions & 0 deletions tests/shared/src/test/scala/cats/tests/TrySuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,20 @@ class TrySuite extends CatsSuite {
}
}

test("catchOnlySafe works") {
forAll { (e: Either[String, Int]) =>
val str = e.fold(identity, _.toString)
val res = MonadThrow[Try].catchOnlySafe[Try, NumberFormatException](str.toInt)
// the above should never raise an exception in the outer try
assertEquals(res.toEither.map(_.isSuccess), Right(e.isRight), clue(res))
}
}

test("catchOnlySafe only raise the specified type in Try") {
val res = MonadThrow[Try].catchOnlySafe[Try, UnsupportedOperationException]("str".toInt)
assert(res.isFailure, clue(res))
}

test("fromTry works") {
forAll { (t: Try[Int]) =>
assert((MonadThrow[Try].fromTry(t)) === t)
Expand Down
Loading