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: NotificationChannelGroup crash (WPB-6233) #2691

Merged

Conversation

AndroidBob
Copy link
Collaborator

@AndroidBob AndroidBob commented Feb 13, 2024

BugWPB-6233 [Android] Notification playstore crash with createNotificationChannel

Cherry pick from the original PR:


⚠️ Conflicts during cherry-pick:
kalium

What's new in this PR?

Issues

crash:
https://play.google.com/console/u/2/developers/7098984309886892484/app/4973241010395499500/vitals/crashes/cb7ccba8c13d0a497df6edf45962bb4f/details?days=7&isUserPerceived=true

Causes (Optional)

Logs says that app is trying to create a NotifcationChannel in a NotifcationChannelGroup that doesn't exist. This looks strange as we create Group first and only after it create channels in that group.
So it's hard to reproduce the crash.

From the logs in DataDog found out that at least some part of the crashes happens after logout.
AND deleting the NotifcationChannelGroup is called from a few places (basically on logout) and launched in a separate CoroutineScope.

So there is a theory that when user logs out, gets invalid data first (list of users with logged out user) and after some time - valid data. This makes the app create notification group and channels for the user who is logged out. And somewhere during this in another CoroutineScope is called.

Solutions

Move calling into the same CoroutineScope to escape such conflicts.
For that update to not just create Groups and Channels for active users, but also remove the Groups and Channels that don't belong to any active user.

@AndroidBob AndroidBob added the cherry-pick PR is cherry-picking changes from another banch label Feb 13, 2024
Copy link

codecov bot commented Feb 13, 2024

Codecov Report

Attention: 10 lines in your changes are missing coverage. Please review.

Comparison is base (f2c5104) 43.02% compared to head (0c5ab5d) 43.01%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2691      +/-   ##
===========================================
- Coverage    43.02%   43.01%   -0.02%     
===========================================
  Files          408      408              
  Lines        13525    13527       +2     
  Branches      2393     2393              
===========================================
- Hits          5819     5818       -1     
- Misses        7021     7024       +3     
  Partials       685      685              
Files Coverage Δ
.../kotlin/com/wire/android/GlobalObserversManager.kt 88.67% <ø> (+1.64%) ⬆️
...i/home/appLock/forgot/ForgotLockScreenViewModel.kt 60.82% <ø> (-0.40%) ⬇️
...id/ui/userprofile/self/SelfUserProfileViewModel.kt 51.87% <ø> (+0.38%) ⬆️
...ndroid/notification/NotificationChannelsManager.kt 1.26% <0.00%> (-0.09%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f2c5104...0c5ab5d. Read the comment docs.

Copy link
Contributor

github-actions bot commented Feb 13, 2024

Test Results

819 tests  ±0   819 ✅ ±0   14m 12s ⏱️ + 6m 14s
114 suites ±0     0 💤 ±0 
114 files   ±0     0 ❌ ±0 

Results for commit 0c5ab5d. ± Comparison against base commit f2c5104.

♻️ This comment has been updated with latest results.

Copy link
Contributor

APKs built during tests are available here. Scroll down to Artifacts!

@AndroidBob
Copy link
Collaborator Author

Build 3072 succeeded.

The build produced the following APK's:

Copy link
Contributor

APKs built during tests are available here. Scroll down to Artifacts!

@AndroidBob
Copy link
Collaborator Author

Build 3085 failed.

Copy link
Contributor

APKs built during tests are available here. Scroll down to Artifacts!

@AndroidBob
Copy link
Collaborator Author

Build 3197 succeeded.

The build produced the following APK's:

@MohamadJaara MohamadJaara added this pull request to the merge queue Feb 19, 2024
Copy link
Contributor

APKs built during tests are available here. Scroll down to Artifacts!

@AndroidBob
Copy link
Collaborator Author

Build 3203 succeeded.

The build produced the following APK's:

Merged via the queue into develop with commit 36e10cb Feb 19, 2024
14 checks passed
@MohamadJaara MohamadJaara deleted the fix/notification_channel_group_crash-cherry-pick branch February 19, 2024 10:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick PR is cherry-picking changes from another banch size/XS
Projects
None yet
4 participants