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

IllegalArgumentException Thrown from WrapperWorkItem.complete #644

Closed
jkasten2 opened this issue Oct 9, 2018 · 4 comments · Fixed by #730
Closed

IllegalArgumentException Thrown from WrapperWorkItem.complete #644

jkasten2 opened this issue Oct 9, 2018 · 4 comments · Fixed by #730

Comments

@jkasten2
Copy link
Member

jkasten2 commented Oct 9, 2018

Description:
Reported Crashlytics crash, issue happening at ccom.onesignal.JobIntentService$JobServiceEngineImpl$WrapperWorkItem.complete(JobIntentService.java:290).

Full stack trace below.

#0. Crashed: AsyncTask #4
       at android.os.AsyncTask$3.done(AsyncTask.java:353)
       at java.util.concurrent.FutureTask.finishCompletion(FutureTask.java:383)
       at java.util.concurrent.FutureTask.setException(FutureTask.java:252)
       at java.util.concurrent.FutureTask.run(FutureTask.java:271)
       at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1162)
       at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:636)
       at java.lang.Thread.run(Thread.java:764)

--

Fatal Exception: java.lang.RuntimeException: An error occurred while executing doInBackground()
       at android.os.AsyncTask$3.done(AsyncTask.java:353)
       at java.util.concurrent.FutureTask.finishCompletion(FutureTask.java:383)
       at java.util.concurrent.FutureTask.setException(FutureTask.java:252)
       at java.util.concurrent.FutureTask.run(FutureTask.java:271)
       at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1162)
       at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:636)
       at java.lang.Thread.run(Thread.java:764)

Caused by java.lang.IllegalArgumentException: Given work is not active: JobWorkItem{id=4 intent=Intent { (has extras) } dcount=1}
       at android.app.job.JobParameters.completeWork(JobParameters.java:233)
       at com.onesignal.JobIntentService$JobServiceEngineImpl$WrapperWorkItem.complete(JobIntentService.java:290)
       at com.onesignal.JobIntentService$CommandProcessor.doInBackground(JobIntentService.java:416)
       at com.onesignal.JobIntentService$CommandProcessor.doInBackground(JobIntentService.java:405)
       at android.os.AsyncTask$2.call(AsyncTask.java:333)
       at java.util.concurrent.FutureTask.run(FutureTask.java:266)
       at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1162)
       at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:636)
       at java.lang.Thread.run(Thread.java:764)

From investigation it seems this is happening when notifications are restored after reopening the app.

Environment
Almost all devices are Android 8.x.
Happening on OneSignal SDK 3.10.1 and 3.10.2.
NotificationExtenderService was not used.

Steps to Reproduce Issue:
Attempted to reproduce but was not able to, below is what was attempted;

  1. Send 50 notifications
  2. Force Kill the app.
  3. Reopen it, see notifications are restored correctly
  4. Tried redeploying app from Android Studio as well instead of step 2 + 3.
  5. Tried with and without a big picture to a remote URL.

Mitigating Issue
While attempting to reproduce this issue I noticed that some apps could be trying to process restoring notifications that will end up being thrown away by the Android system due to some limits.

E/NotificationService: Package enqueue rate is 10.56985. Shedding events. package=####
E/NotificationService: Package has already posted 50 notifications. Not showing more.  package=####

Restoring notifications can be slowed down as well as limiting to 50. This will at least prevent less jobs and possibly lower the chance of a race condition, if it is the cause here.

jkasten2 added a commit that referenced this issue Oct 9, 2018
* Only the most recent 49 notifications will be restore instead of just limiting on the past week.
   - Ensures only the most recent relevant notifications are shown.
* Also increased delay between restoring each notification to 200ms.
* This fixes the following logcat messages
   - E/NotificationService: Package has already posted 50 notifications. Not showing more.  package=####
   - E/NotificationService: Package enqueue rate is 10.56985. Shedding events. package=####
* This can mitigate #644
jkasten2 added a commit that referenced this issue Oct 10, 2018
* Only the most recent 49 notifications will be restore instead of just limiting on the past week.
   - Ensures only the most recent relevant notifications are shown.
* Also increased delay between restoring each notification to 200ms.
* This fixes the following logcat messages
   - E/NotificationService: Package has already posted 50 notifications. Not showing more.  package=####
   - E/NotificationService: Package enqueue rate is 10.56985. Shedding events. package=####
* This can mitigate #644
@jkasten2
Copy link
Member Author

I haven't seen this bug come up in any recent builds, closing this issue.

@jkasten2
Copy link
Member Author

jkasten2 commented Apr 2, 2019

This is due to a bug in the Android Support library which is a long running issue reported on Google's issue tracker.
https://issuetracker.google.com/u/1/issues/63622293?pli=1
The best approach is to handle this expectation as some other libraries such as evernote/android-job since Google hasn't solved this issue.

It seems there is a new API, WorkManager that is probably best to migrate to at some point going forward however.

jkasten2 added a commit that referenced this issue Apr 3, 2019
@rgomezp rgomezp reopened this Apr 9, 2019
@jkasten2
Copy link
Member Author

This is now fixed in the 3.10.8 release.

@atulgpt
Copy link

atulgpt commented Sep 15, 2019

The issue is still happening it there any fix??

baptiste-nv added a commit to nventive/OneSignal-Android-SDK that referenced this issue Feb 5, 2020
* Add Update Handler for Send/Delete Tags

• Adds an update handler to methods that update tags to allow developers to respond when the HTTP request is finished (or fails)
• Adds a new public method oneSignalLog() to allow wrapper SDK's to use OneSignal logging

* Maintain Array of Send Tags Handlers

• Since users may call sendTags() multiple times, and the sendTags() requests are batched, this would have resulted in only the most recent callback/handler being called.
• Implemented an array of SendTagsHandlers to make sure that when the batched SendTags request is finished, it will execute each unique handler individually.

* Fix Tags Batching

• The SDK was not checking to make sure that a ChangeTagsUpdateHandler reference was non-null before executing it.
• Added checks to make sure null handlers aren't invoked
• Creates a copy of the sendTagsHandlers array since it gets cleared immediately afterwards

* Add requiresUserPrivacyConsent() getter

• Since developers can manually require consent by editing the manifest, the wrapper SDK's need a way of knowing if the Android SDK is waiting on the user's privacy consent
• Exposes a public getter that returns TRUE if consent is required but hasn't been provided yet.

* Add Additional Testcase

• Adds a testcase to make sure that the new requiresUserPrivacyConsent() method returns correct results

* Correct wakeful service fallback to job call

* Stylistic Change

• Adds curly braces since the for-loops are indented. Applies the change to all loops for consistency.

* 3.10.0 Release Commit

• Adds a ChangeTagsUpdateHandler to allow for callbacks when sending/deleting tags
• Publicly exposes onesignalLog() method so that wrapper SDK's can use it.
• Adds a getter method for requiresUserPrivacyConsent
• Corrects wakeful service GCM fallback

* Fix ANR

• In rare situations, the main UI thread can block on the user state synchronization making a potentially very long running HTTP request
• The user state synchronization model of the Android SDK needs to be significantly refactored & simplified
• In the mean time, this commit makes a minor change so that the user state synchronizer never blocks/holds locks while making HTTP requests

* Change User State Method Synchronization

• In the previous commit, I wanted to explicitly use a sync lock object for different methods to synchronize on, to allow more fine-grained control over what parts of methods get synchronized
• However, I mistakenly left out a method called getToSyncUserState().
• This commit removes the method synchronization and synchronizes on the syncLock, an instance property that other methods also use to synchronize on.

* Improve Android SDK Demo Project

• We have several demo projects for Android, but the default demo project has few options
• This commit adds multiple buttons to the demo to test several different aspects of the SDK without having to open a different project or deal with modules/etc.

* 3.10.1 Release Commit

• Fixes a somewhat frequent ANR (more details in 3.10.1 release notes)

* Fix invoking interface method on a null object reference

* Added test to go with PR OneSignal#611 for null callback.

* Fix FirebaseInstanceIdService Missing Exception

• It appears that in a previous commit (f614e8b) we added code to disable the FirebaseInstanceIdService if there isn't a default Firebase app
• It looks like we tried to handle the case where the FirebaseInstanceIdService class was missing, but we were only catching NoClassDefFound errors
• After some further research, it looks like we should also be catching IllegalArgumentExceptions as well since this is what the package manager instance will throw if the FirebaseInstanceIdService class is missing

* Bundle NPE in RestoreJobService.onHandleWork

* Fixes reported NPE from RestoreJobService.onHandleWork. Issue OneSignal#591
* Issue maybe related to bug in Android 6.0.1 with BaseBundle.java:127
   - This works around any possible issue here.

* Fixes disabling sound & vibration in some configs

* If OneSignal was not initialized from onCreate of the application class on Android 7 and older.
* This was due to the appContext not being set before trying to read from shared preferences.
* All entry points now set the appContext to avoid any future issue with dependencies on this.
   - Includes OneSignal BroadcastReceivers, Services, and Activities.
* Fixes OneSignal#618

* Importance fallback fix for notification channels

* If the Android Oreo notification channel has a Medium importance or lower
    we now correctly skip setting sound, led, and vibration settings for pre-Oreo devices.
* Refactored these setting into a new setAlertnessOptions method to group this logic.

* Updated internal JobIntentService to latest

* Added useWakefulService option to JobIntetService

* This allows overriding the API 26 check so when we have FCM high priority wakelock we can start a IntentService instead of using a JobService,
  - A JobService won't start when doze is active so the job would delayed otherwise.
