Refactor error handling to use exceptions #1719
Conversation
cibuildwheel has up until now handled most errors by printing an error message to sys.stderr and calling sys.exit. Others were handled using Logger.error, depending on the context. We also had return codes, but these weren't explicitly defined anywhere. This makes that convention more explicit and codified. Now to halt the program, the correct thing to do is to throw a cibuildwheel.errors.FatalError exception - that is caught in main() and printed before exiting. The existing behaviour was kept - if an error occurs within a build step (probably something to do with the build itself), the Logger.error() method is used. Outside of a build step (e.g. a misconfiguration), the behaviour is still to print 'cibuildwheel: <message>' I also took the opportunity to add a debugging option `--debug-traceback` (and `CIBW_DEBUG_TRACEBACK`), which you can enable to see a full traceback on errors. (I've deactivated the flake8-errmsg lint rule, as it was throwing loads of errors and these error messages aren't generally seen in a traceback context)
Co-authored-by: Henry Schreiner <HenrySchreinerIII@gmail.com>
cibuildwheel/errors.py
Outdated
| @@ -0,0 +1,20 @@ | |||
| class FatalError(SystemExit): | |||
There was a problem hiding this comment.
Btw, I couldn't decide whether to subclass SystemExit or Exception here, any strong opinions?
There was a problem hiding this comment.
On reflection, I'll change it to Exception to avoid the confusion around SystemExit's code attribute.
| class FatalError(SystemExit): | |
| class FatalError(Exception): |
There was a problem hiding this comment.
If it needs to behave like SystemExit, I'd inherit from BaseException. Have you considered this?
There was a problem hiding this comment.
(since it's raised in places where sys.exit() used to be, I'd expect this, I think)
|
A review here would be good too. It's not critical, maybe just a little code quality improvement. I'm not too attached to it though, we could drop the idea if the existing convention approach is preferred. |
|
Let's release after this. I've been trying out macOS free threading and |
cibuildwheel has up until now handled most errors by printing an error message to sys.stderr and calling sys.exit. Others were handled using Logger.error, depending on the context. We also had return codes, but these weren't explicitly defined anywhere.
This makes that convention more explicit and codified. Now to halt the program, the correct thing to do is to throw a cibuildwheel.errors.FatalError exception - that is caught in main() and printed before exiting. The existing behaviour was kept - if an error occurs within a build step (probably something to do with the build itself), the Logger.error() method is used. Outside of a build step (e.g. a misconfiguration), the behaviour is still to print 'cibuildwheel: '
I also took the opportunity to add a debugging option
--debug-traceback(andCIBW_DEBUG_TRACEBACK), which you can enable to see a full traceback on errors.(I've deactivated the flake8-errmsg lint rule, as it was throwing loads of errors and these error messages aren't generally seen in a traceback context)