-
Notifications
You must be signed in to change notification settings - Fork 263
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
Reconsider the advice on exception types #9
Comments
Using very generic exceptions types (especially
I think a better rubric would be:
|
@lucaswiman I think you have a good point there. Thank you for the suggestion. I think this section will definitely get some rework soon. |
Some interesting thoughts on exceptions in Python from the Launchpad team here: |
Apologies for not posting a response yet. I'll try to do more soon. But in general my real advice is closer to Hettinger's from his "Beyond PEP-8" talk. Yes, use them to disambiguate. But what I see in practice and rail against in code reviews is custom exception spam. |
I think this is bad advice. Python 3 added many subclasses of |
@chepner Thank you for this comment -- I didn't know about this stdlib change in Python 3 re: |
My general advice on exceptions is two-fold. First, resist the urge to catch exceptions except when absolutely necessary---it is often better to just let the code crash or for errors to be caught at a higher level. Second, if you're going to intentionally raise an exception, use a custom exception. The latter often helps separate cases of "I meant to do that" (custom exception) versus "there's a programming error in my code" (built-in exception). |
I'd add there is a social element to it as well. I've been on too many projects where people won't read the freaking traceback message--instead they just copy/paste it into the issue tracker with some vague "code's broken" message. I really don't want to be someone's personal debugger. If I see one my custom exceptions at the bottom, I can just look at it and pretty much know what they did "Oh, you're getting THAT error because you did X." On the other hand, if I see a Python built-in exception at the bottom, well, then maybe there's a bug in my code. I know that because I would never intentionally raise a built-in exception. |
I noticed that people found the "rarely create exception types" suggestion in this style guide problematic, for some very valid reasons.
There is the worry that by raising built-in types, like
ValueError
andLookupError
, you could mask actual coding errors. It was suggested that it's always better to subclass these types and raise your own exception type, to make the difference clear. This was in a discussion with @ramalho and @dabeaz on Twitter.Quite a few people brought up that the impulse for this suggestion is that a caller shouldn't need to know about 10 different exception types to catch every last well-defined error condition. It seems most agree on that. @jackdied said: "Most exceptions I see are defined once, raised once and caught once." This would indeed be bad style. Many suggested that when making many different types, you could simplify life for the caller by having a "root" error type for a given library/API, and making all special errors subclass this.
I took a look at how
requests
does this. They introduce 19 exception types related to HTTP and a few of them subclass built-in exception types. Most of them also sub-class a "root" type, calledRequestException
, which seems to cover library-specific errors you may choose to intentionally ignore, likeInvalidURL
.https://github.com/kennethreitz/requests/blob/master/requests/exceptions.py
I doubt that most user code should be creating exception trees like this -- HTTP can fail in a number of specific and well-known ways, and this is a library of standard library quality. It's also important to point out that even
requests
chooses to keep most of these types implementation-free.So, I wonder if I'm too harsh on exception types since Python does make it exceedingly easy to create no-impl exception classes that are instantly useful in explaining well-known errors and disambiguate them from code problems. I'll reconsider this one.
The text was updated successfully, but these errors were encountered: