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

Fix sporadic NoMatches error reported from Header on app shutdown #4817

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

slawek-es
Copy link

@slawek-es slawek-es commented Jul 29, 2024

During application shutdown, particularly when running via App.run_test(), it seems that the Header's watcher callback on title or sub_title is called while the Header's children, such as the HeaderTitle, are already unmounted. This results in a NoMatches error being raised.

We fix this by ignoring the NoMatches error, like we did with NoScreen in the recent "hardening" commit 604b04d.

Please review the following checklist.

  • Docstrings on all new or modified functions / classes
  • Updated documentation
  • Updated CHANGELOG.md (where appropriate)

During application shutdown, particularly when running via
`App.run_test()`, it seems that the Header's watcher callback on
`title` or `sub_title` is called while the Header's children, such as
the `HeaderTitle`, are already unmounted. This results in a
`NoMatches` error being reported.

We fix this by ignoring the `NoMatches` error, like we did with
`NoScreen` in the recent "hardedning" commit 604b04d.
@mzebrak
Copy link

mzebrak commented Jul 29, 2024

I tried to follow the issue of NoMatches raised in the Header widget because we have a similar workaround for this problem in our application and I feel responsible for writing this and questioning the changes proposed here.

I may be wrong, but I think this MR is more like a workaround and will hide more significant problems with --probably-- internal task handling?

Please see: #4268 (reply in thread) which reproduced this NoMatches error.

Later it resulted in #4305 which I tested and I was still able to reproduce NoMatches in the header including these changes.

But such a NoMatches try-catch will hide that error forever when there might be something else (bad) going on. In that case, most developers will probably need to also workaround similar issues with try-catch also, which is not so promising?

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