Method

The analysis is based on the 8,000 most popular PyPI packages, in terms of number of downloads in the last 30 days. They were downloaded on the 17th-18th of October, using a script written by Guido van Rossum, which in turn relies on Hugo van Kemenade’s tool that creates a list of the most popular packages.

Once downloaded, a second script was used to construct an AST for each file, and traverse it to identify break, continue and return statements which are directly inside a finally block.

I then found the current source code for each occurrence, and categorized it. For cases where the code seems incorrect, I created an issue in the project’s bug tracker. The responses to these issues are also part of the data collected in this investigation.

Results

I decided not to include a list of the incorrect usages, out of concern that it would make this report look like a shaming exercise. Instead I will describe the results in general terms, but will mention that some of the problems I found appear in very popular libraries, including a cloud security application. For those so inclined, it should not be hard to replicate my analysis, as I provided links to the scripts I used in the Method section.

The projects examined contained a total of 120,964,221 lines of Python code, and among them the script found 203 instances of control flow instructions in a finally block. Most were return, a handful were break, and none were continue. Of these:

• 46 are correct, and appear in tests that target this pattern as a feature (e.g., tests for linters that detect it).
• 8 seem like they could be correct - either intentionally swallowing exceptions or appearing where an active exception cannot occur. Despite being correct, it is not hard to rewrite them to avoid the bad pattern, and it would make the code clearer: deliberately swallowing exceptions can be more explicitly done with except BaseException:, and return which doesn’t swallow exceptions can be moved after the finally block.
• 149 were clearly incorrect, and can lead to unintended swallowing of exceptions. These are analyzed in the next section.

The Error Cases

Many of the error cases followed this pattern:

try:
    ...
except SomeSpecificError:
    ...
except Exception:
    logger.log(...)
finally:
    return some_value

Code like this is obviously incorrect because it deliberately logs and swallows Exception subclasses, while silently swallowing BaseExceptions. The intention here is either to allow BaseExceptions to propagate on, or (if the author is unaware of the BaseException issue), to log and swallow all exceptions. However, even if the except Exception was changed to except BaseException, this code would still have the problem that the finally block swallows all exceptions raised from within the except block, and this is probably not the intention (if it is, that can be made explicit with another try-except BaseException).

Another variation on the issue found in real code looks like this:

    try:
        ...
    except:
        return NotImplemented
    finally:
        return some_value

Here the intention seems to be to return NotImplemented when an exception is raised, but the return in the finally block would override the one in the except block.

Author reactions

Of the 149 incorrect instances of return or break in a finally clause, 27 were out of date, in the sense that they do not appear in the main/master branch of the library, as the code has been deleted or fixed by now. The remaining 122 are in 73 different packages, and I created an issue in each one to alert the authors to the problems. Within two weeks, 40 of the 73 issues received a reaction from the code maintainers:

• 15 issues had a PR opened to fix the problem.
• 20 received reactions acknowledging the problem as one worth looking into.
• 3 replied that the code is no longer maintained so this won’t be fixed.
• 2 closed the issue as “works as intended”, one said that they intend to swallow all exceptions, but the other seemed unaware of the distinction between Exception and BaseException.

One issue was linked to a pre-existing open issue about non-responsiveness to Ctrl-C, conjecturing a connection.

Two of the issue were labelled as “good first issue”.

The correct usages

The 8 cases where the feature appears to be used correctly (in non-test code) also deserve attention. These represent the “churn” that would be caused by blocking the feature, because this is where working code will need to change. I did not contact the authors in these cases, so we need to assess the difficulty of making these changes ourselves. It is shown in the full report, that the change required in each case is small.

Discussion

The first thing to note is that return/break/continue in a finally block is not something we see often: 203 instance in over 120 million lines of code. This is, possibly, thanks to the linters that warn about this.

The second observation is that most of the usages were incorrect: 73% in our sample (149 of 203).

Finally, the author responses were overwhelmingly positive. Of the 40 responses received within two weeks, 35 acknowledged the issue, 15 of which also created a PR to fix it. Only two thought that the code is fine as it is, and three stated that the code is no longer maintained so they will not look into it.

The 8 instances where the code seems to work as intended, are not hard to rewrite.