3 ms·
Just noting it here: your code is incorrect. In case of a KeyboardInterrupt error and another error raised by `log_tons_of_debug_info()` (there's no error free
by 1st1 2y ago
Just noting it here: your code is incorrect. In case of a KeyboardInterrupt error and another error raised by `log_tons_of_debug_info()` (there's no error free code, right?), KeyboardInterrupt would end up being masked (it would go into the __context__ attribute of another error). The program won't abort its execution. And it's just one example out of many where it's critical to not mask error types.
Correct code would be:
try:
something()
except BaseException as ex:
try:
log_tons_of_debug_info()
finally:
raise ex
But really, you don't want to mess with BaseExceptions at all, so just do `except Exception` instead of a bare `except:`.
- theamk 2y agoWhy wouldn't I want to mess with BaseExceptions? They are not magic, and add only 3 classes to the list: SystemExit - You _definitely_ want to catch this one for logging. If a library (not top-level app) calls `sys.exit` you at least want to know what's happening, if anything so you can talk to author and get them to use proper exception types. KeyboardInterrupt - I normally want to catch this one as well. If the program was taking too long and I hit Ctrl-C to stop it, I _do_ want to see all the debug output. And if I don't, for some reason, there is always Ctrl-\ which kills python immediately and unconditionally. GeneratorExit - this one is tricky and I agree that in a lot of cases, you don't want to print logs on it. But it also very rare - it only appears in async functions (emitted by yield), and never propagated to caller. So as long as you are not doing async, you can simply ignore it, which covers majority of the the code I write.
- 1st1 2y agoBecause accidentally masking some BaseExceptions like `asyncio.CancelledError` can lead to things like memory/resource leaks and potentially your production app going down in pretty hard to debug ways.
- theamk 2y agowell, yeah, you don't want to mask any of those. Thats why all my examples talk about logging + re-raising.
- quietbritishjim 2y ago> Just noting it here: your code is incorrect. In case of a KeyboardInterrupt error and another error raised by `log_tons_of_debug_info()` (there's no error free code, right?), KeyboardInterrupt would end up being masked (it would go into the __context__ attribute of another error). Your snippet has the opposite problem: if ex is a regular Exception, but then KeyboardInterrupt is raised by `log_tons_of_debug_info()`, then your snippet would mask that by re-raising ex in its place. I think the original snippet is probably better on balance, even ignoring difference in code complexity.