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

Implemented submarine error propagation for Handle #489

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

djspiewak
Copy link
Member

This adds builder syntax to Handle which makes it possible to materialize an instance of Handle for any error type given an ApplicativeThrow[F]. Syntax is intentionally reminiscent of try/catch:

def foo[F[_]](implicit h: Raise[F, DomainError]) = ???
def bar[F[_]](implicit h: Handle[F, DomainError]) = ???

Handle.ensure[IO, DomainError] { implicit h =>
  foo *> bar
} recover {
  case DomainError.Failed => ...l
  case DomainError.Derped => ...
}

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 or IorT outside of tests (where both can still be useful for building ad-hoc harnesses that are less powerful than IO). Types infer quite nicely on both Scala 2 and Scala 3, though obviously Scala 3's using syntax makes this even nicer.

Copy link
Member

@armanbilge armanbilge left a 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"))
}

{
Copy link
Member

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("...") { ?

Copy link
Member Author

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.

Copy link
Member

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 😂

Copy link
Member Author

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.

Copy link
Member

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!

def apply[F[_], E](implicit ev: Handle[F, E]): Handle[F, E] = ev

def ensure[F[_], E]: AdHocSyntax[F, E] =
Copy link
Member

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?

Copy link
Member Author

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
Copy link
Member

@armanbilge armanbilge Apr 30, 2023

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 😅

Copy link
Member Author

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.

@djspiewak
Copy link
Member Author

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.

@benhutchison
Copy link
Member

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?

@armanbilge
Copy link
Member

Ruby's equivalent of try/catch is ensure/rescue

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 Handle.begin { ... } rescue { ... } or maybe Handle.in { ... } rescue { ... }? I also entertained ideas of embed or embedError since we are embedding E in the outer effect 🤔

@djspiewak
Copy link
Member Author

So we could have Handle.begin

Oh this is an interesting idea. It would allow us to potentially have uniform syntax across the classes.

@armanbilge
Copy link
Member

more thoughts:

  1. we should consider having a cross-compilable syntax for 2/3 as well as a Scala 3 only syntax which uses Context functions. Annoyingly the method name will need to be different.
Handle.begin[IO, DomainError] { // look ma, no implicit!
  foo *> bar
} rescue {
  case DomainError.Failed => ???
  case DomainError.Derped => ???
}
  1. braceless syntax is increasing in popularity. will this work? if not then how does it look without braces?
Handle.begin[IO, DomainError]:
  foo *> bar
rescue:
  case DomainError.Failed => ???
  case DomainError.Derped => ???
  1. can we make the type parameter for F[_] go away? it should be inferable from the body.
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] =
Copy link
Member

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]?

@armanbilge
Copy link
Member

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 Handle is passed explicitly. So I do think we will need a different method name.

@armanbilge
Copy link
Member

armanbilge commented May 11, 2023

Ha, for fun you can even slap on a guarantee at the end and then it's truly like a try/catch/finally.

begin[MyError]:
  foo *> bar
.rescue:
  case MyError.Derp => ???
  case MyError.Fart => ???
.guarantee:
  IO.unit

@armanbilge
Copy link
Member

armanbilge commented May 11, 2023

Some more name ideas: allow[Error], anticipate[Error], expect[Error], intercept[Error]

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 allow especially when paired with typelevel/cats-effect#3575. Because that makes it impossible to raise errors in F[_]: Concurrent etc. unless someone allows you to 😜

@benhutchison
Copy link
Member

  1. braceless syntax is increasing in popularity. will this work? if not then how does it look without braces?
Handle.begin[IO, DomainError]:
  foo *> bar
rescue:
  case DomainError.Failed => ???
  case DomainError.Derped => ???

❤️ 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 allow instead of begin if that's what it takes to avoid namespace collisions.

Overall, would really like to use this capability, hit the Merge already :P

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants