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 catchOnly to ApplicativeError. #3165

Merged
merged 2 commits into from
Nov 26, 2019
Merged
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
22 changes: 19 additions & 3 deletions core/src/main/scala/cats/ApplicativeError.scala
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
package cats

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

import scala.reflect.ClassTag
import scala.util.{Failure, Success, Try}
import scala.util.control.NonFatal
import scala.util.{Failure, Success, Try}

/**
* An applicative that also allows you to raise and or handle an error value.
Expand Down Expand Up @@ -157,8 +158,6 @@ trait ApplicativeError[F[_], E] extends Applicative[F] {
* @param fa is the source whose result is going to get transformed
* @param recover is the function that gets called to recover the source
* in case of error
* @param map is the function that gets to transform the source
* in case of success
*/
def redeem[A, B](fa: F[A])(recover: E => B, f: A => B): F[B] =
handleError(map(fa)(f))(recover)
Expand Down Expand Up @@ -216,6 +215,12 @@ trait ApplicativeError[F[_], E] extends Applicative[F] {
case NonFatal(e) => raiseError(e)
}

/**
* Evaluates the specified block, catching exceptions of the specified type. Uncaught exceptions are propagated.
*/
def catchOnly[T >: Null <: Throwable]: CatchOnlyPartiallyApplied[T, F, E] =
new CatchOnlyPartiallyApplied[T, F, E](this)
Comment on lines +221 to +222
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity... I'm struggling to understand this.

What was the T >: Null constraint added for in there? To me it seems that due to the latter constraint T <: Throwable the former one will be always satisfied automatically, won't it? This is because Throwable is AnyRef and Null <: T <: AnyRef for any T.

Or am I missing something?
@takayahilton could you clarify please?


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

final private[cats] class CatchOnlyPartiallyApplied[T, 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): F[A] =
try {
F.pure(f)
} catch {
case t if CT.runtimeClass.isInstance(t) =>
F.raiseError(t)
}
}

/**
* lift from scala.Option[A] to a F[A]
*
Expand Down
4 changes: 4 additions & 0 deletions core/src/main/scala/cats/instances/try.scala
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,10 @@ trait TryInstances extends TryInstances1 {
}

override def isEmpty[A](fa: Try[A]): Boolean = fa.isFailure

override def catchNonFatal[A](a: => A)(implicit ev: Throwable <:< Throwable): Try[A] = Try(a)

override def catchNonFatalEval[A](a: Eval[A])(implicit ev: Throwable <:< Throwable): Try[A] = Try(a.value)
}
// scalastyle:on method.length

Expand Down
17 changes: 17 additions & 0 deletions tests/src/test/scala/cats/tests/TrySuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,23 @@ class TrySuite extends CatsSuite {
res should not be (null)
}
}

test("catchOnly works") {
forAll { e: Either[String, Int] =>
val str = e.fold(identity, _.toString)
val res = MonadError[Try, Throwable].catchOnly[NumberFormatException](str.toInt)
// the above should just never cause an uncaught exception
// this is a somewhat bogus test:
res should not be (null)
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Might it be useful to also test that catchOnly on an unrelated exception does not catch?


test("catchOnly catches only a specified type") {
a[NumberFormatException] should be thrownBy {
MonadError[Try, Throwable].catchOnly[UnsupportedOperationException]("str".toInt)
}
}

test("fromTry works") {
forAll { t: Try[Int] =>
(MonadError[Try, Throwable].fromTry(t)) should ===(t)
Expand Down