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

Warning on the catching of all exception types #256

Closed
pyflakes-bot opened this issue Sep 27, 2016 · 2 comments
Closed

Warning on the catching of all exception types #256

pyflakes-bot opened this issue Sep 27, 2016 · 2 comments

Comments

@pyflakes-bot
Copy link

Original report by conrad-p-dean on Launchpad:


It's generally bad practice to catch all exception types like this:

   try:
      o = open('file.txt', 'rb')
      data = json.loads(o.read())
   except:
      print("Can't parse file")

For example, sometimes the except block handles the wrong sort of exception. There are many sorts of exceptions that could have been thrown in the example above (file not found, bad json format, etc), and the original exception would have done a much better job of informing us of what went wrong. The author should only catch specific types of exceptions for specific ways to handle them.

It would be great if a rule could be added to catch instances where all exceptions are being handled, either in the above form or with the catch-all except Exception:. Where an explicit catch-all could be silenced with a # NOQA in instances where they really want everything caught.

@pyflakes-bot
Copy link
Author

Original comment by asmeurer (@asmeurer?) on Launchpad:


Sadly there are a lot of false positives here, and pyflakes tries to avoid all false positives. While one class of false positives can be ignored easily (if there is a 'raise' call in the except block), not all have that form. It's a bit of a style issue. For those cases where you really do want to catch all exceptions, it "really" better to write "except:" instead of "except BaseException:"? And do you "really" want to be catching BaseException instead of Exception? Usually you don't, but not always.

"except Exception:", the less bad variant, is very common in acceptable forms (i.e., false positives for a rule that disallows them).

@bitglue
Copy link
Member

bitglue commented Jun 1, 2017

Won't fix this: we can't throw an error on a construct which is sometimes useful. Like:

start_transaction()
try:
    do things()
except:
    rollback_transaction()
    raise
else:
    commit_transaction()

This is a case where catching any exception is really the right thing to do, and this is the right way to do it.

@bitglue bitglue closed this as completed Jun 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants