[Python-ideas] Use `isinstance` check during exception handling

[sjoerdjob at sjec.nl]

Hi all,

TL;DR: Change exception checking logic from
    err.__class__ in exc.__mro__
to
    isinstance(err, exc)
Because it is more powerful, more consistent and allows for cleaner
code.


The reason I am proposing this change is that quite often, one has to
deal with libraries behaving 'badly' with regard to exception
granularity.

I will take the Python base library before PEP3151 [1] as an example. It
is not hard to imagine there are a lot of other cases of libraries that
behave with little granularity in the exception classes.

    try:
        some(operation)
    except OSError as err:
        if err.errno == errno.ENOENT:
            # handle file-not-exists
        elif err.errno == errno.EACCES:
            # handle permission stuff.
        else:
            raise

After PEP3151 one can now write:

    try:
        some(operation)
    except FileNotFoundError:
        # handle file-not-exists
    except PermissionError:
        # handle permission stuff.

Now, PEP3151 took over a year to be resolved, judging by the dates. If
we have to go through changes like this more often, one will have to
wait quite a while. Also, consider backwards compatibility issues. Code
using FileNotFoundError will only run on Python3.3+.

Thus I propose to have the exception handling code use the normal
`isinstance` machinery, as it is more powerful, and allows one to use
virtual base classes to check if the instance of the exception is
relevant to catch. An example:

    import errno

    # Fake `FileNotFoundError` in Python3.2
    class ENOENTCheck(type):
        def __instancecheck__(cls, inst):
            return isinstance(inst, EnvironmentError) and \
                   inst.errno == errno.ENOENT
    class FileNotFoundError(EnvironmentError, metaclass=ENOENTCheck):
        pass

    try:
        open("no-such-file")
    except Exception as e:
        print(isinstance(e, FileNotFoundError))

This would print `True`. But, using `FileNotFoundError` as a class to
catch will not work, because the `__mro__` is checked instead.

I proposed the change on the Python issue tracker [2] already, but it
was (rightfully, I think) decided that it needs some more discussion
because it will bring a (potentially huge) semantic change.

As an opening, here are some pros and cons I can think of:

Cons:

- *Code already written might break*: Highly unlikely. If you have
  written code that overrides `isinstance` logic, you'd more than likely
  not write code that depends on there being a difference in matching
  logic between the exception handling and normal `isinstance` calls.

- *Performance will go down*: Yes, instance-checking is probably a lot
  more expensive then subclass-checking. I haven't made the change and
  benchmarked it yet, but based on Martin Panter's comment [3], it might
  only cost a lot when actually checking against a virtual base class.
  If cost is limited to those cases, I think the cost is negligible.

- *Exception handling like this is just magic*: Yes, anything regarding
  the metaclasses might be considered as magic. But metaclasses also
  allow you to build great abstractions, like for instance the ORM
  included with Django. I think this is one of the cases where the
  benefits outweigh the cost.

Pros:

- *Cleaner code*: The same benefit PEP 3151 brought for the base library
  and OS errors can be extended to all the Python libraries that lack
  granularity.

- *Consistency*: Who would ever expect that you can not always catch an
  exception `foo` using class `Bar` when `isinstance(foo, Bar)`.

Even though I can currently think of more downsides than upsides, I do
think that the downsides are really minor, while the upsides are huge:
clean code and consistency.

Also, I set up a proof-of-concept PEP3151 compatibility layer for
Python2.7 that sort-of fakes this feature. It fakes it by checking the
instance `sys.exc_info()[1]` in the `__subclasscheck__`. For the proof
of concept see [4]. I only found out during porting to Python3 that it
does not actually work, because of the direct checking of the `__mro__`,
but that's hopefully going to be fixed soon-ish [5].

I'd be interested to hear your thoughts about this. Any more pros and
cons would be welcome, as well as your judgement as to whether the
breaking of backwards compatibility is potentially a bad idea.

Kind regards,
Sjoerd Job Postmus

[1] PEP 3151 -- Reworking the OS and IO exception hierarchy
    https://www.python.org/dev/peps/pep-3151/
[2] Call `isinstance` instead of `issubclass` during exception handling
    http://bugs.python.org/issue25537
[3] http://bugs.python.org/issue25537#msg253963
[4] https://github.com/sjoerdjob/exhacktion/
[5] Catching virtual subclasses in except clauses
    http://bugs.python.org/issue12029