classification
Title: Exception handling should match subclasses, not subtypes
Type: Stage: resolved
Components: Interpreter Core Versions: Python 3.8, Python 3.7, Python 3.6, Python 3.4, Python 3.5
process
Status: closed Resolution: duplicate
Dependencies: Superseder: Allow catching virtual subclasses in except clauses
View: 12029
Assigned To: Nosy List: Michael McCoy, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2018-04-13 07:39 by Michael McCoy, last changed 2018-04-13 20:12 by serhiy.storchaka. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 6461 open python-dev, 2018-04-13 07:44
Messages (5)
msg315241 - (view) Author: Michael McCoy (Michael McCoy) * Date: 2018-04-13 07:39
Exception handling matches subtypes, not subclasses


# Example


    from abc import ABC
    
    class MyException(Exception, ABC):
        pass
    
    class OtherException(Exception):
        pass
    
    MyException.register(OtherException)
    
    try:
        raise OtherException
    except MyException:
        print("Correct: Caught MyException")
    except Exception:
        print("Wrong: Caught something else")
        
    # "Wrong: Caught something else"


# Background and evidence of bug-ness

Issue 2534 [1] (10 years ago!) introduced the behavior, but only in the Python 3 patch [2]. During code review, the correct function call was used [3], but the function's name got switched in the final python3 patch without any comment.

The current Python 2 code uses `PyObject_IsSubclass`, and produces the correct behavior in the example above (using `__metaclass__ = ABCMeta`, of course). This leads me to strongly suspect that this is a bug, not a feature. The note below regarding unittest for further evidence that this code has eight legs.

Given the ancient nature of this bug, it affects all versions of python3.

[1] https://bugs.python.org/issue2534
[2] https://bugs.python.org/file11257/isinstance3k-2.patch
[3] https://codereview.appspot.com/483/diff/1/21#newcode114
[4] https://github.com/python/cpython/blob/2.7/Python/errors.c#L119


# Solution

Coming very soon in a PR on Github, but in short, we do the following:

  1. Switch `PyType_IsSubtype` to  `PyObject_IsSubclass`.
  2. Revert the changes made to remove “dead code” in https://bugs.python.org/issue31091. The code was dead because the wrong function was used—the PR left only the bug.
  3. Add tests. Note that `unittest`’s `self.assertRaises` function uses `issubclass` and does not alert to this bug. (Different symptom, same cause.)


# Note

This bug has nothing to do with the `abc` package, beyond being a simple way to generate the error.


-Mike

Gitub: mbmccoy
msg315252 - (view) Author: Michael McCoy (Michael McCoy) * Date: 2018-04-13 18:32
Adding Serhiy because this relates to issue31091, and specifically claims that there _was_ a bug in the old code (msg299585).
msg315255 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-04-13 18:55
Isn't this a duplicate of issue12029?
msg315258 - (view) Author: Michael McCoy (Michael McCoy) * Date: 2018-04-13 19:20
Serhiy, it sure is. I'll note that the issue is open. Moreover, reading through the history, Guido says it's a bug (msg160418), and there was agreement that it should be fixed. It's unclear why it was dropped.

Can you help me get this in? I'm not sure the correct protocol, since this is my first contrib.

Note that if this is not a bug, then there are probably bugs anywhere issubclass(...) is used to emulate exception handling. For example, unittest's self.assertRaises is inconsistent with the current behavior:

    from abc import ABC
    import unittest
    
    class TestExceptionABC(unittest.TestCase):
    
        def test_exception(self):
            class A(Exception, ABC):
                pass
            class B(Exception):
                pass
            A.register(B)
            with self.assertRaises(A, msg="Wrong exception raised"):
                raise B

Test passes in python3, but not in python2.
msg315262 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-04-13 20:12
Let to keep the discussion in a single place -- in issue12029.
History
Date User Action Args
2018-04-13 20:12:26serhiy.storchakasetstatus: open -> closed
superseder: Allow catching virtual subclasses in except clauses
messages: + msg315262

resolution: duplicate
stage: patch review -> resolved
2018-04-13 19:20:20Michael McCoysetmessages: + msg315258
2018-04-13 18:55:19serhiy.storchakasetmessages: + msg315255
2018-04-13 18:32:15Michael McCoysetnosy: + serhiy.storchaka
messages: + msg315252
2018-04-13 18:18:24Michael McCoysettitle: Exception handling matches subclasses, not subtypes -> Exception handling should match subclasses, not subtypes
2018-04-13 18:16:25Michael McCoysettitle: Exception handling matches subtypes, not subclasses -> Exception handling matches subclasses, not subtypes
2018-04-13 07:44:52python-devsetkeywords: + patch
stage: patch review
pull_requests: + pull_request6155
2018-04-13 07:39:34Michael McCoycreate