Skip to content
This repository has been archived by the owner on Nov 2, 2022. It is now read-only.

Consider decorator-based exception catch API #5

Open
njsmith opened this issue Jan 19, 2019 · 17 comments
Open

Consider decorator-based exception catch API #5

njsmith opened this issue Jan 19, 2019 · 17 comments

Comments

@njsmith
Copy link
Member

njsmith commented Jan 19, 2019

See python-trio/trio#611 (comment)

This is probably nicer than with catch? @1st1 hates with catch so maybe he'll like this better :-)

@njsmith
Copy link
Member Author

njsmith commented Jan 21, 2019

Pasting from that comment:

try:
    ...
except:
    @ExceptionGroup.handle(FooException)
    def handler(exc):
        ...

One limitation of this approach is that handler has to be synchronous. Python doesn't have "async decorators". That's probably not a killer, but it is worth noting.

try:
    ...
except:
    handler_chain = exceptiongroup.HandlerChain()
    @handler_chain(FooException)
    def handler(exc):
        ...
    @handler_chain(BarException)
    def handler(exc):
        ...
    handler_chain.run()

This version could support async handlers, with a bit of extra complexity. (I guess we need both sync and async versions of handler.run(), etc.)

@abetkin
Copy link

abetkin commented Jan 22, 2019

You can probably use assignment expressions too)

try:
except:
  if exc := match(group := get_exception(), MyException):
    print(exc)
  elif match(group, SecondException):
    print('second exception')

@njsmith
Copy link
Member Author

njsmith commented Jan 22, 2019

Unfortunately, the ExceptionGroup catching code is extremely complex, due to all the hacks it has to use to convince python's hard-coded exception-handling not to interfere. For example: if the custom handler itself raises an exception, then that needs to be merged back into the ExceptionGroup, with its __context__ set correctly. There's like 30 lines of code that have to be wrapped around the actual exception handler: https://github.com/python-trio/exceptiongroup/blob/master/exceptiongroup/_tools.py

So in practice, there's really no way to avoid putting the handler into something heavyweight like a function, or mayyybe a with block. Actually, I don't think even a with block is quite powerful enough, because there's no way for __enter__ to set up the interpreter's exception context correctly.

@abetkin
Copy link

abetkin commented Jan 23, 2019

Imho this can't compete with context manager version:

try:
except:
  with get_exception() as group:
    if ex := handle(group, Exception):
      raise OtherException

In the above example handle is destructive and modifies group to contain the rest of exceptions (unhandled)

@njsmith
Copy link
Member Author

njsmith commented Jan 23, 2019

Is the idea that you would chain these to handle multiple exceptions, like this?

try:
    ...
except:
    with get_exception() as group:
        if ex := handle(group, OSError):
            raise OtherException
        if ex := handle(group, ValueError):
            log(ex)

That won't work because after the first if branch raises OtherException, then the second if branch won't run at all.

Also, it's very difficult to get OtherException.__context__ set correctly here. With the handler function approach we can set the exc that's being handled as the current pending exception before calling the function, so that it's correctly set as __context__ for any new exceptions and you can use a bare raise to re-raise it, just like a regular except: block. But because of technical limitations, you can't hide this logic inside a context manager.

@abetkin
Copy link

abetkin commented Jan 23, 2019

Exactly, then probably only replacing both ifs with with will work. So you can suppress all the exceptions and accumulate them. But yes, you have less flexibility with context managers then with functions.

@njsmith
Copy link
Member Author

njsmith commented Jan 23, 2019

You mean, something like this?

try:
    ...
except:
    with open_handler() as handler:
        with handler.handle(OSError) as exc:
            raise OtherException
        with handler.handle(ValueError) as exc:
            log(exc)

This version does have a lot going for it. But I'm still not sure we can get acceptable handling of implicit chaining and re-raising.

@njsmith
Copy link
Member Author

njsmith commented Jan 23, 2019

