fix: replace bare except with except BaseException in check-docker.py#10407
fix: replace bare except with except BaseException in check-docker.py#10407harshadkhetpal wants to merge 1 commit intolablup:mainfrom
Conversation
Bare `except:` clauses catch all exceptions including SystemExit and KeyboardInterrupt. Replace with `except BaseException:` since the block re-raises a new RuntimeError, preserving interrupt handling semantics while making the intent explicit.
|
|
1 similar comment
|
|
HyeockJinKim
left a comment
There was a problem hiding this comment.
Thanks for the fix! However, bare except: already behaves identically to except
BaseException: — both catch everything including KeyboardInterrupt and
SystemExit. So this change doesn't affect Ctrl+C behavior.
To actually prevent suppressing keyboard interrupts, except Exception: is the
right choice here, since Exception excludes KeyboardInterrupt, SystemExit, and
GeneratorExit.
# Suggested
except Exception:
raise RuntimeError("Failed to query Snapd package information")
# Or even better, since r.raise_for_status() only raises
# aiohttp.ClientResponseError:
except aiohttp.ClientResponseError:
raise RuntimeError("Failed to query Snapd package information")
Summary
Replace bare
except:clauses withexcept BaseException:inscripts/check-docker.py.Bare
except:catches all exceptions includingSystemExit,KeyboardInterrupt, andGeneratorExit, which is almost never intentional. Since both affected blocks immediately re-raise a newRuntimeError,except BaseException:is the correct replacement to preserve interrupt-handling semantics while making intent explicit.Changes (2 occurrences):
Testing
No behavior change for normal exception paths — style/correctness fix only. This prevents accidentally suppressing
KeyboardInterrupt/SystemExitin non-reraise scenarios.