softwaremill / ox

Safe direct style concurrency and resiliency for Scala on the JVM
https://ox.softwaremill.com
Apache License 2.0
366 stars 26 forks source link

The `race` function doesn't handle functions that can throw a `ControlThrowable` exception #213

Closed rcardin closed 15 hours ago

rcardin commented 5 days ago

I'm the maintainer of the library raise4s(link to the repo here). I use context functions to let the user declare functions that can raise logic-typed errors instead of exceptions. Jon Pretty is doing something close to my library with Contingency.

I'd love to integrate with ox and write code like the following:

@main
def main(): Unit = {

  val result: Int raises String = ox.race(
    {
      ox.sleep(500.milliseconds)
      println("short task")
      Raise.raise("Something went wrong")
      2
    }, {
      ox.sleep(1.second)
      println("long task")
      Raise.raise("Something went wrong too late")
      1
    }
  )

  Raise.run(result) match {
    case value: Int => println(s"Value: $value")
    case error: String => println(s"Error: $error")
  }
}

However, the above code doesn't work. In detail, the execution of the race function never stops.

The implementation of the raise method uses the following exception:

private[raise4s] case class Raised[Error](original: Error)
    extends ControlThrowable
    with NoStackTrace

However, the implementation of the ox.race function handles the execution of the given lambdas through a Try:

fs.foreach(f => forkUnsupervised(result.put(Try(f()))))

Unfortunately, the Try type doesn't handle exceptions extending ControlThrowable. You can find here more details.

I already tried changing the ox code locally to add the handling of ControlThrowable, and everything started to integrate smoothly.

Can I open a PR to address the problem? Would you like to chat about it first?

adamw commented 5 days ago

Hey - sure, a PR would be great. Definitely we don't want any situations where Ox's code just "hangs". Worst case it should throw at least some exception. So maybe that's a wider problem, but let's start with race.