During the discussion on PEP 760 (no more bare excepts) we got to the issue of return/break/continue in finally blocks, in particular the fact that they swallow an in-flight exception. This is flagged by linters, but so far the decision has not been taken to make it a SyntaxWarning/SyntaxError in Python (PEP 601 was rejected).
I did some work to evaluate the cost/benefit of such a move, by analysis of real code.
The details are in this report.
TL;DR:
these features are rare (possibly thanks to the linters)
where they are used, they are usually used incorrectly
code authors are typically receptive to changing the code, and find it easy to do
Thanks for doing this work! It seems like the return-in-finally appears on about 2 lines in a million, which certainly classes as “rare”.
Given this information, I’d tentatively agree with the proposal to work toward deprecating return-in-finally.
The one caution I’d like to give is that being in the 8000 most popular projects would, one would hope, correlate with code quality. So it might be that a lot of bad programs use this feature, and these programmers, many of whom might not be full-time Python programmers at all, might not find this change so obvious.
(My experience would say that programmers who use finally: in the first place are more advanced. But that’s not actually data.)
First off, thanks for looking into this and reaching out to possibly affected projects.
I’d like to add a small detail that I don’t think was well captured in the analysis there.
One case which was addressed with a code change would have been working as intended if not for unresolved differences in the ProactorEventLoop in asyncio (namely, not having a way to add signal handlers to the event loop, nor a default signal handler which would gracefully shutdown an event loop on windows, it’s handled by repeatedly catching and reraising KeyboardInterrupt in various places)
The intent in this case remained to suppress all non-shutdown exceptions, and if asyncio had implemented signal handling as part of the normal asyncio event loop runners in a uniform manner, no code change would have happened for this.
In the same repository, there was a false positive involving threading (explicitly in a threading.Thread subclass), as catching KeyboardInterrupt or SystemExit wouldn’t be a concern, and the intent was to silence all exceptions that could occur as any exceptions that would happen here would not be something a user of the library would be able to handle.
It seems to me that the danger here is not returning in finally, but that reasonable expectations about where certain exceptions as interrupts can be raised aren’t accurate, and that fixing this would help a lot more code than changing the nature of swallowing exceptions in finally which is rarely used.
Following up from the SymPy issue (which I assume is one of the three cases involving unmaintained/unused code) I found that at least ruff’s SIM107 rule did not pick up on the example that Irit found. Is there a different lint rule for this?
Having seen a real case of return-in-finally I definitely found it hard to intuit what the intention of the original author had been. Did they mean to swallow all exceptions or was it unintentional? Did they consider all possible kinds of exceptions when writing it? If the intention is to swallow the exceptions then it is not hard to rewrite the code so that it reads less ambiguously without using return-in-finally.
Thanks, I confirm that B012 does pick up on this case. Much like SIM107 it is in a somewhat random rule category along with many questionable rules so I wonder how many projects actually have this lint rule enabled.
Possibly. It’s also possible that less professional programmers are even more likely to use the feature incorrectly, and will therefore benefit even more from a warning or error.
I’m generally in favor of PEP 601 being resurrected, but a concern I’d want at least mentioned within the PEP involve the age old question of how do we get the warning about the future change in front of mostly the right people: The code owners / maintainers. Rather than being seen by innocent application and library users as an issue within code that they do not own/maintain.
When these occur in libraries that other programs use, it isn’t helpful for users of Python applications to see a SyntaxWarning. In actual practice though… maybe they wouldn’t in one common scenario: A side effect of packages often being compiled to pyc files at installation time (not always the default, ex: pip vs uv pip differ in --compile default IIUC) rather than dynamically compiled from .py sources upon use means the warning would sometimes only appear during that build/install phase and not during application use where users see it?
The other “surface the issue to the right people” concern is within notebooks. Do SyntaxWarning’s even show up to code authors within notebooks? I do not know. That’d be something notebook implementations could fix though.
Might we need a SyntaxDeprecationWarning to bridge this gap of showing it to the right people at the right time? What semantics would that even mean for right people at the right time?
Or am I overthinking this? Did the SyntaxWarning: "is" with a literal. Did you mean "=="? cause mass disruption from transitive deps library reports when it was added several years back?
This finally: unclear intent occurs so infrequently that I’m leaning towards “stop overthinking it Greg!”.
To me, it seems most likely that the author meant else instead of finally (possibly they weren’t aware of else, which doesn’t exist in other languages with try/catch/finally).
But I’m in agreement that there is a problem here, and making it a syntax error (after a deprecation period where it’s a warning) sounds like a fine solution.
I do agree that maybe doing the same (hopefully mostly automated) analysis on a similar amount of random GitHub repos using Python would be nice to confirm what the “average” Python coder does.
I want to note that at least JavaScript has the same behavior as Python currently – a return in a finally clause eats the exception.I betcha it’s misused there too.
Given we’re talking about a pretty rare situation according to @iritkatriel 's investigation, we shouldn’t be shy of actually pestering users with a potential bug in a package they’re using.
A common pattern in third-party libraries is to use FutureWarning when wanting to make sure that users are actually aware of deprecated APIs or constructs.
When I first read @iritkatriel’s report (thank you for writing it!) the cases that most concerned me from a “What do we do about it?” perspective were the ones in no longer updated packages: if we escalate this to a full SyntaxError, we’re necessarily inducing fatal bitrot in any unmaintained packages that use the syntax (most likely in hard-to-hit code paths) and haven’t otherwise become incompatible with modern Python implementations.
From that perspective, my favoured resolution is “SyntaxWarning in CPython, but both SyntaxWarning and SyntaxError permitted in the language specification”.
With a SyntaxWarning (emitted when producing the AST in the parser rather than when compiling the AST):
linters (including IDEs and presumably notebooks) would all trigger the warning when parsing
unmaintained but still working libraries and programs wouldn’t break (they’d just emit the new warning when being parsed)
regular users would only get the warning for uncached source files and not be hassled after that
other implementations could still avoid the implementation complexity needed to support this (the original MicroPython concern in PEP 601)
Deciding on if it can become an error in CPython is something to revisit many years down the line based on the state of the world. I currently see no reason to pre-plan a specific release for it to become an error on.
Thanks for sharing this investigation! I had code in my local ecosystem blow up because of this missing stair, and it’s great to have a well-written document to point to when it happens again.
