acton icon indicating copy to clipboard operation
acton copied to clipboard

How to indicate intention to "return" an exception?

Open plajjan opened this issue 2 years ago • 4 comments

If actor A calls a method on actor B and the method raises an exception, this exception is sent to actor A so it can handle it but it also bubbles up in actor B as an unhandled exception. We currently print all unhandled exceptions.

In general we want to print unhandled exceptions since we want to encourage developers to handle all exceptions! However, in this case, there is no way to handle the exception in actor B when we in fact want the exception to be "returned" to actor A. Our syntax doesn't currently allow us to differentiate between a truly unhandled exception and one where the intention is to return it to the calling actor.

How to solve?

@nordlander had an idea about having some form of wrapping exception. I think it's best if you can outline that proposal Johan.

A language addition is another alternative, something along the lines of return raise ValueError("foo"), but I know @nordlander voted against this ;)

plajjan avatar Nov 16 '23 21:11 plajjan

Another interesting case is if a function does return raise ValueError("foo") (I'm using this as the placeholder example now for when we want to "return" an exception and not have it treated like an unhandled exception in the local actor). Now imagine the calling actor called this function async style and doesn't care about the return value. Can we detect that there is no AWAIT on us and then print an unhandled exception message?

plajjan avatar Nov 17 '23 14:11 plajjan

I was thinking of writing something like raise ForClientOnly(exc) in a method that handles exception exc but wants to re-raise it just for any waiting client (the wrapper ForClientOnly would have to be a built-in thing, and the name is just a suggestion). Raising an original exception exclusively for clients would then be written

    raise ForClientOnly(ValueError("foo"))`

Inventing some special syntax is of course also plausible, I'm just wary of mixing the keywords return and raise in a way that suggests raise something is a value that can be returned. As I see it, our primary interest here is just to avoid logging in case the exception is taken up by somebody else, so some variant of the raise statement would be a more appropriate candidate. Perhaps async raise something? (Not a well-pondered proposal, just an example!)

And yes, the state of having no waiting clients when a method terminates is visible in the RTS. So a simple solution could be to just avoid logging an unhandled exception if there's at least one waiting client that can take over that responsibility (no new syntax or built-ins needed). But it should also be noted that it's entirely possible that a client does its await right after the async call has terminated with an exception. In that case the exception would be logged in both places, but that doesn't look like a big drawback to me. In fact, since we actually support the ability of letting multiple clients wait on the same message, there might be multiple log entries for a single exception anyway, even if we ensure that the server skips its entry.

nordlander avatar Nov 20 '23 09:11 nordlander

Right, so I think we need a short term solution because adding exceptions to normal control flow will now print endless amounts of Unhandled Exception messages and that's just not very good. Checking if there are waiting clients should fix that...

but I think in the longer term we still might want a way for the programmer to explicitly specify whether return-raising a message is the intention or if it is truly unhandled.

I suspect we will have syntax changes every now and then or changes to common enough builtins / stdlib that it is of equal size as syntax changes. Once we start stabilizing stuff a bit (but I think we will still have a long time of changes as mentioned in front of us), we could instead offer rewrite support in the compiler, so we simply upgrade code. Thus, doing raise ForClientOnly(ValueError("foo")) and changing to async raise ValueError("foo") (again, bad example) would be easy-peasy.

plajjan avatar Nov 20 '23 20:11 plajjan

In #1586 I have implemented a check so that exceptions are only considered unhandled (and thus printed) in the actor that raised the exception when there is no other actor waiting on our result. The more I think about this, the more I like it. Maybe it's the pareto solution. It really does feel good enough.

If anyone does a an assignment with explicit async, like

actor Foo():
    def foo():
        print("FOO")
        raise ValueError("FOO")

actor main(env):
    f = Foo()
    m = async f.foo()

    def blargh():
        # This function is called later
        await m

then we will print an extra unhandled exception... but we could probably fix that one up as well through compiler analysis, i.e. the compiler can check if we use the result from a called actor method at all (either await or just capturing the msg to await later) and pass this bit in the Msg so the RTS can act accordingly. I feel inclined to actually do that rather than meddle with new syntax or stuff to indicate intent.

plajjan avatar Nov 21 '23 10:11 plajjan