Skip to content

fix: add graceful error logging for BaseThread crashes#908

Open
aphrodoe wants to merge 1 commit intoSeedSigner:devfrom
aphrodoe:feature/refactor-basethread
Open

fix: add graceful error logging for BaseThread crashes#908
aphrodoe wants to merge 1 commit intoSeedSigner:devfrom
aphrodoe:feature/refactor-basethread

Conversation

@aphrodoe
Copy link
Copy Markdown

@aphrodoe aphrodoe commented Apr 9, 2026

Description

Problem or Issue being addressed

Currently, any class that inherits from BaseThread (which handles nearly all hardware listeners, camera loops, and scrolling UI text) can silently crash without yielding any errors or stack traces to the output, refer to issue #907.

Because the underlying loop encapsulates its execution in an isolated boundary, if a developer writes buggy code or an edge-case logic error occurs inside def run(self):, the thread terminates abruptly. It never halts the main UI/application event loop, but the targeted feature has zero trace output, which might make debugging harder.

Solution

By wrapping the subclass's inherited .run() method in a protective global try/except Exception: block at the instance level:

  1. Crashing threads now securely log a full stack trace utilizing logger.exception.
  2. The instance state gracefully exits by securely flagging self.keep_running = False.

Additional Information

MicroPython Portability: Since it natively preserves the threading.Thread inheritance hierarchy, it stays clean and is cross-compatible if there is MicroPython _thread port in the future.

Screenshots

image image

Tried adding an intentional bug in class HorizontalTextScrollThread, it logs the exception properly. This will help in future dev workflows, rather than making it go unnoticed.


This pull request is categorized as a:

  • New feature
  • Bug fix
  • Code refactor
  • Documentation
  • Other

Checklist

I ran pytest locally

  • All tests passed before submitting the PR
  • I couldn't run the tests
  • N/A

I included screenshots of any new or modified screens

Should be part of the PR description above.

  • Yes
  • No
  • N/A

I added or updated tests

Any new or altered functionality should be covered in a unit test. Any new or updated sequences require FlowTests.

  • Yes
  • No, I’m a fool
  • N/A

I tested this PR hands-on on the following platform(s):


I have reviewed these notes:

  • Keep your changes limited in scope.
  • If you uncover other issues or improvements along the way, ideally submit those as a separate PR.
  • The more complicated the PR, the harder it is to review, test, and merge.
  • We appreciate your efforts, but we're a small team of volunteers so PR review can be a very slow process.
  • Please only "@" mention a contributor if their input is truly needed to enable further progress.
  • I understand

Thank you! Please join our Devs' Telegram group to get more involved.

Resolves #907

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.

Missing error logging for BaseThread crashes

1 participant