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

WeeklyRoundupBackgroundTask - no launch handler #18162

Conversation

sla8c
Copy link
Contributor

@sla8c sla8c commented Mar 18, 2022

This PR fixes an issue where iOS was throwing the following error NSInternalInconsistencyException: No launch handler registered for task with identifier org.wordpress.bgtask.weeklyroundup

This was caused by when FeatureFlag.weeklyRoundupBGProcessingTask was enabled the launch handler identifier registered changed. However WeeklyRoundupBackgroundTasks that were pre-scheduled the week before on existing app installs would be still scheduled with the old identifier.

Fixes #18156

To test:

  1. Test on device. Disable FeatureFlag.weeklyRoundupBGProcessingTask so that it uses the old identifier
  2. When app launches go to Me -> App Settings -> Debug. Tap Weekly Roundup
  3. If you don't have enough blogs / history also slide to the right to "Include A8c P2s"
  4. Tap one of the Schedule in 10 sec options and then minimise the app in background
  5. Weekly Round up Notification should eventually appear as normal
  6. Re-enable FeatureFlag weeklyRoundupBGProcessingTask
  7. Repeat the same test steps and Weekly Roundup Notification should eventually appear as normal

Regression Notes

  1. Potential unintended areas of impact
    WeeklyRoundupNotificationTask

  2. What I did to test those areas of impact (or what existing automated tests I relied on)
    Tested on device

  3. What automated tests I added (or what prevented me from doing so)
    None

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding unit tests for my changes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@sla8c sla8c added this to the Pending milestone Mar 18, 2022
@sla8c sla8c requested review from frosty, mokagio and momo-ozawa March 18, 2022 10:35
@sla8c sla8c self-assigned this Mar 18, 2022
@wpmobilebot
Copy link
Contributor

You can test the Jetpack changes on this Pull Request by downloading it from AppCenter here with build number: pr18162-ba093de. IPA is available here. If you need access to this, you can ask a maintainer to add you.

@wpmobilebot
Copy link
Contributor

You can test the WordPress changes on this Pull Request by downloading it from AppCenter here with build number: pr18162-ba093de. IPA is available here. If you need access to this, you can ask a maintainer to add you.

Base automatically changed from release/19.4 to trunk March 18, 2022 11:01
Copy link
Contributor

@frosty frosty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me!

@mokagio mokagio changed the base branch from trunk to release/19.5 March 22, 2022 02:49
@mokagio
Copy link
Contributor

mokagio commented Mar 22, 2022

@sla8c I changed the base branch for this PR to release/19.5. It originally was release/19.4 but GitHub updated it to trunk once the 19.4 release was finalized last Friday.

I'm going to merge this now. Just a note: We should monitor the crash occurrences on 19.4 closely over the next few days. If they keep growing, we might want to cherry-pick this and ship an hotfix (19.4.1).

@mokagio mokagio merged commit 0b2a1b7 into release/19.5 Mar 22, 2022
@mokagio mokagio deleted the issue/17956-WeeklyRoundupBackgroundTask-no-launch-handler branch March 22, 2022 02:53
@sla8c
Copy link
Contributor Author

sla8c commented Mar 22, 2022

@mokagio thanks + thx for heads up!

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.

4 participants