Another interesting trade-off between these different approaches: normally in py3, when you write except ... as exc:, the exc variable is cleared at the end of the except block. (I think the motivation is because py3 exceptions hold tracebacks which pin stack frames and all their variables in memory, so you maybe don't want to accidentally hold onto them.)

In the handler-function approaches, you get this effect automatically, because the exc is restricted to the inner finding function's scope. In the with variant above, though, the exc variable leaks out into the enclosing function.

@abetkin
Copy link

abetkin commented Jan 23, 2019

Well, anyway it doesn't look too good. There is a crazy idea how to replace a couple of functions with one:

try:
except:
  def handler(ex):
    if  match(MyException, ex):
      ...
    elif match(OtherException, ex):
      ...
  run(handler)

You can make that work if the match function understands special objects passed as 2nd argument. For example if match(_, MySpecialObject()) always evaluates to false, but records all the calls made, you can get the list of possible matches.

Of course, good run function should check that handler consists only of if match() statements, by looking at AST :)

@njsmith
Copy link
Member Author

njsmith commented Jan 23, 2019

Another option to consider is a hybrid:

try:
    ...
except:
    async with open_handler() as handler:
        @handler.handle(OSError)
        def handler(exc):
            ...
        @handler.handle(RuntimeError)
        async def handler(exc):
            ...

@njsmith
Copy link
Member Author

njsmith commented Jan 23, 2019

Btw, there is no reliable way to get from a function object to its AST :-(

@njsmith
Copy link
Member Author

njsmith commented Feb 4, 2019

@WindSoilder asked for more details on the actual semantics of these different ideas. Fair question! :-)

Normally when people want to handle an exception, they write something like:

try:
    ...
except ExcType as exc:
    # some arbitrary code involving exc, possibly re-raising it or raising a new exception...
    ...

In the original "MultiError v2" proposal (python-trio/trio#611), these was the idea of a catch context manager. The idea is that you'd write:

with catch(ExcType, my_handler, match=my_predicate):
    ...

and this would be an "exception-group aware" version of this code:

try:
    ...
except ExcType as exc:
    if my_predicate(exc):
        # handler is some arbitrary code involving exc, possibly re-raising it or raising a new exception...
        handler(exc)

In particular:

  • handler is always called exactly zero or one times. If there were any exceptions raised that matched ExcType+my_predicate, then it's called once; otherwise it's called zero times.
  • handler can swallow the exception, re-raise the exception, or raise a new exception
  • the return value from handler is ignored

Actually implementing this is pretty complicated:

  • You have to handle all the different cases of no exception, exception group where everything matches the ExcType+my_predicate, exception group where nothing matches ExcType+my_predicate, exception group where part of it matches and part of it doesn't
  • You have to handle all the different things that handler can do (exit normally, raise original exception, raise new exception)
  • If handler was only being called on part of an exception group, you have to take any exception that comes out of handler and combine it back with the un-caught parts of the group
  • And all the while, Python is trying to stick "helpful" stuff in __traceback__ and __context__, and we have to manually undo this

There's a first (untested) attempt here: https://github.com/python-trio/exceptiongroup/blob/master/exceptiongroup/_tools.py#L106-L146

Here's an actual example. Suppose you're writing an HTTP server. You'll probably want to have a "catch-all" handler around each request, that catches any Exceptions (but not KeyboardInterrupt!), and logs them. In regular Python:

try:
    ...
except Exception as exc:
    logger.log_exception(exc)

With catch, this would look like:

def handler(exc):
    logger.log_exception(exc)

with catch(Exception, handler):
    ...

This is annoying, though, because we've had to flip our code upside down: the handler comes first, instead of at the end.

So the first proposal here was that we could instead write it like:

try:
    ...
except:
    @handle(Exception)
    def handler(exc):
        logger.log_exception(exc)

This would have exactly the same semantics as with catch(Exception, handler), just it's laid out differently in the code. This means the implementation would have to jump through slightly different hoops (e.g. context managers get the current exception's info passed directly into their __exit__ method, while handle here would have to retrieve it from sys.exc_info()), but all the other things are the same: handler gets called either zero or one times, it can raise or not, etc.

(Reminder: @ decorators are just a shorthand for something like:

    def handler(exc):
        logger.log_exception(exc)
    handler = handle(Exception)(handler)

Here we're abusing this as a way to run handle immediately; we don't care about the return value that gets assigned to handler.)

Multiple except blocks

