Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add optional exception checking #1773

Open
Dakkaron opened this issue Jun 30, 2016 · 10 comments
Open

Add optional exception checking #1773

Dakkaron opened this issue Jun 30, 2016 · 10 comments

Comments

@Dakkaron
Copy link

@Dakkaron Dakkaron commented Jun 30, 2016

Over here python/typing#71 it was discussed to add exception information to stubs, that can be interpreted by tools like mypy. It would be great to have that as an optional function that gives warnings on uncaught exceptions, especially on user-declared exceptions.

Are there any plans on adding a feature like that to mypy?

@JukkaL
Copy link
Collaborator

@JukkaL JukkaL commented Jun 30, 2016

I've thought about this a bit but there are no concrete plans. For this to work in practice, this would likely have to be opt-in: you don't need to annotate exceptions but you can, and you can define whether exceptions should be checked on a per-file basis.

I suspect that this would work best for exceptions that are mostly unpredictable (such as network related errors), especially if they happen pretty rarely so it's easy to forget about them. For things like KeyError there would likely be too many false positives for this to be useful.

@oakkitten
Copy link

@oakkitten oakkitten commented Sep 1, 2019

I would prefer annotated exceptions. Consider this silly example:

def who_is_on_the_toilet(house: House) -> Person:   # raises: NoOneIsInTheToilet
    for room in house.rooms:
        if isinstance(room, Toilet):
            if not room.person:
                raise NoOneIsInTheToilet
            return room.person
    raise ThereIsNoToiletInTheHouse

If by design of the application every house should have a toilet, in a correct application the last line shouldn't happen. We might want it still, however, to avoid weird behavior, for testing, and satisfying mypy. The caller wouldn't want to check for ThereIsNoToiletInTheHouse, as well as for any other exceptions that can occur in this method.

NoOneIsInTheToilet, on the other hand, would crash the caller quite soon as people don't usually sit in their toilets all day. Along with the return value, this exception is what the caller should expect to get from this method. It would make sense for the caller for either catch this exception, ignore it if the caller is sure that someone is on the toilet, e.g.

who_is_on_the_toilet(house)    # ignore-exception: NoOneIsOnTheToilet

or let it propagate

def prank_the_pooper(house: House) -> None   # raises: NoOneIsInTheToilet
	prank(who_is_on_the_toilet(house))

In other words, I'd like mypy to check for exceptions that are a part of the control flow of a correctly written application.

@ilevkivskyi
Copy link
Collaborator

@ilevkivskyi ilevkivskyi commented Sep 2, 2019

From purely theoretical point of view (because it is completely unclear when and how this will be implemented), I think it makes sense to use assert False in such cases. Mypy could then simply special-case AssertionError.

@oakkitten
Copy link

@oakkitten oakkitten commented Sep 2, 2019

assert False would be fine in a world of correct applications, but in reality most applications eventually crash, and having a sensible exception in the traceback would be nice—useful both during development and when an end-user encounters it.

If it was Java, I could perhaps throw a subclass of RuntimeException instead, but Python doesn't make the distinction between “recoverable” and “unrecoverable” exceptions; besides, unlike Java, Python much more heavily uses exceptions for control flow in the first place. You could raise a KeyError that your caller is supposed to check for, or you can make a mistake in code and get it raised otherwise.

If you think of exceptions as not only errors, but also as possible return values, I think it only makes sense to have them a part of the function signature.

@ilevkivskyi
Copy link
Collaborator

@ilevkivskyi ilevkivskyi commented Sep 2, 2019

Just to clarify, I meant that AssertionError (or a sublclass) should be used in the place of the second one, otherwise raising A from function that is annotated to raise B is like returning int from a function that is annotated to return str. AssertionError would be special-cased, indicating it is not a control flow, something actually weird happened.

But again, as I said this is all just my fantasies.

@graingert
Copy link
Contributor

@graingert graingert commented Jan 16, 2020

