Restart desktop process on systray errors #1835
Merged
+1,062
−18
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
RebeccaMahany
force-pushed
the
becca/systray-errors
branch
from
07b7f94
to
741af25
Compare
RebeccaMahany
force-pushed
the
becca/systray-errors
branch
from
2fc7a31
to
ea10836
Compare
zackattack01
reviewed
Sep 3, 2024
This was referenced Sep 10, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #1717.
Explanation of underlying cause
If we fail to init the instance,
initializedwill never get set, which means isReady will always return false. That means any time we attempt to perform an action on the systray in the future, like setting an icon, we'll always get the "tray not ready" errors.Evaluation of options for solving the issue
registerSystray()menumenuIt would be cleanest to have systray manage its own errors. However, we do not want to take any action while we're in modern standby, and systray doesn't currently have knowledge of the
InModernStandbyflag. I don't think it's worthwhile to build in access to the flag.I don't think it's practical for the desktop process to handle fixing the issue instead. It doesn't have knowledge of
InModernStandbyand it doesn't have access to systray state: it can't see the systray logs, and systray doesn't return any errors when menu actions fail.On the runner side, we already process desktop logs; we could monitor these for systray errors, and take action once we reach a threshold of "tray not ready" errors when not in modern standby. We could easily restart the entire desktop process, or we could build a new endpoint to ask the desktop process to shut down and restart the
menuonly.Implementation
The third option (handling in the runner, rather than in systray or desktop) is the approach I've taken.
I've also chosen the bigger and simpler hammer of restarting the entire desktop process, rather than implementing the ability to restart only the menu portion of the process.
Testing
It's proving difficult to trigger this error condition, so I've feature-flagged these changes so that we can enable them only for internal test devices.