PEP: 765 Title: Disallow return/break/continue that exit a finally block Author: Irit Katriel , Alyssa Coghlan Discussions-To: https://discuss.python.org/t/pep-765-disallow-return-break-continue-that-exit-a-finally-block/71348 Status: Draft Type: Standards Track Created: 15-Nov-2024 Python-Version: 3.14 Post-History: `09-Nov-2024 `__, `16-Nov-2024 `__, Replaces: 601 Abstract ======== This PEP proposes to withdraw support for ``return``, ``break`` and ``continue`` statements that break out of a ``finally`` block. This was proposed in the past by :pep:`601`. The current PEP is based on empirical evidence regarding the cost/benefit of this change, which did not exist at the time that :pep:`601` was rejected. It also proposes a slightly different solution than that which was proposed by :pep:`601`. Motivation ========== The semantics of ``return``, ``break`` and ``continue`` in a finally block are surprising for many developers. The :ref:`documentation ` mentions that: - If the ``finally`` clause executes a ``break``, ``continue`` or ``return`` statement, exceptions are not re-raised. - If a ``finally`` clause includes a ``return`` statement, the returned value will be the one from the ``finally`` clause’s ``return`` statement, not the value from the ``try`` clause’s ``return`` statement. Both of these behaviours cause confusion, but the first is particularly dangerous because a swallowed exception is more likely to slip through testing, than an incorrect return value. In 2019, :pep:`601` proposed to change Python to emit a ``SyntaxWarning`` for a few releases and then turn it into a ``SyntaxError``. It was rejected in favour of viewing this as a programming style issue, to be handled by linters and :pep:`8`. Indeed, :pep:`8` now recommends not to use control flow statements in a ``finally`` block, and linters such as `Pylint `__, `Ruff `__ and `flake8-bugbear `__ flag them as a problem. Rationale ========= A recent `analysis of real world code `__ shows that: - These features are rare (2 per million LOC in the top 8,000 PyPI packages, 4 per million LOC in a random selection of packages). This could be thanks to the linters that flag this pattern. - Most of the usages are incorrect, and introduce unintended exception-swallowing bugs. - Code owners are typically receptive to fixing the bugs, and find that easy to do. See `the appendix <#appendix>`__ for more details. This new data indicates that it would benefit Python's users if Python itself moved them away from this harmful feature. One of the arguments brought up in `the PEP 601 discussion `__ was that language features should be orthogonal, and combine without context-based restrictions. However, in the meantime :pep:`654` has been implemented, and it forbids ``return``, ``break`` and ``continue`` in an ``except*`` clause because the semantics of that would violate the property that ``except*`` clauses operate *in parallel*, so the code of one clause should not suppress the invocation of another. In that case we accepted that a combination of features can be harmful enough that it makes sense to disallow it. Specification ============= The change is to specify as part of the language spec that Python's compiler may emit a ``SyntaxWarning`` or ``SyntaxError`` when a ``return``, ``break`` or ``continue`` would transfer control flow from within a ``finally`` block to a location outside of it. These examples may emit a ``SyntaxWarning`` or ``SyntaxError``: .. code-block:: :class: bad def f(): try: ... finally: return 42 for x in o: try: ... finally: break # (or continue) These examples would not emit the warning or error: .. code-block:: :class: good try: ... finally: def f(): return 42 try: ... finally: for x in o: break # (or continue) CPython will emit a ``SyntaxWarning`` in version 3.14, and we leave it open whether, and when, this will become a ``SyntaxError``. However, we specify here that a ``SyntaxError`` is permitted by the language spec, so that other Python implementations can choose to implement that. The CPython implementation will emit the ``SyntaxWarning`` during ``AST`` construction, to ensure that the warning will show up during static anlaysis and compilation, but not during execution of pre-compiled code. We expect that the warning will be seen by a project maintainer (when they run static analysis, or CI which does not have precompiled files). However, end users of a project will only see a warning if they skip precompilation at installation time, check installation time warnings, or run static analysis over their dependencies. Backwards Compatibility ======================= For backwards compatibility reasons, we are proposing that CPython emit only a ``SyntaxWarning``, with no concrete plan to upgrade that to an error. Code running with ``-We`` may stop working once this is introduced. Security Implications ===================== The warning/error will help programmers avoid some hard to find bugs, so will have a security benefit. We are not aware of security issues related to raising a new ``SyntaxWarning`` or ``SyntaxError``. How to Teach This ================= The change will be documented in the language spec and in the What's New documentation. The ``SyntaxWarning`` will alert users that their code needs to change. The `empirical evidence <#appendix>`__ shows that the changes necessary are typically quite straightforward. Rejected Ideas ============== Emit ``SyntaxError`` in CPython ------------------------------- :pep:`601` proposed that CPython would emit ``SyntaxWarning`` for a couple of releases and ``SyntaxError`` afterwards. We are leaving it open whether, and when, this will become a ``SyntaxError`` in CPython, because we believe that a ``SyntaxWarning`` would provide most of the benefit with less risk. Change Semantics ---------------- It `was suggested `__ to change the semantics of control flow instructions in ``finally`` such that an in-flight exception takes precedence over them. In other words, a ``return``, ``break`` or ``continue`` would be permitted, and would exit the ``finally`` block, but the exception would still be raised. This was rejected for two reasons. First, it would change the semantics of working code in a way that can be hard to debug: a ``finally`` that was written with the intention of swallowing all exceptions (correctly using the documented semantics) would now allow the exception to propagate on. This may happen only in rare edge cases at runtime, and is not guaranteed to be detected in testing. Even if the code is wrong, and has an exception swallowing bug, it could be hard for users to understand why a program started raising exceptions in 3.14, while it did not in 3.13. In contrast, a ``SyntaxWarning`` is likely to be seen during testing, it would point to the precise location of the problem in the code, and it would not prevent the program from running. The second objection was about the proposed semantics. The motivation for allowing control flow statements is not that this would be useful, but rather the desire for orthogonality of features (which, as we mentioned in the introduction, is already violated in the case of ``except*`` clauses). However, the proposed semantics are complicated because they suggest that ``return``, ``break`` and ``continue`` behave as they normally do when ``finally`` executes without an in-flight exception, but turn into something like a bare ``raise`` when there is one. It is hard to claim that the features are orthogonal if the presence of one changes the semantics of the other. Appendix ======== ``return`` in ``finally`` considered harmful -------------------------------------------- Below is an abridged version of a `research report `__ by Irit Katriel, which was posted on 9 Nov 2024. It describes an investigation into usage of ``return``, ``break`` and ``continue`` in a ``finally`` clause in real world code, addressing the questions: Are people using it? How often are they using it incorrectly? How much churn would the proposed change create? 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: .. code-block:: :class: bad 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: .. code-block:: :class: bad 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. .. note:: Following the `discussion `__, I repeated the analysis on a random selection of PyPI packages (to analyze code written by *average* programmers). The sample contained in total 77,398,892 lines of code with 316 instances of ``return``/``break``/``continue`` in ``finally``. So about 4 instances per million lines of code. **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. Copyright ========= This document is placed in the public domain or under the CC0-1.0-Universal license, whichever is more permissive.