This would make more sense with a Failure type so you could adapt code with exceptions into a codebase that wants to entirely opt out of exceptions

@Dakkaron
Copy link
Author

@Dakkaron Dakkaron commented Jan 19, 2020

I don't think it would be good to couple checked exceptions to the AssertionError exception, because then you'd have to change your code to make sure an exception has to be caught.

I would rather do something like this:

def f(): # raises checked(ExceptionClass)

Additionally, you could declare an Exception class as checked:

class MyException(Exception): # checked

A class that is declared as checked would need to be checked every time it is raised.

This way you could make any kind of exception checked or not.

This could be helpful e.g. when writing a library that does network or other io stuff, so that you don't have to change all exceptions to be subclasses of AssertionError only to have them checked.

Also, having exception checking based on the exception's actual type would be useless when making a stub for a class. I believe that all mypy annotations must be implementable without actual code changes, otherwise it would break stubs.

@graingert
Copy link
Contributor

@graingert graingert commented Jan 19, 2020

Exceptions shouldn't be declared as checked or not - all exceptions should be checked

functions without a -> typing.raises_[T, E] annotation should default to -> typing.raises_[T, BaseException] and be allowed to call any code that throws anything

While code with a specific annotation would be limited to only calling checked functions that raise a subclass of that exception

This way existing code will still typecheck new code without exceptions will only be required to wrap external functions with a function that returns a Failure/Success instance

def try_(fn: Callable[[], raises_[T, E]) -> raises_[None, Union[Failure[E], Success[T]]:
try:
    return Success(fn())
except BaseException as e:
    return Failure(e)

@Dakkaron
Copy link
Author

@Dakkaron Dakkaron commented Jan 22, 2020

I don't think all exceptions should be checked. There are a lot of exceptions that could be raised on almost any line (e.g. RecursionError, ZeroDivisionError, IndexError, MemoryError, OverflowError, KeyError, NameError, ...). If all exceptions were checked, you would need to try/except every single line (because if a variable or a function is not defined you get a NameError) and then you would need to also try/except every line in the except-block. It would be physically impossible to check every possible exception.

This effect can also occur with user code. Say, you implement something similar to a dict. The __getitem__ method would need to raise a KeyError if the key is not in the dict. Thus every time you do x[y] where x is a dict instance, you would need to try/except that.

Thus it must be possible to declare an exception as checked or unchecked.

Also I would hate if this syntax could not be implemented as a stub. Having to change existing code would seriously break using mypy with 3rd-party libraries that have not been typed by the author of that library.

E.g. I use Django, which is not typed. The way I work with that is that I declare stubs for all methods that I call (in our setup we have mandatory typing enabled). That way I can safely update the library without having to merge anything. I only have to check that the stubs still fit with the interface declared by Django.

@Dakkaron
Copy link
Author

@Dakkaron Dakkaron commented Feb 18, 2020

By the way, what about a related feature that might overlap in usage but could be easier to implement:

This feature would take, as an input, the code base and a reference to a function and would return all uncaught exceptions that would be raised along the way, together with all lines of code that raise that specific exception. So e.g. for this code:

def f2(b):
    if b:
        raise Exception1()
    else:
        raise Exception2()

def f1(b):
    try:
        f2(b)
    except Exception1:
        raise Exception3

and the reference to function f1 it would return something like

Exception2: file test.py line 5
Exception3: file test.py line 11

This could be used either to check for all possible exceptions that a function could raise while creating code that uses the function, so that the programmer knows what exceptions to expect.

But it could also be used as a kind of static exception checker, if you input the reference to the main function / the main module. That way you can easily spot what exceptions could still be leaked.

Optionally, the programmer could supply the checker a list of exceptions to ignore, e.g. MemoryError. This way there is no need to differentiate between checked or unchecked exceptions, since the programmer decides which exceptions they want to check for and which ones to ignore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants