org.sonar.l10n.py.rules.python.S1143.html Maven / Gradle / Ivy
Why is this an issue?
Using return, break or continue in a finally block suppresses the propagation of any unhandled
exception which was raised in the try, else or except blocks. It will also ignore their return statements.
SystemExit is raised when sys.exit() is
called. KeyboardInterrupt is raised when the user asks
the program to stop by pressing interrupt keys. Both exceptions are expected to propagate up until the application stops. It is ok to catch them when
a clean-up is necessary but they should be raised again immediately. They should never be ignored.
If you need to ignore every other exception you can simply catch the Exception class. However you should be very careful when you do
this as it will ignore other important exceptions such as MemoryError
In python 2 it is possible to raise old style classes. You can use a bare except: statement to catch every exception. Remember to
still reraise SystemExit and KeyboardInterrupt.
This rule raises an issue when a jump statement (break, continue, return) would force the control flow to
leave a finally block.
Noncompliant code example
def find_file_which_contains(expected_content, paths):
file = None
for path in paths:
try:
# "open" will raise IsADirectoryError if the provided path is a directory but it will be stopped by the "return" and "continue"
file = open(path, 'r')
actual_content = file.read()
except FileNotFoundError as exception:
# This exception will never pass the "finally" block because of "return" and "continue"
raise ValueError(f"'paths' should only contain existing files. File ${path} does not exist.")
finally:
file.close()
if actual_content != expected_content:
# Note that "continue" is allowed in a "finally" block only since python 3.8
continue # Noncompliant. This will prevent exceptions raised by the "try" block and "except" block from raising.
else:
return path # Noncompliant. Same as for "continue"
return None
# This will return None instead of raising ValueError from the "except" block
find_file_which_contains("some content", ["file_which_does_not_exist"])
# This will return None instead of raising IsADirectoryError from the "try" block
find_file_which_contains("some content", ["a_directory"])
import sys
while True:
try:
sys.exit(1)
except (SystemExit) as e:
print("Exiting")
raise
finally:
break # This will prevent SystemExit from raising
def continue_whatever_happens_noncompliant():
for i in range(10):
try:
raise ValueError()
finally:
continue # Noncompliant
Compliant solution
# Note that using "with open(...) as" would be better. We keep the example as is just for demonstration purpose.
def find_file_which_contains(expected_content, paths):
file = None
for path in paths:
try:
file = open(path, 'r')
actual_content = file.read()
if actual_content != expected_content:
continue
else:
return path
except FileNotFoundError as exception:
raise ValueError(f"'paths' should only contain existing files. File ${path} does not exist.")
finally:
if file:
file.close()
return None
# This raises ValueError
find_file_which_contains("some content", ["file_which_does_not_exist"])
# This raises IsADirectoryError
find_file_which_contains("some content", ["a_directory"])
import sys
while True:
try:
sys.exit(1)
except (SystemExit) as e:
print("Exiting")
raise # SystemExit is re-raised
import logging
def continue_whatever_happens_compliant():
for i in range(10):
try:
raise ValueError()
except Exception:
logging.exception("Failed") # Ignore all "Exception" subclasses yet allow SystemExit and other important exceptions to pass
Resources
- Python documentation - the
try statement