NonFatal and ControlThrowable (changed in stdlib)

Scala Platform
1

Currently, ControlThrowable is excluded from handling in NonFatal. All asynchronous wrappers (i.e., Future, IO, ZIO) all use NonFatal in the catch clause.

I think it’s incorrect – execution engines should report ControlThrowable as usual unhandled exceptions because it’s a non-critical error in the other layer than an execution engine.

Also, this is a problem for direct-style transformers because it’s impossible to pass ControlThrowable via monadic boundaries. Therefore for support of returning, we need or do a compile-time translation or wrap each operation in an extra try/catch.

Proposed changes:

  1. Introduce something like NonFatalOnly. (what will be the better name?)
  2. Change Future in the standard library and recommend monad authors use NonFatalOnly
  3. Introduce ‘UserThrowable’ (or NonSystem) as a synonym for current NonFatal and deprecate NonFatal.
2

Note: scala.util.Try also uses NonFatal, so also has this behavior

3

With Try it’s ok – In the user code, the behavior should be the same as now.

4

Mostly yes, but it’ll be a bit weird if Future can hold a ControlThrowable and Try can’t, considering how similar their behavior is otherwise.

5

I agree. ControlThrowable should be included it NonFatal. If one does general handling via ControlThrowable, the intention is to handle all aborts, including non-local returns, breaks, and so on.

Instead of having another kind NonFatal we could also have another kind of ControlThrowable. Maybe:

   class ControlException extends RuntimeException

Then that one could be used by non-local returns and breaks. I believe that’s the easier path, given how widespread NonFatal is.

Aside: I believe NonFatal was a dubious development. The Java APIs state quite clearly:

An Error is a subclass of Throwable that indicates serious problems that a reasonable application should not try to catch.

So, Scala applications should (at most) catch exception, not Throwable, which would make the whole NonFatal construction unnecessary.

But I guess that train has departed and we have to live with NonFatal. Only in the future, please don’t invent new Throwables that are not Exceptions.

1 Like
6

Difference between Try and Future reflect reality: it should handle a different kind of exceptions.
Try(expr) is forcing the evaluation of the expression in the same thread, so it should be transparent against ControlThrowable. (i.e. allows using Try in returning and breaks).
Future(expr) if forcing the evaluation of the expression in the other (logical or physical) thread and therefore. returning { ... Future( .... throwReturn(..). ) } have no sense. But this is an exception in the user code, not in the code of the execution engine. So, a Future should fail, not a thread pool.

7

Yes, the new ControlException will be easier and maybe ‘more right’. On the other side, code that was transparent against non-local returns and breaks will start catching ones. I.e. if the library has its own analog of Try(), then it should be modified. Knowing the amount of DSLs arround, this looks like a huge breaking change.

8

Only stdlib Future does that and only with default executor. Twitter Future or IO monads do not force it and just suspend execution. And I think it’s not too hard to write ExecutionContext that does the same.

So the difference between Try/Future lies not in error handling, but eagerness/lazyness.

1 Like
9

… and is catching all NonFatal exceptions. So the only code that breaks is code that does a non-local return from a Try. Or, who else uses ControlThrowable?.

I believe directly returning from a Try(...) is morally the wrong thing to do, but we can’t exclude code that does it. Fortunately, non-local returns are about to be deprecated. So that means:

  • non-local returns will continue to use ControlThrowable and therefore return from a Try directly.
  • new control abstractions will use ControlException, which will be caught in a Try.
  • Migration guides how to move off non-local returns will mention this point.
10

Directly returning from a Try(...) looks very much the same to me as returning from a try..catch. Is that morally wrong as well? I don’t think so. Or is it because there’s a lambda there? But then how is it different than returning from a map(...)? If all of those are morally wrong, then everything in https://github.com/lampepfl/dotty/pull/16550 is also morally wrong, because non-local return is literally a special-case of boundary+break.

It seems a lot of nuance is getting lost in the above discussion. I don’t think there is a one-size-fits-all solution.

By design, a control-throwable is meant to implement a control structure. To me, anything that catches a control-throwable to short-circuit that control structure is doing something wrong, unless it knows what to do with it to correctly restore the semantics of the control structure. So a Try interrupting my control-throwable would be bad. Now, if for some programmer error reason, a control-throwable escapes its dedicated structure, it is not wrong to catch and recover from it at a let-it-crash-style boundary, such as the ExecutionContext or actor level. But then it should be reported as a bug, for example in logs.

I think NonFatal was meant for user-level catch-all things. As such, it seems appropriate to me that it avoids catching control-throwables. For “system”-level things like ExecutionContext, it is probably OK to catch pretty much anything but VirtualMachineErrors.

6 Likes
11

With “morally wrong” I meant that the semantics of non-local returns out of a Try(...) block are very subtle, probably unclear to many, and therefore they are best avoided.

So it’s a good thing that non-local returns are deprecated now. The question is what properties should their replacement have? I believe one problem with non-local returns was that they had a fuzzy semantics. In principle they are exceptions, but they hide that to a large degree. However, the abstraction is leaky since one can catch a nonlocal return with a catch clause on Throwable. Not ideal.

I believe that the replacement of non-local returns (aka break) should be unambiguously exceptions. That means any try around a break will be tested for eligible catch clauses and any finally clause will be executed. This irrespective of whether the break is local or non-local. So, breaks are exceptions but we can optimize them sometimes by rewriting them to labeled returns when nobody can tell the difference.

12

I don’t think this is workable. The behavior of the library has been that ControlThrowable is not caught:

Instances of ControlThrowable should not normally be caught.
As a convenience, NonFatal does not match ControlThrowable.

That’s pretty clear. Just in case there was any doubt, NonFatal says so too:

Note that scala.util.control.ControlThrowable, an internal Throwable, is not matched by NonFatal

and although Try unfortunately doesn’t mention this itself, it does say what behavior it follows:

Note: only non-fatal exceptions are caught by the combinators on Try (see scala.util.control.NonFatal).

If people have written code that breaks because they wanted something else, I think it’s on them to fix it rather than breaking the working code written by people who took the care to do the right thing.

The Scala library is not the only case where custom control flow is used. The really pernicious thing about changing existing behavior is that it seemed like a strong guarantee, and seemed like there was no way it could break, so it’s unlikely to be caught even if there were moderately careful unit tests, and the compiler is unlikely to help at all.

If we want different control constructs that work differently, I think we should just create the ones we want. For instance, we could have a NonFatal-like thing called, say, ThreadBound that will catch anything that you want to try not to let leak out of the thread (including stray control flow).

Let’s not pretend that catching control flow in a Try is ever likely to be the desired outcome. Your control flow is not going to work the way you want if it leaks out into some Try or other catch statement. It’s already broken. The question is only whether you handle the broken behavior within the thread or if it kills the thread.

So if you want something that works like Try but catches ControlThrowable, create a new one that does the right thing. Then existing correct code stays correct, and existing wrong code created without regard to control throwables can be converted into existing less perniciously wrong code by a search-and-replace.

Alternatively, leave NonFatal and ControlThrowable alone, but switch Try to not use NonFatal and somehow try to very clearly document that everyone who followed the link and understood what Try was and was not going to catch should switch to using something else (e.g. Try.withControl).

3 Likes
13

This might have been the original sin. It was transplanted from Akka/Future. The email threads from a decade ago, which were fun to re-read instead of mulling over the year just past, detail what to include or exclude, and it’s obvious that framework code and various user code have different needs. The wrangling over OutOfMemoryError and StackOverflowError also came down to “use try catch”. The error from ??? (NotImplementedError) was also non-NonFatal, like ControlThrowable.

util.Using is also sensitive to ControlThrowable.

Jason and Rex made various useful distinctions at the time, including that any change will break a lot of code, to which Viktor added the warning not to conflate “fatal” and “catchable”.

Another theme or meme from the threads was, Can we make it customizable. But that is what catch is for, or UncaughtExceptionHandler, or util.control.Exception.catching.

It seems that ??? was the first thing I expressed an opinion about. What a slippery slope that turned out to be.

Should the future API decide what exceptions are or aren’t fatal?
NotImplementedError considered fatal?
OutOfMemoryError considered fatal?
Adding ??? to Predef???

1 Like
14

My proposal was to leave NonFatal and ControlThrowable alone, deprecate ControlThrowable (since the primary user, non-local returns, is deprecated as well), introduce a new runtime exception ControlException, and use that one for all new control flow abstractions. It so happens that with the phasing out of non-local returns we need some new abstractions, and the one currently proposed (i.e. ReturnThrowable, returning and throwReturn) are young enough to be replaced by something else in an unobtrusive way. Did I overlook something?

15

I wonder: Since Akka now has a Java compatible API, what do they use instead of NonFatal? NonFatal does not work in Java since it is an extractor.

16

That’s a very reasonable advice!

Should we put it somewhere in the official documentation?
I tried looking, but haven’t found a good place to put it. It seems that we only document how to try/catch things, never how (and what) to throw and how to declare Exceptions (and Throwables :scream_cat: ):

(note that these are two different links)

17

The problem with RuntimeException in control flow, that we can have inside returning something like own control-flow DSL based on by-name parameters:

def   withErrorHandling(cmd: => () ): Unit = {
    try {
        cmd
   } catch {
        case NonFatal(ex) =>
           log(s"Error during $op",  ex)
   }
}

and later

 returning {
     while(true) {
         withErrorHandling{
             receiveCommand() match
                 case  'quit' =>
                      throwReturn true
                 case ...
         }
     }
 }

Assuming that previously it was converted from return:.

def process(): Boolean = {
    while(true) {
         withErrorHandling{
             receiveCommand() match
                 case  'quit' =>
                      return true
                 case ...
         }
     }
}

This will compile, and process() will start to handle throwReturn.

Of course, we can hope that returning is relatively new or think about the migration rule to check.
(Correct refactoring will be changing NonFatal to something like NonSystem in the user code
and marking all handling of generic exceptions. (case ex: RuntimeException => ...) )

1 Like
18

You might be underestimating how much ControlThrowable is actually used in the wild, beyond return: GitHub search for “extends ControlThrowable”

19

Yes, but logging all errors and swallowing them as in withErrorHandling is terrible style. One should never do that.

I do think it’s better to own up and say “non-local returns are exceptions” instead of giving them this weird half-way status, where they are exceptions but hide that fact in many ways. It’s a leaky abstraction, so best to be avoided. Also, if non-local returns are exceptions it’s one concept less to learn since we already know exceptions and how to deal with them.

20

You need to put “extends ControlThrowable” in quotes. Then the number goes down by a factor of almost 10.