But, what if we have some code like this, that we want to convert to handle exception groups?

try:
    ...
except OSError as exc:
    do_one_thing(exc)
except RuntimeError as exc:
    do_something_different(exc)

That's the idea of the last proposal – you would write it like:

try:
    ...
except:
    with open_handler() as handler:
        @handler.handle(OSError)
        def handler1(exc):
            do_one_thing(exc)

        @handler.handle(RuntimeError)
        def handler2(exc):
            do_something_different(exc)

Now, there's a subtlety: what if our exception is something like ExceptionGroup([ValueError(), OSError(), RuntimeError()]), i.e. it matches multiple handlers?

I think the right semantics are: we should walk through the handlers from top to bottom. At each moment we have a "remaining" exception – initially this is the entire ExceptionGroup, but then we repeatedly split off the part of it that matches each handler, and then move on to the next handler. Then at the very end, we collect up all the exceptions from all the handlers, plus any remaining uncaught exception, and bundle them back up into a new ExceptionGroup. So something like:

remaining = sys.exc_info()[0]
to_rethrow = []
for this_exc_type, this_handler, this_match in registered_handlers:
    if remaining is None:
        break
    this_handler_caught, remaining = split(exc_type, remaining, match=match)
    if this_handler_caught is not None:
        try:
            this_handler(this_handler_caught)
        except BaseException as this_handler_raised:
            to_rethrow.append(this_handler_raised)
if remaining is not None:
    to_rethrow.append(remaining)
raise ExceptionGroup(to_rethrow)

I'm sure the implementation will be more complicated than this in practice! But hopefully that gives the idea.


One nice thing about this part: we don't necessarily have to get this right for the first release, or implement all the features. This is a separate piece of code from the core ExceptionGroup and split algorithms, and if we have multiple versions, eh, that's fine. So we might want to start out by implementing something relatively simple and then seeing how it goes.

@WindSoilder
Copy link
Contributor

WindSoilder commented Feb 13, 2019

I fell into trouble to think about the following case:

try:
    ...
except:
    with open_handler() as handler:
        @handler.handle(RuntimeError)
        def handler1(exc):
              do_one_thing(exc)
              raise exc

        @handler.handle(ValueError)
        def handler2(exc):
              do_another_thing(exc)
              raise exc

        # do something that raise
        # ExceptionGroup(
        #    "many error",
        #    [RuntimeError("error1"), ValueError("error2")],
        #    ["error1", "error2"]
        # ) 

The function handler1 and handler2 will get an ExceptionGroup object, and they re-raise that ExceptionGroup :(
So the re-raised list contains two ExceptionGroup object....If we do nothing with them, we will get many ExceptionGroup when we exit with open_handler() as handler block.
It's hard to read I think..
Or we can merge these re-raised exceptiongroups into one and then re-raised that one?

@h-vetinari
Copy link

IMO the most pythonic-looking (and ergonomic!) API proposal was from @maxfischer2781 in this comment:

try:
    async with something:
        something.raise_concurrent()
except MultiError[A]:
     print("Handled concurrent A")
except MultiError[A, B] as err:
     print("Handled concurrent A and B")
except MultiError[B], MultiError[C] as err:
     print("Handled concurrent B or C:", err)
except A:
    print("Handled non-concurrent A")

This hasn't received an answer in that other thread, but it should be reflected here as well in any case, I think.

@maxfischer2781
Copy link

I wouldn't mind contributing a proposal for that API via PR if there is any interest. Is it realistic that this would be reviewed/used?

@graingert
Copy link
Member

how about with PEP 622

try:
    async with something:
        something.raise_concurrent()
except MultiError as e:
    match e:
        case MultiError(OSError(errno)):
            print(f"{errno=}")

@graingert
Copy link
Member

graingert commented Sep 30, 2020

I think if exceptiongroup lands in CPython, supporting the following syntax will be quite persuasive:

eg

try:
    async with something:
        something.raise_concurrent()
match MultiError as e:
    case MultiError(OSError(errno)):
        print(f"{errno=}")

or even:

try:
    async with something:
        something.raise_concurrent()
except case MultiError(OSError(errno)):
    print(f"{errno=}")

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

No branches or pull requests

6 participants