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

[flutter_local_notifications] fixed an issue with null schedule mode for older notifications scheduled using periodicallyShow on Android #2076

Merged
merged 2 commits into from
Aug 24, 2023

Conversation

MaikuB
Copy link
Owner

@MaikuB MaikuB commented Aug 22, 2023

Relates to #2033

@nohli
Copy link
Contributor

nohli commented Aug 22, 2023

Thanks for the fix!

I don't know how to test it properly though?
It gladly only affects a few users.

@MaikuB
Copy link
Owner Author

MaikuB commented Aug 22, 2023

When you say you don't know how to fix, by that do you mean you don't know how to reproduce these issue or that you don't know how to update your app to point to the branch referenced by this PR?

@nohli
Copy link
Contributor

nohli commented Aug 22, 2023

Don't know how to reproduce the issue, as it happens for <0.001% of users (with daily notifications).

@MaikuB
Copy link
Owner Author

MaikuB commented Aug 22, 2023

Based on the trace you shared before, it indicates to me that you have an app that has used version 1.x of this plugin and calls the periodicallyShow() method as the androidAllowWhileIdle parameter was added in 2.0. The issue happens as the changes done as part of #1881 didn't seem to account for this and is similar to the #2033 but for a different method provided by the plugin. If so, you could schedule a repeating notification on an older version of the app then update to a new version that references the fix in this PR

@nohli
Copy link
Contributor

nohli commented Aug 22, 2023

Oh, interesting. You mean, the users who experience this crash use the app for about 3 years, where it used plugin version <2? That would explain why the exception is so rare.

It's a bit hard to roll back the system to a state of 3 years ago, isn't it? I can try to downgrade Flutter etc., checkout an old commit, install the app, and then revert everything, install again and see if the crash happens.

But the app calls cancelAll() on a fresh app start and reschedules all notifications. Could it still cause this error?

On a side note, one user reported that he can't cancel a daily notification. His workaround is to disable notifications from the app's channelId in system settings. Could it be that it's him having the exceptions, and for some reason the plugin can't cancel very old notifications, which then cause this exception?
The app regularly calls cancelAll() and reschedules all notifications. Users can see their notifications (which get rescheduled after cancelling), and he reported the notification doesn't show up in the list.

@MaikuB
Copy link
Owner Author

MaikuB commented Aug 23, 2023

It's a bit hard to roll back the system to a state of 3 years ago, isn't it? I can try to downgrade Flutter etc., checkout an old commit, install the app, and then revert everything, install again and see if the crash happens

Yeah it could be quite hard to roll back. May have to simulate by modifying the code so that the JSON representing the notification saved to shared preferences is same as what would get saved from version 1.x

But the app calls cancelAll() on a fresh app start and reschedules all notifications. Could it still cause this error?

It should but perhaps there are users who haven't started another session?

On a side note, one user reported that he can't cancel a daily notification. His workaround is to disable notifications from the app's channelId in system settings. Could it be that it's him having the exceptions, and for some reason the plugin can't cancel very old notifications, which then cause this exception?

I'm doubtful as I've seen a correlation between a channel and cancellation. Should be something that can be verified easily though

@MaikuB MaikuB marked this pull request as ready for review August 24, 2023 09:24
@MaikuB MaikuB merged commit 02e3441 into master Aug 24, 2023
@MaikuB MaikuB deleted the fix-null-schedule-mode-repeat branch August 24, 2023 09:24
MaikuB added a commit that referenced this pull request Aug 24, 2023
MaikuB added a commit that referenced this pull request Aug 24, 2023
@MaikuB
Copy link
Owner Author

MaikuB commented Aug 24, 2023

@nohli FYI this has been released as part of 16.0.0-dev.2, 15.1.1 and 14.1.3

@nohli
Copy link
Contributor

nohli commented Aug 24, 2023

@nohli FYI this has been released as part of 16.0.0-dev.2, 15.1.1 and 14.1.3

Wow thank you so much!! ❤️

@nohli
Copy link
Contributor

nohli commented Aug 24, 2023

15.1.1 is not yet available on pub.dev.

@MaikuB
Copy link
Owner Author

MaikuB commented Aug 24, 2023

Hmm thought I published it. Perhaps I forgot to hit the button. Hang on

@MaikuB
Copy link
Owner Author

MaikuB commented Aug 24, 2023

Ok done now. Got mixed up with when I pushed out 14.1.3+1

@nohli
Copy link
Contributor

nohli commented Aug 24, 2023

Your support is really amazing! ❤️

Still can't see 15.1.1 nor 14.1.3 though 🙈

Screenshot 2023-08-25 at 00 48 58

@MaikuB
Copy link
Owner Author

MaikuB commented Aug 24, 2023

I forgot to bump the version in pubspec.yaml file that I missed publishing failed. It's done but you have to go to versions tab to see 14.1.3

image

@nohli
Copy link
Contributor

nohli commented Aug 25, 2023

Great, 15.1.1 is live, thank you again 😊

@nohli
Copy link
Contributor

nohli commented Aug 30, 2023

FYI no exceptions with the latest version 🙏🏽

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