* Implementing the flag will be in the next commit, split this out so we can see what was changed easier if we need to apply any upstream changes from Google.

* Added new high priority flag to JobIntentService

* Added result to GCM broadcast aborts

* Adding a result prevents the GMS  app from putting a confusing CANCELLED entry into the logcat.

* Updated test for GCM receiver result

* 3.10.2 Release Commit

* Misuse of multiple SQL statements in execSQL().

* -Modified Random generation of values to SecureRandom

* -Modified Random generation of values to SecureRandom

* Limit number of notification restored to 49

* Only the most recent 49 notifications will be restore instead of just limiting on the past week.
   - Ensures only the most recent relevant notifications are shown.
* Also increased delay between restoring each notification to 200ms.
* This fixes the following logcat messages
   - E/NotificationService: Package has already posted 50 notifications. Not showing more.  package=####
   - E/NotificationService: Package enqueue rate is 10.56985. Shedding events. package=####
* This can mitigate OneSignal#644

* Allow setting provideUserConsent before init

* This is safe as OneSignalPrefs will cache the value if the context is not set yet.

* Remove Privacy Consent Requirement to Add Observers

• Apps should be able to add observers (ie. addSubscriptionObserver) even if privacy consent has not yet been granted.

* Add Regression Tests

• Adds tests to make sure that apps can add the observers even if privacy consent hasn't been granted
• The tests verify that the observers fire correctly once consent is granted.

* Handle JobIntentService IllegalArgumentException

* This handles the IllegalArgumentException which can throw on Oreo+
  when a high priority notification is sent and when a NotificationExtenderService is setup.
  - Only thrown in the case where FCM could not add the app to the whitelist.

* Permission check added to job service setPersisted

* Added boot permission check to setPersisted(true) which could throw on some devices.

* Possible fix for "Package manager has died"

* Lowered parcel size from package manager which means less chance of requesting over the limit.
* Package manager may have already die so it won't fix these cases.

* 3.10.3 Release commit

* Add External ID

• Adds public methods setExternalUserId() and removeExternalUserId() that let developers map their own userID's in our system
• TODO: Add tests to ensure the methods generate correct API requests and that it uses the on_session request if it hasn't already occurred

* Add Tests

• Adds tests to make sure that setExternalUserId requests are correctly formatted and will wait for registration request if needed
• Fixes the setExternalUserId() request to use a PUT HTTP request if registration has already occurred

* Use State Synchronizer

• It turns out the Android SDK already has a state synchronizer that can persist previously set player values and will not send duplicate requests if the value is unchanged.
• This commit switches to utilizing then state synchronizer
• Adds a new test to make sure that the external ID is not sent if setExternalUserId() is called but the ID is the same as the previous session/s

* Add Test

• Adds a test to verify that the externalUserId is set on both Email and Push player accounts

* Removed ACTION_BOOT_COMPLETED for BOOT_COMPLETED

* ACTION_BOOT_COMPLETED is invalid, BOOT_COMPLETED is the correct intent action

* Catch for SecurityException from JobIntentService

* Thrown in rare cases due to know Android issue
   - https://issuetracker.google.com/issues/63622293
* Fixes issue OneSignal#673

* 3.10.5 Release Commit

• Includes miscellaneous bug fixes and improvements

* Add External ID Methods to Unity Proxy

• Adds the set & remove external ID methods to the unity proxy

* Making setAppContext public

* Wont't be commonly used, however it is valid if you need to call provideUserConsent before OneSignal.init

* • This commit changes the SDK so that it maintains an array of getTags handlers instead of only retaining a reference to the most recent handler.
• So in the past, if getTags() was called multiple times, only the handler for the most recent call would get executed.
• Makes a small change to the example project to fix a crash when GDPR consent was not granted

* Add Test

• Adds a test to ensure that multiple handlers for getTags() are retained and executed independently

* Update badge count when restoring notifications

* Also omit trying to restore notifications if notifications are disabled.
* Misc code cleanup around this

* Badge now uses notification count from system

* Badge count is now always set to based on the notifications in the shade, regardless if they are from OneSignal or not
* Fixing issue where new notifications would not show if there are already 49 notifications in the shade
   - Now handles removing oldest notifications to make room for new ones.
* Clearing notifications now works even if the privacy was not accepted.
* Fixed issue where badges did not clear if clear notifications was called before init.
* Badge counts are now updated after restoring notifications.

* 3.10.6 Release commit

* Rebase on 3.10.6

* REenable location tracking

Co-authored-by: Brad Hesse <nightsd01@gmail.com>
Co-authored-by: Josh Kasten <jkasten@gmail.com>
Co-authored-by: Tran-Loc Le <letranloc94@gmail.com>
Co-authored-by: csnagy <csaba.nagy@usi.ch>
Co-authored-by: jramosMobilendo <39481972+jramosMobilendo@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants