Repository navigation
MultiError.catch should take an exception type, like except: does #408
Description
In regular Python, a catch-all exception handler looks like:
try:
...
except Exception as exc: # Don't catch KeyboardInterrupt, etc.
logger.error(..., exc_info=exc)In Trio we need to handle MultiErrors, which might contain a mix of exceptions we want to catch and ones we don't, etc. So you have to use MultiError.catch, and the above becomes:
def handler(exc):
if isinstance(exc, Exception):
logger.error(..., exc_info=exc)
return None # swallow this exception
else:
return exc # let other exceptions propagate
with MultiError.catch(handler):
...This is pointlessly cumbersome. We should make it look like this:
def handler(exc):
logger.error(..., exc_info=exc)
with MultiError.catch(Exception, handler): # Note the extra argument
...
Activity
WindSoilder commentedon Aug 18, 2018
For the newer version of catch, if we have
Does it means that we want the MultiError filter out the
RuntimeError(andCancelled) ?If so, what if we want to keep the behavior default? (just filter out the
Cancelledexception)....May be just pass
Noneto catch? So the code would be something like:njsmith commentedon Aug 18, 2018
@WindSoilder the idea is that
is a substitute for
The work the same except the first one handles
MultiErrors.When you talk about filtering
Cancelled, you're thinking of what cancel scope blocks do.MultiErroritself doesn't know anything about cancellation. In fact,open_cancel_scopeusesMultiError.catchto catchCancelledexceptions. So that's a higher level than all this.WindSoilder commentedon Aug 21, 2018
@njsmith Thanks for your comment! It makes me more clear about this.
But there is something still confuse me, take the code as example:
According to your description, when handler is called, it's argument type(
Exception) can beRuntimeErrororMultiError.I have take a look at
_exc_filterin CancelScope, whenMultiError.catchtake an exception type,_exc_filterstill need to check if the exception type isCancelled, because it can beMultiError.Maybe this question is too foolish, but it still confuse me...I still don't understand the value of add an exception type to
catch:(May be it's better to just let
handlergetCancelledexception? ThenMultierrorcan be handled inMultiErrorCatchernjsmith commentedon Aug 21, 2018
With the current
MultiErrordesign, thecatchhandler never gets passedMultiErrorobjects. I'm not sure what you saw that makes you think otherwise. Currently, the handler gets called on all the basic exception objects, so e.g. inMultiError([Cancelled(), ValueError()]), the_exc_filterinCancelScopewould get called twice: one withCancelledand once withValueError. So theisinstancecheck you're seeing is to ignore theValueError. The original idea for this issue was to simply move this check intoMultiError.catch, since otherwise every handler ends up reimplementing it.With the "MultiError v2" proposal (#611), this changes slightly, and the exception type becomes more important. This proposal isn't implemented yet. But here, the idea is
MultiError.catch(RuntimeError, handler)takes whatever exception comes out of the code block, and then splits it into two halves: theRuntimeErrorhalf and the non-RuntimeErrorhalf. This uses thesplitoperation described in #611. And then it calls the handler on theRuntimeErrorhalf. This might be a simpleRuntimeError, or it might be aMultiError... but if it's aMultiError, then it's one that only containsRuntimeErrors. So again, the handler doesn't have to check the exception type explicitly: it knows that if it's running at all, it's because there was at least oneRuntimeError.belm0 commentedon May 28, 2019
I haven't found catch() to be useful, and this improvement wouldn't help.
The problem with the catch API is that the handler can only see one exception at a time. So far I've needed to see all exceptions to decide the desired filtering.
Scenario: I want to defer any exceptions raised by my async API to Cancelled if they are raised simultaneously (e.g. if move_on_after expires I'm fine dropping any API exceptions). So if MultiError contains Cancelled, then filter my API's exceptions. It can't be done with catch().
njsmith commentedon May 28, 2019
@belm0 Interesting... are you up for writing a longer description of what you're trying to do in the v2 thread, #611? That's where the main design discussions have been happening for the
MultiErrorrewrite, and so far everything has been based around the simplifying assumption that the way you handle one exception type in aMultiErrorisn't affected by the presence or absence of other exception types...Zac-HD commentedon Oct 30, 2022
Closing this because
MultiError.catch()is deprecated in favor ofexceptiongroup.catch()(orexcept*, on Python 3.11).