Include traceback when reporting import errors#4972
Conversation
"tmt package does not seem to be installed" alone does not seem to be very helpful. Patch changes import error reporting to use the same code as "regular" errors. We can try and use our nice error reporting, if that's broken, we can always fall back to much less prettier, verbose, uncontrolled, but very useful tracebacks. Note that the "ugly" mode triggers when we break things in a really bad way. In a way that should have been captured by a test, a broken import deep somewhere in tmt code. It's not how tmt wishes to present itself to users, it's simply the best we can do to get useful info in such a weird state of things.
There was a problem hiding this comment.
Code Review
This pull request refactors exception handling in tmt/__main__.py by introducing a centralized report_exception helper function. In the fallback exception handler, print the original exception traceback directly using traceback.print_exception instead of traceback.print_exc() to ensure the root cause is correctly reported if the reporting tool fails to load.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| print("Error: tmt package does not seem to be installed") | ||
| raise SystemExit(1) from error | ||
| report_exception( | ||
| error, exit_code=1, message="Error: tmt package does not seem to be installed" |
There was a problem hiding this comment.
This error message was not quite helpful in the first place, the only case where we would catch "tmt package is not installed" is in
Line 7 in b1751ea
But even then, isn't that error caught before even being able to enter tmt.__main__?
My preference would be to drop the message in both cases.
There was a problem hiding this comment.
The message actually appeared in other cases, and it's really not helpful. I ran into a type being used in an annotation while being imported under TYPE_CHECKING guard only, in the recipe code, which raised an exception that resulted in the "tmt package does not seem to be installed" message emitted after fairly successful tmt lint run. The message alone was extremely unhelpful, hence the addition of traceback.
I can drop it, make no distinction between import and regular errors, and have just one except branch to handle it all.
| """ | ||
|
|
||
| try: | ||
| import tmt.utils # noqa: F401,I001,RUF100 |
There was a problem hiding this comment.
What about moving the special handling of tmt.utils more explicitly here and split these into 2 try-imports
There was a problem hiding this comment.
Not sure about this one. I'll make the ImportError & message change from above first, then we'll see.
"tmt package does not seem to be installed" alone does not seem to be very helpful. Patch changes import error reporting to use the same code as "regular" errors. We can try and use our nice error reporting, if that's broken, we can always fall back to much less prettier, verbose, uncontrolled, but very useful tracebacks.
Note that the "ugly" mode triggers when we break things in a really bad way. In a way that should have been captured by a test, a broken import deep somewhere in tmt code. It's not how tmt wishes to present itself to users, it's simply the best we can do to get useful info in such a weird state of things.
Pull Request Checklist