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

shorten notification setup, fix potential crashes #2029

Merged
merged 2 commits into from
Dec 24, 2023

Conversation

r10s
Copy link
Member

@r10s r10s commented Dec 23, 2023

  • improve error handling when notification registering fails
  • force dispatching to main, this could be a source of crashes as registerForNotifications() was called at least partly on a background thread from the connectivity handler
  • when looking on any example, the additional check to getNotificationSettings() is redundant

cmp #2009, #1882, #1202

@r10s r10s force-pushed the r10s/notification-error-handling branch from 513cc3a to d1ddd34 Compare December 23, 2023 18:00
@r10s r10s requested review from Simon-Laux and adbenitez December 23, 2023 18:37
@r10s r10s force-pushed the r10s/notification-error-handling branch from d1ddd34 to f92fd29 Compare December 23, 2023 21:42
- log errors
- force dispatching to main,
  this could be a source of crashes
  as `registerForNotifications()` was called
  at least partly on a background thread from the connectivity handler
- when looking at examples, the additional check to getNotificationSettings()
  is redundant
@r10s r10s force-pushed the r10s/notification-error-handling branch from f92fd29 to 02d6ce0 Compare December 23, 2023 21:47
@r10s r10s changed the title shorten notification setup shorten notification setup, fix potential crashes Dec 23, 2023
Co-authored-by: Asiel Díaz Benítez <[email protected]>
@r10s r10s merged commit 1bd80d9 into main Dec 24, 2023
1 check passed
@r10s r10s deleted the r10s/notification-error-handling branch December 24, 2023 08:17
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