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 Thread sync throwing exception sometimes crashing app #3634

Merged
merged 1 commit into from
Jul 7, 2023

Conversation

jpelgrom
Copy link
Member

@jpelgrom jpelgrom commented Jul 4, 2023

Summary

Thread credential syncing uses the provided CoroutineScope to run other functions that throw exceptions. An user found an issue when not connected to their home network, this will throw an exception when getting the device credentials. Depending on where the function is called, this can crash the app.

When using a normal Job, like in the mainScope that we create in a few places, a thrown exception will mean that everything in that scope is cancelled and the exception propagates up, even if 'caught'. That isn't wanted so use a SupervisorJob instead, like the default viewModelScope, when running this function to make sure that when caught nothing else stops.

Screenshots

n/a

Link to pull request in Documentation repository

n/a

Any other notes

Easiest way to test is to access Home Assistant while only connected to mobile data and initiating a Thread credentials sync from the app settings.

 - Thread credential syncing uses the provided CoroutineScope to run other functions that throw exceptions. When using a normal Job a thrown exception will mean that everything in that scope is cancelled and propagates, even if caught. That isn't wanted so use a SupervisorJob instead when running this function to make sure that when caught nothing else stops.
@JBassett JBassett merged commit e53533e into home-assistant:master Jul 7, 2023
3 checks passed
@jpelgrom jpelgrom deleted the fix-thread-sync-crash branch July 7, 2023 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants