Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Catch all exceptions at top level for better UX and bug reporting. #2203

Closed
Athanasius opened this issue Apr 14, 2024 · 0 comments · Fixed by #2211
Closed

Catch all exceptions at top level for better UX and bug reporting. #2203

Athanasius opened this issue Apr 14, 2024 · 0 comments · Fixed by #2211
Assignees
Labels
enhancement Staged Feature Complete and in Testing for Next Release
Milestone

Comments

@Athanasius
Copy link
Contributor

Given issues like #2199 , where failure to catch all relevant exceptions in the killswitch code can lead to the program crashing on startup, looking to the user like it's not working at all, it would be an improvement to User Experience (UX), not to mention gathering bug reports, to have a top level try/except to catch all unhandled exceptions.

  1. There's already a try/except around the tkinter root.mainloop() call, in order to catch KeyboardInterrupt and attempt a clean shutdown. This is where a catch-all except would go.
  2. The body of this accept should:
    1. Log the exception, so there's some useful information to go on about the cause/source.
    2. Pop up a blocking (the program is in a BAD state and going DOWN, and you're outside of the tkinter mainloop now anyway), alert to the user about having encountered a fatal bug, and to please go make a bug report. Given being outside the tkinter mainloop you're going to have to create a wholly new top-level window for this. Something basic with the message and a single 'OK' button to close it.
    3. Close the program with no attempt to clean up anything. There's no telling what bad state the program might be in.
  3. Importantly such a general exception catch will not stop any core or plugin code from catching and handling its own exceptions. It's just the place of last resort to handle any errors.

There are some choices for the exact nature of this general exception catch:

  1. except: - completely bare, which catches literally every exception, including KeyboardInterrup, SystemExit and some others.
  2. except Exception as e: - Will not catch those "very top level, system" exceptions, and is likely the behaviour you want. Also means you can do something with e if needs be (but remember you can just logger.exception(...), no need to reference it).
  3. except BaseException as e: - will likely give much the same behaviour as with Exception.

To hammer it home, the point of this is:

  1. Better User Experience - not having the program just disappear or look like it never started.
  2. Encourage users who experience such a crash to make a bug report so the culprit code can be improved.
@Rixxan Rixxan added this to the 6.0.0 milestone Apr 20, 2024
@Rixxan Rixxan modified the milestones: 6.0.0, 5.11.0 Apr 26, 2024
@Rixxan Rixxan linked a pull request Apr 26, 2024 that will close this issue
@Rixxan Rixxan self-assigned this May 6, 2024
@Rixxan Rixxan added the Staged Feature Complete and in Testing for Next Release label May 14, 2024
@Rixxan Rixxan closed this as completed Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Staged Feature Complete and in Testing for Next Release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants