Skip to content

Fix typing errors in console.py#2845

Open
Tridwoxi wants to merge 3 commits into
beeware:mainfrom
Tridwoxi:console-typing
Open

Fix typing errors in console.py#2845
Tridwoxi wants to merge 3 commits into
beeware:mainfrom
Tridwoxi:console-typing

Conversation

@Tridwoxi
Copy link
Copy Markdown
Contributor

src/briefcase/console.py contained a few type errors according to ty. These have been fixed.

Behaviour changes:

  • If Console.close is called more than once on the same instance, the method will do nothing instead of throwing an AttributeError.
  • If the exception being captured by Console.capture_stacktrace has attribute skip_logfile yet is not a BriefcaseError, skip_logfile will be ignored. An alternative implementation more amiable to duck typing would be a hasattr + isinstance check.
  • If Console.capture_stacktrace is called before an exception has been thrown, it will do nothing instead of throwing an AttributeError (due to implementation details of rich v15.0.0).
  • If sys.__stdout__ is None then Console.is_interactive will return False instead of throwing a AttributeError.

I don't think these branches are taken under normal execution, so there should be no runtime impact. Corresponding tests have been added.

PR Checklist:

  • I will abide by the BeeWare Code of Conduct
  • I have read and have followed the CONTRIBUTING.md file
  • This PR was generated or assisted using an AI tool

Assisted-by: Claude Opus 4.7

Copy link
Copy Markdown
Member

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

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

Thanks for this PR; these all makes sense, except for one flagged inline.

Comment thread src/briefcase/console.py Outdated
self.skip_log = exc_info[1].skip_logfile
except AttributeError:
pass
_, exception, _ = sys.exc_info()
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.

Is there a typing-specific reason for throwing away the exception type and traceback here, and then re-constructing those a couple of lines later for the Traceback construction?

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 3.10 docs for sys.exc_info() specify:

If no exception is being handled anywhere on the stack, a tuple containing three None values is returned. Otherwise, the values returned are (type, value, traceback).

In the type system, this shows up as tuple[type[BaseException], BaseException, TracebackType] | tuple[None, None, None].

This means once we know the exception value is not None, the exception type is provably not None. Equally correct and more performant:

        exc_type, exc_value, exc_trace = sys.exc_info()
        if exc_value is None:
            return

        if isinstance(exc_value, BriefcaseError):
            self.skip_log = exc_value.skip_logfile

        trace = Traceback.extract(
            exc_type,  # ty:ignore[invalid-argument-type]
            exc_value,
            exc_trace,
            show_locals=True,
        )
        self.stacktraces.append((label, trace))

I'll write this in a moment.

@Tridwoxi Tridwoxi requested a review from freakboy3742 May 20, 2026 03:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants