typelevel / cats-effect-cps

An incubator project for async/await syntax support for Cats Effect
Apache License 2.0
89 stars 12 forks source link

Behavior of try/finally #226

Open alexandru opened 2 days ago

alexandru commented 2 days ago

What are your thoughts on this sample:

//> using scala "3.5.1"
//> using dep "org.typelevel::cats-effect:3.5.4"
//> using dep "org.typelevel::cats-effect-cps:0.4.0"

import cats.effect.*
import cats.effect.cps.*

object Main extends IOApp.Simple:
    def run = async[IO]:
        try 
            IO.canceled.await
        finally 
            println("Finalizing!") 

The finalizer doesn't get invoked, and unfortunately, I find the sample counter-intuitive.

rssh commented 2 days ago

https://github.com/rssh/dotty-cps-async/issues/20

But not 100% sure. will try to reproduce.

armanbilge commented 2 days ago

Unless the finally expression gets translated to a IO.guaranteed by the CPS, I wouldn't expect it to run.

rssh commented 2 days ago

You say that IO interpreter skip IO.Cancelled unless inside guaranteed?

armanbilge commented 2 days ago

@rssh After IO.canceled, the IO interpreter will skip anything that's not in a special block e.g. guaranteed, onCancel, etc.

alexandru commented 1 day ago

@rssh I opened an issue here as well, as I wanted to get your feedback, I apologize for the noise: https://github.com/rssh/cps-async-connect/issues/9

So this code:

try 
    println("Starting!")
    IO.canceled.await
finally
    println("Finalizing!") 

Would need to be translated to the equivalent of this code:

IO {
  println("Starting!")
}.flatMap { _ =>
    IO.canceled
}.guarantee {
    IO(println("Finalizing!"))
}

This is by design in Cats-Effect because interruption has its channel, separate from exceptions. This is because, in Java, it's pretty bad that people can catch and ignore InterruptedException.

So, for example, it's fine if this code does not work with Cats-Effect:

try
  IO.cancelled.await
catch
  case e: Throwable => 
    println("Caught interruption?")

But despite this, I still feel like the finally should work because finally is the most important part of a try expression, being about resource safety. And in the context of a “direct style” syntax, finally not working for interruption is bad, IMO. I'm not 100% sure, though, happy to be proven wrong.