classification
Title: Call `isinstance` instead of `issubclass` during exception handling
Type: enhancement Stage: resolved
Components: Interpreter Core Versions: Python 3.6
process
Status: closed Resolution: later
Dependencies: Superseder:
Assigned To: Nosy List: martin.panter, r.david.murray, sjoerdjob
Priority: normal Keywords:

Created on 2015-11-02 21:10 by sjoerdjob, last changed 2015-11-04 01:53 by r.david.murray. This issue is now closed.

Messages (7)
msg253946 - (view) Author: Sjoerd Job Postmus (sjoerdjob) Date: 2015-11-02 21:10
Currently Python2.7 calls `PyObject_IsSubclass` during exception handling. This allows some virtual-base-class machinery to make Python believe a certain class should match an exception while it in reality does not.

However, this does not necessarily give one really enough granularity when dealing with base libraries which are not as granular in raising exceptions as one would like (the Python base library before PEP3151 comes to mind).

Currently I used the trick of calling `isinstance(sys.exc_info()[1], cls)` in the `__subclasscheck__`, which is sort-of a great work-around.[*].

This method is great for sort-of emulating PEP3151 in Python2.7, and similar work can be done for other libraries: making the exception classes appear more granular based on properties of the instance of the exception.

My belief is that by calling the `isinstance` during exception handling, one can use virtual base classes (or should I say virtual base instances) to their fullest potential in writing cleaner code.

I think this is important because exception-handling is already a place where messy code is likely to occur, and we need all the support we can get in making it just a tad less messy.[**]

Note: Python3.5 calls `PyType_IsSubtype`, which has #12029 open for a change towards `PyObject_IsSubclass`. As soon as (or before) that's implemented, I'd like to propose a similar change for Python3.5+: call `PyObject_IsInstance` when the raised exception is an instance.

[*] See https://github.com/sjoerdjob/exhacktion/ for how I use the described hack to somewhat emulate PEP3151 in Python2.7.
[**] One question that comes to mind is: why not just write a wrapper around the offending library. (1): If it's the base library, almost nobody is going to bother. (2): even if it's not the base library, the wrapper will likely be even more function calls, which may cost performance in both the good and the bad cases, instead of just the bad cases.
msg253951 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-11-02 23:44
This does not sound like the kind of change we would accept for python 2.7.  (I also don't understand what the difference is between calling issubclass on the type and isinstance on the instance in this case.  But, then, I haven't looked at the code in question.)
msg253963 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-11-03 03:04
[Avoiding UTF-8 error]

This is an interesting idea that never occurred to me. It would be nice to support it if the performance impact isn’t a problem. By the sound of Issue 12029, performance is barely affected until you actually try to catch a “virtual exception”. So the impact of changing from subclass to instance checking shouldn’t be much worse if we do it right.

I agree this would be a new feature for a new release, not for 2.7 or 3.5.

BTW here’s another approach using the same sys.exc_info() hack that also works in Python 3:

>>> def catch_not_implemented():
...     '''Helper to catch "API not implemented" exceptions'''
...     [_, exc, _] = sys.exc_info()
...     if isinstance(exc, NotImplementedError):
...         return BaseException  # Catch the exception
...     if isinstance(exc, EnvironmentError) and exc.errno == ENOSYS:
...         return BaseException  # Catch the exception
...     if isinstance(exc, HTTPError) and exc.code == HTTPStatus.NOT_IMPLEMENTED:
...         return BaseException  # Catch the exception
...     return ()  # Don't catch the exception
... 
>>> try: raise OSError(ENOSYS, "Dummy message")
... except catch_not_implemented() as err: print(repr(err))
... 
OSError(38, 'Dummy message')
msg253996 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-11-03 15:10
Ah, now I understand.  That is definitely not something that could go into 2.7.  It is also a level of change that would need to go to python-ideas first, and possibly even a PEP.  Closing this pending such a discussion.
msg254030 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-11-03 23:39
FWIW I don’t see it as a drastic change. If the current patch for Issue 12029 went ahead, I imagine the change for instance checking could look like

@@ PyErr_GivenExceptionMatches()
-    /* err might be an instance, so check its class. */
-    if (PyExceptionInstance_Check(err))
-        err = PyExceptionInstance_Class(err);
@@ given_exception_matches_inner()
-    res = PyObject_IsSubclass(err, exc);
+    if (PyExceptionInstance_Check(err)) {
+        res = PyObject_IsInstance(err, exc);
+    } else {
+        res = PyObject_IsSubclass(err, exc);
+    }
msg254035 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-11-04 01:49
The code change may be minor, but the semantic change is not.  It needs discussion.
msg254036 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-11-04 01:53
Let me rephrase that.  The semantic change *may* not be.  That's what needs discussion :)
History
Date User Action Args
2015-11-04 01:53:04r.david.murraysetmessages: + msg254036
2015-11-04 01:49:38r.david.murraysetmessages: + msg254035
2015-11-03 23:39:18martin.pantersetmessages: + msg254030
2015-11-03 15:10:48r.david.murraysetstatus: open -> closed
messages: + msg253996

dependencies: - Allow catching virtual subclasses in except clauses
resolution: later
stage: resolved
2015-11-03 03:04:41martin.pantersetversions: - Python 2.7, Python 3.5
nosy: + martin.panter

messages: + msg253963

dependencies: + Allow catching virtual subclasses in except clauses
2015-11-02 23:44:28r.david.murraysetnosy: + r.david.murray
messages: + msg253951
2015-11-02 21:10:26sjoerdjobcreate