classification
Title: Catching virtual subclasses in except clauses
Type: enhancement Stage: needs patch
Components: Interpreter Core Versions: Python 3.3
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Jim.Jewett, Trundle, Yury.Selivanov, benjamin.peterson, cvrebert, daniel.urban, eric.araujo, gcbirzan, gvanrossum, jamesh, pitrou
Priority: normal Keywords: patch

Created on 2011-05-08 08:53 by acooke, last changed 2012-09-16 21:37 by Trundle.

Files
File name Uploaded Description Edit
issue12029.patch gcbirzan, 2012-05-12 10:48 review
Messages (15)
msg135521 - (view) Author: andrew cooke (acooke) Date: 2011-05-08 08:53
Hi,

In general, registering a class with an ABC is equivalent to making it a subclass (isinstance and issubclass are patched through ABCMeta).  However, this does not work for exceptions (see example below, where exception is not caught).

This doesn't seem terribly surprising to me - I imagine that checking would slow down exception handling - but I couldn't find any documentation (and posting on c.l.p didn't turn up anything either).

So I thought I would raise it here - perhaps there is a possible fix (my obscure use case is that I have a backtracking search; backtracking occurs when a certain exception is encountered; making that exception an ABC and allowing existing exceptions to be registered with it allows the search to work with existing code without a wrapper that catches and translates exceptions that should trigger a backtrack).  Or perhaps the docs could be extended.  Or perhaps I've misunderstood something...

Cheers,
Andrew

Python 3.2 (r32:88445, Feb 27 2011, 13:00:05) 
[GCC 4.5.0 20100604 [gcc-4_5-branch revision 160292]] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> from abc import ABCMeta
>>> class RootException(Exception,metaclass=ABCMeta): pass
... 
>>> class MyException(Exception): pass
... 
>>> RootException.register(MyException)
>>> try:
...     raise MyException
... except RootException:
...     print('caught')
... 
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
__main__.MyException
msg136712 - (view) Author: Chris Rebert (cvrebert) * Date: 2011-05-24 02:10
Scouting around the CPython codebase a bit, I speculate that the cause of this behavior is that PyErr_GivenExceptionMatches() in errors.c uses PyType_IsSubtype() [which simply walks a class's __mro__ checking for pointer equality] rather than PyObject_IsSubclass()/PyObject_IsInstance() [which are smart enough to consult __subclasscheck__()/__instancecheck__() if they exist].

Of course, the more important issue here is whether this behavior is intended or not. I surmise python-dev needs to have a discussion about it?
msg136717 - (view) Author: Chris Rebert (cvrebert) * Date: 2011-05-24 02:45
Surveying the docs, the current behavior *is* /technically/ correct (in a suspiciously precise way) according to the Language Reference:
http://docs.python.org/dev/reference/compound_stmts.html#grammar-token-try_stmt :
"For an except clause with an expression [...] the clause matches the exception if the resulting object is 'compatible' with the exception. An object is compatible with an exception if it is the class or a base class of the exception object" (which exactly describes what PyType_IsSubtype() checks for)

The Tutorial is by contrast much more vague:
http://docs.python.org/dev/tutorial/errors.html#handling-exceptions :
"if [the raised exception's] type matches the exception named after the except keyword, the except clause is executed, and then execution continues after the try statement."
No definition of what it means for the types to "match" seems to be given.
msg160399 - (view) Author: James Henstridge (jamesh) Date: 2012-05-11 04:44
The documentation for ABCMeta.register() says that it makes the other class a "virtual subclass".  That would make the ABC a "virtual base class".

So whether the current behaviour is correct depends on whether you consider a "virtual base" to count as a base.  From the reasoning behind the introduction of ABCs, it certainly sounds like it should count.

Also, this is a feature that works correctly in Pyton 2.7, so could trip people up who are trying to move to Python 3.
msg160418 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2012-05-11 16:37
I agree it's a bug and should be fixed.  It's too confusing that there would be two slightly different interpretations of isinstance/issubclass where the isinstance() and issubclass() would be using the extended interpretation but the except clause would use the narrow interpretation.

The exception matching done by the except clause ought to be explainable in terms of issubclass/isinstance.
msg160426 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2012-05-11 17:04
I think being able to catch exception with ABCs is esssentially useless. The originally stated "usecase" can be simply solved by putting classes into a tuple and putting that in the except clause.

In general, the whole abc machinary causes lots of code which expects instance and subclass checks to be side-effect free to be able to execute arbitrary code, which creates messes.
msg160427 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-05-11 17:08
Perhaps ABCMeta could raise a UserWarning when creating an Exception subclass?
msg160428 - (view) Author: andrew cooke (acooke) Date: 2012-05-11 17:20
perhaps it could just work in a simple, consistent way?

in my original report i wondered whether there was a significant performance hit.  but so far the objections against fixing this seem to be (1) a lawyer could be convinced the current behaviour is consistent with the docs (2) python 3 should remain compatible with python 2 (3) abcmeta is the sucksorz.

those don't seem like great arguments against making it just work right, to me.
msg160430 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-05-11 17:32
> perhaps it could just work in a simple, consistent way?

That would be best obviously. But as Benjamin explained it's quite delicate to make it work while avoiding pitfalls where code involved in exception checking may itself fail with arbitrary errors - say, enter an infinite recursion. It's also why I think it would be a bad idea to fix it in 3.2 (the bugfix branch). In 3.3 we can take riskier decisions.
msg160432 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2012-05-11 17:37
Basically, someone needs to produce a patch and we can go from there.
msg160435 - (view) Author: George-Cristian Bîrzan (gcbirzan) Date: 2012-05-11 17:47
I posted on python dev that this would slow exception checking considerably so that is a concern. As for possible bugs, this has been working in the 2 branch for a while now, so I don't think that is the biggest issue. 
As for possible use cases, writing a wrapper around backend, each with its own exceptions and still being able to catch a 'base' exception in your code while still having the ability to catch specific exceptions, without doing awkward stuff like looking at __cause__ (let alone that you have to reraise that in 2 for code that has to run on both branches). Yes, you could patch the exceptions' bases but that is what Abc was created to avoid. 

Sorry for the mistakes and weird phrasing, posting this off my phone.
msg160436 - (view) Author: George-Cristian Bîrzan (gcbirzan) Date: 2012-05-11 17:49
I have a patch, with tests, but no Internet on my computer so going out, will post it when I get back/my Internet comes back
msg160461 - (view) Author: James Henstridge (jamesh) Date: 2012-05-12 00:54
Benjamin: if you are after a use case for this feature, see  https://code.djangoproject.com/ticket/15901

In Django, there are multiple database backends, each of which currently catch the adapter's DatabaseError and reraise it as Django's DatabaseError so that Django code can handle database errors in a standard way without having to care about which backend they came from.  Unfortunately, this loses some information from the exception.

My idea for solving that bug was to make Django's DatabaseError an ABC.  By registering the various adapter's DatabaseErrors with the ABC, it would not be necessary to catch and reraise them in the backends while still preserving the ability to catch the generic errors in the core.  This works fine in Python 2.x, but it was pointed out that it would cause compatibility problems when porting to Python 3.2.
msg160468 - (view) Author: George-Cristian Bîrzan (gcbirzan) Date: 2012-05-12 10:48
As promissed the patch. It doesn't break any tests, and it passes the ones I added. I have a pybench one as well, which even though trivial, does point to the fact that there is a degradation in performance, but not sure it's worth posting here.
msg161473 - (view) Author: Jim Jewett (Jim.Jewett) Date: 2012-05-24 02:26
When does the performance hit occur?

If it is only when an exception has been raised, and its own class is not listed by the except clause, then I personally wouldn't worry about it; tracing the MRO *could* get arbitrarily long already; it just doesn't in practice.  The same should be true of virtual subclassing.

On the other hand, if it adds another module or three to the required startup set, that might be a concern...
History
Date User Action Args
2012-09-16 21:37:51Trundlesetnosy: + Trundle
2012-05-24 02:26:45Jim.Jewettsetnosy: + Jim.Jewett
messages: + msg161473
2012-05-12 10:48:57gcbirzansetfiles: + issue12029.patch
keywords: + patch
messages: + msg160468
2012-05-12 01:09:05jameshsettype: behavior -> enhancement
2012-05-12 00:54:20jameshsettype: enhancement -> behavior
messages: + msg160461
2012-05-11 20:17:09Yury.Selivanovsetnosy: + Yury.Selivanov
2012-05-11 17:49:48gcbirzansetmessages: + msg160436
2012-05-11 17:47:57gcbirzansetnosy: + gcbirzan
messages: + msg160435
2012-05-11 17:37:20benjamin.petersonsetmessages: + msg160432
2012-05-11 17:32:28pitrousetmessages: + msg160430
2012-05-11 17:22:13acookesetnosy: - acooke
2012-05-11 17:20:31acookesetmessages: + msg160428
2012-05-11 17:09:00pitrousetnosy: + pitrou
messages: + msg160427
2012-05-11 17:04:52benjamin.petersonsetmessages: + msg160426
2012-05-11 16:57:39pitrousetnosy: + benjamin.peterson

type: behavior -> enhancement
versions: - Python 3.2
2012-05-11 16:44:47eric.araujosetstage: needs patch
versions: + Python 3.2
2012-05-11 16:37:40gvanrossumsetnosy: + gvanrossum
messages: + msg160418
2012-05-11 11:31:55eric.araujosettitle: ABC registration of Exceptions -> Catching virtual subclasses in except clauses
versions: + Python 3.3, - Python 3.2
2012-05-11 04:44:47jameshsetnosy: + jamesh
messages: + msg160399
2011-07-27 15:08:12eric.araujosetfiles: - generictramadolhclonline.html
2011-07-27 15:08:10eric.araujosetfiles: - cheapcodfedextramadolvery.html
2011-07-27 12:48:12junior1971setfiles: + generictramadolhclonline.html
2011-07-27 12:04:51junior1971setfiles: + cheapcodfedextramadolvery.html
2011-05-24 02:45:08cvrebertsetmessages: + msg136717
2011-05-24 02:10:15cvrebertsetmessages: + msg136712
2011-05-13 17:07:20eric.araujosetnosy: + eric.araujo
2011-05-08 09:32:58daniel.urbansetnosy: + daniel.urban
2011-05-08 09:02:07cvrebertsetnosy: + cvrebert
2011-05-08 08:53:43acookecreate