Speculative Fix for WorkManager not Initialized Crash

[nan-li]

Description

One Line Summary

This is a speculative fix for the non-reproducible WorkManager is not Initialized Crash we have been seeing many reports of.

Details

Motivation

Please see issue this aims to address: #1672.

We don't completely understand this crash, so this fix is speculative and based off a response from Google Issue Tracker, though the responder may not completely understand how OneSignal is used in applications.

Scope

• Before using WorkManager, we will check for its existence. Else, in rare cases that were crashing, initialize it ourselves.
• We use a method to check if WorkManager is initialized in the process.
  • This is effectively the WorkManager.isInitialized() public method introduced in androidx.work:work-*:2.8.0-alpha02.
  • Please see https://android-review.googlesource.com/c/platform/frameworks/support/+/1941186 for the library's implementation

⚠️ Concerns ⚠️

We take over and initialize WorkManager if we see it is nil, which so far is rare.

Perhaps this crash happens because the default WorkManager initializer hasn't run yet, but is about to... In which case, right after we initialize WorkManager, the default initializer will trigger a WorkManager is already initialized exception. But, maybe that's not what is happening and this secondary crash doesn't happen.

Before accessing and initializing WorkManager ourselves, we check if it's already initialized using the same logic that the work library provides in a 2.8.0-alpha02 release. However, could we have a race condition such that when we check, it is nil, but momentarily after, it becomes initialized, while we go and try to initialize it? This could create issues that didn't exist before, but I don't think so as the Android library contains synchronization and we do too.

Testing

Unit testing

No unit test was added as it is hard to unit test this.

• we would have to test double Android library components
• androidx.work.testing package didn't have anything for our use case either

Manual testing

Limited ability to test manually.

• Tested normal app flow in an emulator

Affected code checklist

• Notifications
  • Display
  • Open
  • Push Processing
  • Confirm Deliveries
• Outcomes
• Sessions
• In-App Messaging
• REST API requests
• Public API changes

Checklist

Overview

• I have filled out all REQUIRED sections above
• PR does one thing
• Any Public API changes are explained in the PR details and conform to existing APIs

Testing

• I have included test coverage for these changes, or explained why they are not needed
• All automated tests pass, or I explained why that is not possible
• I have personally tested this on my device, or explained why that is not possible

Final pass

• Code is as readable as possible.
• I have reviewed this PR myself, ensuring it meets each checklist item

---

This change is 

[jkasten2]

I think this PR is safe to put into main and a normal release.

This won't introduce any new crashes, the concern of "WorkManager is already initialized" wouldn't happen with the default work manager initialization. This is because they use a Provider which always runs before Application.onCreate per Android's docs and what we have observed in our own testing. However, we have seen a report of one expectation to this where it can be completely skipped and never runs the Provider. This PR should fix this, and hopefully is this is the same root issue for the other crash reports.

If the app is initializing WorkManager themselves there is a very small possibility "WorkManager is already initialized" could happen instead of "WorkManager not Initialized". However this could only happen if there was a pre-existing race condition with OneSignal and the app's custom initializer anyway.

[nan-li]

Consider if we will override an app's custom initializer if there is one.

[jkasten2]

Consider if we will override an app's custom initializer if there is one.

I believe our isInitialized() would detect that, then we would skip WorkManager.initialize so we won't overwrite theirs.

[nan-li]

I was thinking about WorkManager.getInstance(context) (this code here). This method uses the same logic as our isInitialized() check and if null and there is a custom initializer, it would create WorkManager with that configuration.

But we would sort of intercept that early and create it ourselves.

[jkasten2]

True, but if they didn't run their custom initializer soon enough today they would be seeing "WorkManager not Initialized" being thrown by the OneSIgnal SDK today.

I think this is a compatibility issue we should try to improve on in it's own PR, but we need to come up with a strategy and / or an API first.

[Martin-Gonzalez90]

Any ETA for this merge?
Can be please be merged in a 4.x release as well, so we don't have to jump to version 5 🙏

[jennantilla]

Interesting potential for conflict with a custom initializer not running soon enough. Very interested in observing future strategizing on this compatibility issue.