-
Notifications
You must be signed in to change notification settings - Fork 62
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
Implemented submarine error propagation for Handle
#489
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm somewhat meh about the names ensure
and recover
, sorry ... not that I have any better ideas 😛
So on ApplicativeError
we have recover
which takes a PartialFunction[E, A]
and doesn't match how it is used here. handleErrorWith
matches more closely the E => F[A]
.
Meanwhile on MonadError
we have ensure
with takes a predicate: A => Boolean
.
assert(test.value.value.toOption == Some("third1")) | ||
} | ||
|
||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be a test("...") {
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can it be? I have no idea how that works in MUnit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm confused :) is this a test or not haha, that is really what I am asking 😂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a lexical block which contains Discipline stuff, which is many tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see now, sorry. I was confused by the end of the block, it looked like an ordinary test ... but it's because the discipline tests have to exist inside of the Eval
since that's how the Handle
is constructed. Thanks!
cats-mtl/tests/shared/src/test/scala/cats/mtl/tests/HandleTests.scala
Lines 122 to 124 in 0b8e897
test.value.value | |
() | |
} |
def apply[F[_], E](implicit ev: Handle[F, E]): Handle[F, E] = ev | ||
|
||
def ensure[F[_], E]: AdHocSyntax[F, E] = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we ask for the ApplicativeThrow
constraint here, and then pass it into AdHocSyntax
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we do, we lose the syntax and have to insert extra parens at the call site.
} | ||
|
||
private final case class Submarine[E](e: E, marker: AnyRef) | ||
extends RuntimeException |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this just extend Throwable
? (Or even ControlThrowable
, but I think that one is considered fatal by NonFatal
.) It's not really an ordinary exception, and even if it leaked, without a stacktrace it would be hard to do something about it 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I went back and forth. Technically RuntimeException is accurate and appropriate, but it's all sort of dependent on perspective. Certainly anything that would be considered fatal is off the table since it just instantly torpedoes the whole runtime.
Co-authored-by: Arman Bilge <[email protected]>
Co-authored-by: Arman Bilge <[email protected]>
Co-authored-by: Arman Bilge <[email protected]>
Also meh about the names. That's actually the hardest part of this PR honestly. I was going for something close to try/catch, since the structure is precisely analogous. "Rescue" is another word I had in mind (Ruby's equivalent of try/catch is ensure/rescue), but it's not great either way. Attempt would be ideal but that's very much off the table. |
Cool! Having previously built my own Submarine out of cardboard and duct tape to sail the "Sewers of IO", I welcome this factory-built model. Can I paint it yellow? |
Actually I think it's begin/rescue/ensure as in try/catch/finally. That seems like a more appropriate use of "ensure" 😜 So we could have |
Oh this is an interesting idea. It would allow us to potentially have uniform syntax across the classes. |
more thoughts:
Handle.begin[IO, DomainError] { // look ma, no implicit!
foo *> bar
} rescue {
case DomainError.Failed => ???
case DomainError.Derped => ???
}
Handle.begin[IO, DomainError]:
foo *> bar
rescue:
case DomainError.Failed => ???
case DomainError.Derped => ???
Handle.begin[DomainError]:
foo *> bar
rescue:
case DomainError.Failed => ???
case DomainError.Derped => ??? |
|
||
final class AdHocSyntax[F[_], E] { | ||
|
||
def apply[A](body: Handle[F, E] => F[A])(implicit F: ApplicativeThrow[F]): Inner[A] = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we implement this with Handle[F, Throwable]
instead of ApplicativeThrow[F]
?
Cool, I prototyped the syntax for Scala 3 and was able to make this work: begin[MyError]:
foo *> bar
.rescue:
case MyError.Derp => ???
case MyError.Fart => ??? //> using scala "3.3.0-RC5"
//> using lib "org.typelevel::cats-mtl::1.3.1"
//> using lib "org.typelevel::cats-effect::3.4.10"
import cats.*
import cats.effect.*
import cats.mtl.*
import scala.annotation.*
def begin[E]: Begin[E] = ???
class Begin[E]:
def apply[F[_], A](f: Handle[F, E] ?=> F[A])(using ApplicativeThrow[F]): Body[F, E, A] = ???
class Body[F[_], E, A]:
def rescue(f: E => F[A]): F[A] = ???
enum MyError:
case Derp, Fart
def foo(using Raise[IO, MyError]): IO[Unit] = ???
def bar(using Handle[IO, MyError]): IO[Unit] = ???
object Main extends IOApp.Simple:
def run =
begin[MyError]:
foo *> bar
.rescue:
case MyError.Derp => ???
case MyError.Fart => ??? The catch is that it breaks if I add an overload to support the Scala 2 cross-compilable syntax where the |
Ha, for fun you can even slap on a begin[MyError]:
foo *> bar
.rescue:
case MyError.Derp => ???
case MyError.Fart => ???
.guarantee:
IO.unit |
Some more name ideas: allow[MyError]:
foo *> bar
.rescue:
case MyError.Derp => ???
case MyError.Fart => ??? anticipate[MyError]:
foo *> bar
.rescue:
case MyError.Derp => ???
case MyError.Fart => ??? expect[MyError]:
foo *> bar
.rescue:
case MyError.Derp => ???
case MyError.Fart => ??? intercept[MyError]:
foo *> bar
.rescue:
case MyError.Derp => ???
case MyError.Fart => ??? Edit: I think I really like |
❤️ that we're considering how it works braceless. I'm all too happily becoming a braceless mutant that's losing the ability to synthesize braces.. I'm fine with Overall, would really like to use this capability, hit the Merge already :P |
This adds builder syntax to
Handle
which makes it possible to materialize an instance ofHandle
for any error type given anApplicativeThrow[F]
. Syntax is intentionally reminiscent oftry
/catch
:You can see some other examples in the tests. You can suspend as many different error types as you want and it all composes the way you would expect.
This effectively eliminates the need for
EitherT
orIorT
outside of tests (where both can still be useful for building ad-hoc harnesses that are less powerful thanIO
). Types infer quite nicely on both Scala 2 and Scala 3, though obviously Scala 3'susing
syntax makes this even nicer.