Skip to content

Include traceback when reporting import errors#4972

Open
happz wants to merge 1 commit into
mainfrom
report-import-errors-with-tracebacks
Open

Include traceback when reporting import errors#4972
happz wants to merge 1 commit into
mainfrom
report-import-errors-with-tracebacks

Conversation

@happz

@happz happz commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

"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

  • implement the feature
  • write the documentation
  • extend the test coverage
  • update the specification
  • adjust plugin docstring
  • modify the json schema
  • mention the version
  • include a release note

"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.
@happz happz added this to planning Jun 8, 2026
@happz happz added the code | logging Changes related to debug logging label Jun 8, 2026
@github-project-automation github-project-automation Bot moved this to backlog in planning Jun 8, 2026
@happz happz moved this from backlog to implement in planning Jun 8, 2026
@happz happz added the ci | full test Pull request is ready for the full test execution label Jun 8, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread tmt/__main__.py
Comment thread tmt/__main__.py
Comment thread tmt/__main__.py
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"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

__version__ = importlib.metadata.version(__name__)

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread tmt/__main__.py
"""

try:
import tmt.utils # noqa: F401,I001,RUF100

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about moving the special handling of tmt.utils more explicitly here and split these into 2 try-imports

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about this one. I'll make the ImportError & message change from above first, then we'll see.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci | full test Pull request is ready for the full test execution code | logging Changes related to debug logging

Projects

Status: implement

Development

Successfully merging this pull request may close these issues.

2 participants