Terrible Horrible No Good Very Bad Python
39 points by hwayne
39 points by hwayne
The problem here seems to be the return False
What is the code even trying to do?
If you swallow your errors silently, they will be swallowed silently …
i.e. it says that this example came up in real life, but it would be more illuminating to see the real example, rather than the made-up function foo()
Yeah. I’d be curious why they wrote os._exit
rather than sys.exit
. The docs there are explicit about finally
clauses.
raise SystemExit(rc)
I don’t know why, but I prefer this one the most. It correctly expresses the intent of the function API where everything after is unreachable.
It’s probably best to sys.exit()
even though that code just raises a SystemExit
exception.
This is what most Python programmers will expect to see for a a soft-exit.
Definitely avoid os._exit
unless there is no other option. (In fact, sys.exit
is probably only useful in extraordinary circumstances. Otherwise, raise a normal exception.)
Other than sys.exit
(use very sparingly,) os._exit
(try to never use,) there’s also…
from os import getpid, kill
from signal import SIGKILL
kill(getpid(), SIGKILL)
Perhaps use this never.
sys.exit
is probably only useful in extraordinary circumstances. Otherwise, raise a normal exception
When writing CLI utils or scripts, I use sys.exit
in what I would consider some “ordinary” circumstances. For example, an exception handler that prints a nice message to stderr when something’s not right and returns a nonzero exit code. This makes for better shell ergonomics.
The last snippet is the code equivalent of shooting yourself in the back of your head, lol
I think if we restated the example like this, it would appear much less interesting:
from os import _exit
def f():
try:
_ecksit()
finally:
return
print(f'{f() = }')
Obviously we would expect if the code in the try
block were not able to evaluated successfully, then a corresponding except
and finally
blocks will run. I’m not sure how interesting this is.
The code as written below operates as you would expect.
from os import _exit as os_exit, EX_OK
from sys import exit as sys_exit
def f():
try:
sys_exit() # raises SystemExit
except: # catches everything, including SystemExit; this is why bare `except` is a bad idea
pass
try:
os_exit(EX_OK)
finally:
return
print(f'{f() = }')
This code also illustrates an important distinction between sys.exit
(which we should use as needed) and os.exit_
which we should be very careful about using.
sys.exit
just raise
s a SystemExit
exception which (in the absence of the very ill-advised bare except
) will percolate up to the top-level exception handler which will handle shutdown tasks (like __exit__
any active context managers)os._exit
is implemented OS-specific: On POSIX, it calls _exit
(2): Modules/posixmodule.c#L67 and (glibc) sysdeps/unix/sysv/linux/_exit.c
There are some other minor mistakes in the post’s description.
one might be mislead that import os comes after the function is defined. but python has dynamic scoping, so that’s fine.
CPython scoping is actually statically determined at bytecode generation time, which is why we need the global
and nonlocal
keywords. They are hints at the parser to correctly generate the appropriate LOAD_GLOBAL
/STORE_GLOBAL
or LOAD_DEREF
/STORE_DEREF
bytecodes.
I was going to complain how contrived this example is but then the article says “yes someone did write this code by accident”. Oof.
There are so many languages that cannot be read 100% by a human leading to surprising wat moments. Some languages try to wrangle complexity with types. Some opt for conciseness. In Zen of Python, explicit is better than implicit because then you don’t get surprises? Or maybe you limit surprises. But if you were really explicit, you’d end up with a different language. Assembly. Zig where I can define an allocator.
First viewpoint: code should be readable. I might need to change this code or debug it. What I write should be readable even immediately after I write it.
Second viewpoint: I’m not a computer. I can try to execute code in my head but I’m not very good at it. So I need to run it or observe it working at least once, aka a test. I can automate this test if I want to. If I look at it like this, as a black box, then in this viewpoint, it doesn’t matter what it reads like. I am focused on what it seems to do instead of what I hope it does. But then I’m back to the first view. I don’t want to have to simulate/attempt my way to working, readable code.
I’ve been in situations where I have tests and yet there is undocumented behavior or something happening which cannot be easily fixed and these moments are frustrating. Many times, I debug-pull on the mystery-thread and find something piddly like this. Then I just log something in my dev log with WTF near it so I can laugh at it later. This is not unlike OP sharing a rabbit hole story on a blog.
The python puzzle is exactly the kind of “wat” moment that illustrates my point. You can’t reliably read this without pretending to be Python and recalling in the moment how os._exit()
bypasses normal cleanup. It’s no wonder that many languages have a goal of readability and include a testing library. It’s no wonder that there are sweng tools and perspectives beyond code/testing.
This particular bug would have been caught by static typing. That means we’re really dealing with a question of how does Python handle programming errors, not how does it handle conflicting exit vs. finally commands.
That bug yes but it’s hindsight. This bug is caught by a squiggle or a cli command:
# this bug is caught assuming a CI check or enforcement
# Argument of type "Literal['0']" cannot be assigned to parameter "status" of type "int"
os._exit("0")
# this one is not, the type is correct the result is not correct
os._exit(404)
This bug is not caught by type checks. The exit value is not 404.
This is what I meant by:
So I need to run it or observe it working at least once, aka a test.
It’s not either or types or tests but ultimately I need to execute it.
The problem is much more apparent with static typing, most confusing part is people assuming there isn’t a type error.
I am confused as to what the problem is with the return statement in the finally block. The whole code is not sensible but at least that bit does what it looks like it should do. The author chose to disregard the exception and return anyway.
It hides the type error, making it hard to work out why the program failed to exit when expected.