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.

Sign up for GitHub

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: Investigate initializer errors #69

Merged
merged 13 commits into from
Jul 18, 2024
Merged

Fix: Investigate initializer errors #69

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
75fe055
Remove 2 provider setups and include the same setup in no-op manifest
AsimRibo Jul 1, 2024
8c0568a
Revert "Remove 2 provider setups and include the same setup in no-op …
KCeh Jul 5, 2024
4b64129
Add check if WorkManager is initialized
KCeh Jul 5, 2024
d10925c
Add delegate worker for certificate check
KCeh Jul 7, 2024
b2bb4e7
Fix lint
KCeh Jul 7, 2024
e80a050
Add annotation to UI thread only method
KCeh Jul 14, 2024
f7666a1
Remove redundant manifest tags
KCeh Jul 14, 2024
ee6f2e0
Merge branch 'develop' into fix/investigate-initializer-errors
KCeh Jul 14, 2024
c023224
Merge branch 'develop' of https://github.com/infinum/android-sentinel…
KCeh Jul 17, 2024
8e5998d
Remove unnecessary work manager initialization
KCeh Jul 17, 2024
337d91f
Typo fix
KCeh Jul 17, 2024
fbdf3a9
Replace failure result with exception
KCeh Jul 17, 2024
bbd0d8a
Improve DI setup for work manager
KCeh Jul 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion sentinel/src/main/kotlin/com/infinum/sentinel/di/component/DomainComponent.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,9 @@ import com.infinum.sentinel.extensions.sizeTree
import com.infinum.sentinel.ui.bundles.callbacks.BundleMonitorActivityCallbacks
import com.infinum.sentinel.ui.bundles.callbacks.BundleMonitorNotificationCallbacks
import com.infinum.sentinel.ui.bundles.details.BundleDetailsActivity
import com.infinum.sentinel.ui.certificates.observer.CertificateCheckWorker
import com.infinum.sentinel.ui.certificates.observer.CertificatesObserver
import com.infinum.sentinel.ui.certificates.observer.DelegateWorker
import com.infinum.sentinel.ui.certificates.observer.SentinelWorkManager
import com.infinum.sentinel.ui.certificates.observer.SentinelWorkerFactory
import com.infinum.sentinel.ui.crash.anr.SentinelAnrObserver
Expand Down Expand Up @@ -160,7 +162,9 @@ internal abstract class DomainComponent(
@Provides
@DomainScope
fun sentinelWorkerFactory(): SentinelWorkerFactory =
SentinelWorkerFactory(collectors, notificationFactory)
SentinelWorkerFactory(collectors, notificationFactory).also {
DelegateWorker.workerFactories[CertificateCheckWorker.NAME] = it
}
KCeh marked this conversation as resolved.
Show resolved Hide resolved

@Provides
@DomainScope
Expand Down
69 changes: 69 additions & 0 deletions sentinel/src/main/kotlin/com/infinum/sentinel/ui/certificates/observer/DelegateWorker.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package com.infinum.sentinel.ui.certificates.observer

import android.content.Context
import android.util.Log
import androidx.work.Data
import androidx.work.ListenableWorker
import androidx.work.WorkerFactory
import androidx.work.WorkerParameters
import com.google.common.util.concurrent.ListenableFuture
import com.infinum.sentinel.ui.shared.Constants.Keys.WORKER_CLASS_NAME
import com.infinum.sentinel.ui.shared.Constants.Keys.WORKER_ID
import java.util.concurrent.Executor
import java.util.concurrent.TimeUnit

/**
* A worker to delegate work requests from within the library to workers
* that require factories with custom dependencies.
*/
internal class DelegateWorker(
appContext: Context,
parameters: WorkerParameters,
) : ListenableWorker(appContext, parameters) {

private val workerClassName =
parameters.inputData.getString(WORKER_CLASS_NAME) ?: ""
private val workerId = parameters.inputData.getString(WORKER_ID)
private val delegateWorkerFactory = workerFactories[workerId]
private val delegatedWorker = delegateWorkerFactory?.createWorker(appContext, workerClassName, parameters)

override fun startWork(): ListenableFuture<Result> {
return if (delegatedWorker != null) {
delegatedWorker.startWork()
} else {
val errorMessage = "No delegateWorker available for $workerId" +
" with workerClassName of $workerClassName. Is the " +
"RouterWorker.workerFactories populated correctly?"

Log.w("Sentinel", errorMessage)

val errorData = Data.Builder().putString("Reason", errorMessage).build()

object : ListenableFuture<Result> {
override fun isDone(): Boolean = true
override fun get(): Result = Result.failure(errorData)
override fun get(timeout: Long, unit: TimeUnit): Result = Result.failure(errorData)
override fun cancel(mayInterruptIfRunning: Boolean): Boolean = false
override fun isCancelled(): Boolean = false
override fun addListener(listener: Runnable, executor: Executor) = listener.run()
}
Copy link
Member

Choose a reason for hiding this comment

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

Why do we return an "empty" future instead of throwing an error? Shouldn't throwing an error be safer?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Well, it is not "empty". It is failure with errorData. We need to return ListenableFuture<Result>

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, but I'm exploring less "silent" ways of this failing. Realistically, it could happen only if we forget to register a worker factory, but that's something we shouldn't allow to end up in a release. I'm trying to figure out how we could make sure to not fail fast and loud :D

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you think just exception will do the job?

Copy link
Member

Choose a reason for hiding this comment

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

I'm open for discussions, but an exception (if it would crash the app) would definitely be more noticeable.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If we throw exception in this place it will just produce logs, similar to Result.Failure. Because it is running on background thread

Work [ id=42358918-83db-4c07-847b-6020296c3d41, tags={ com.infinum.sentinel.ui.certificates.observer.DelegateWorker } ] failed because it threw an exception/error
                                                                                                    java.util.concurrent.ExecutionException: java.lang.IllegalStateException: No delegateWorker available for com.infinum.sentinel.ui.certificates.observer.CertificateCheckWorker with workerClassName of com.infinum.sentinel.ui.certificates.observer.CertificateCheckWorker. Is the DelegateWorker.workerFactories populated correctly?
                                                                                                    	at androidx.work.impl.utils.futures.AbstractFuture.getDoneValue(AbstractFuture.java:515)
                                                                                                    	at androidx.work.impl.utils.futures.AbstractFuture.get(AbstractFuture.java:474)
                                                                                                    	at androidx.work.impl.WorkerWrapper$2.run(WorkerWrapper.java:316)
                                                                                                    	at androidx.work.impl.utils.SerialExecutorImpl$Task.run(SerialExecutorImpl.java:96)
                                                                                                    	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1137)
                                                                                                    	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:637)
                                                                                                    	at java.lang.Thread.run(Thread.java:1012)
                                                                                                    Caused by: java.lang.IllegalStateException: No delegateWorker available for com.infinum.sentinel.ui.certificates.observer.CertificateCheckWorker with workerClassName of com.infinum.sentinel.ui.certificates.observer.CertificateCheckWorker. Is the RouterWorker.workerFactories populated correctly?
                                                                                                    	at com.infinum.sentinel.ui.certificates.observer.DelegateWorker.startWork(DelegateWorker.kt:40)
                                                                                                    	at androidx.work.impl.WorkerWrapper$1.run(WorkerWrapper.java:302)
                                                                                                    	at android.os.Handler.handleCallback(Handler.java:942)
                                                                                                    	at android.os.Handler.dispatchMessage(Handler.java:99)
                                                                                                    	at android.os.Looper.loopOnce(Looper.java:201)
                                                                                                    	at android.os.Looper.loop(Looper.java:288)
                                                                                                    	at android.app.ActivityThread.main(ActivityThread.java:7872)
                                                                                                    	at java.lang.reflect.Method.invoke(Native Method)
                                                                                                    	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
                                                                                                    	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:936)
2024-07-17 16:57:47.222 15350-15401 WM-WorkerWrapper        com.infinum.sentinel.sample.debug    I  Worker result FAILURE for Work [ id=42358918-83db-4c07-847b-6020296c3d41, tags={ com.infinum.sentinel.ui.certificates.observer.DelegateWorker } ]

But error is defiantly more noticeable in logs

Copy link
Collaborator

@KCeh KCeh Jul 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

If things go 💥 at least people will see errors and open issues 😅
So I will go with exception

}
}

companion object {
const val DELEGATE_WORKER_ID = "com.infinum.sentinel.ui.certificates.observer.CertificateCheckWorker"

val workerFactories = object : AbstractMutableMap<String, WorkerFactory>() {

private val backingWorkerMap = mutableMapOf<String, WorkerFactory>()

// should be invoked only from the main thread
override fun put(key: String, value: WorkerFactory): WorkerFactory? {
KCeh marked this conversation as resolved.
Show resolved Hide resolved
return backingWorkerMap.put(key, value)
}

override val entries: MutableSet<MutableMap.MutableEntry<String, WorkerFactory>>
get() = backingWorkerMap.entries
}
}
}
44 changes: 25 additions & 19 deletions ...inel/src/main/kotlin/com/infinum/sentinel/ui/certificates/observer/SentinelWorkManager.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import com.infinum.sentinel.ui.shared.Constants.Keys.EXPIRE_IN_AMOUNT
import com.infinum.sentinel.ui.shared.Constants.Keys.EXPIRE_IN_UNIT
import com.infinum.sentinel.ui.shared.Constants.Keys.NOTIFY_INVALID_NOW
import com.infinum.sentinel.ui.shared.Constants.Keys.NOTIFY_TO_EXPIRE
import com.infinum.sentinel.ui.shared.Constants.Keys.WORKER_CLASS_NAME
import com.infinum.sentinel.ui.shared.Constants.Keys.WORKER_ID
import java.time.Duration
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.flowOf
Expand All @@ -37,35 +39,38 @@ internal class SentinelWorkManager(
}

init {
WorkManager.initialize(
context,
Configuration.Builder()
.setMinimumLoggingLevel(android.util.Log.INFO)
.setWorkerFactory(workerFactory)
.build()
)
if (WorkManager.isInitialized().not()) {
WorkManager.initialize(
context,
Configuration.Builder()
.setMinimumLoggingLevel(android.util.Log.INFO)
.setWorkerFactory(workerFactory)
KCeh marked this conversation as resolved.
Show resolved Hide resolved
.build()
)
}
}

@RequiresApi(Build.VERSION_CODES.O)
fun startCertificatesCheck(entity: CertificateMonitorEntity) =
fun startCertificatesCheck(entity: CertificateMonitorEntity) {

val delegatedWorkData = workDataOf(
WORKER_CLASS_NAME to CertificateCheckWorker::class.qualifiedName,
WORKER_ID to CertificateCheckWorker.NAME,
NOTIFY_INVALID_NOW to entity.notifyInvalidNow,
NOTIFY_TO_EXPIRE to entity.notifyToExpire,
EXPIRE_IN_AMOUNT to entity.expireInAmount,
EXPIRE_IN_UNIT to entity.expireInUnit.name
)
WorkManager.getInstance(context)
.enqueueUniquePeriodicWork(
CertificateCheckWorker.NAME,
DelegateWorker.DELEGATE_WORKER_ID,
ExistingPeriodicWorkPolicy.CANCEL_AND_REENQUEUE,
PeriodicWorkRequestBuilder<CertificateCheckWorker>(
PeriodicWorkRequestBuilder<DelegateWorker>(
antunflas marked this conversation as resolved.
Show resolved Hide resolved
when (BuildConfig.DEBUG) {
true -> Duration.ofMinutes(DEBUG_INTERVAL)
false -> Duration.ofMinutes(RELEASE_INTERVAL)
}
)
.setInputData(
workDataOf(
NOTIFY_INVALID_NOW to entity.notifyInvalidNow,
NOTIFY_TO_EXPIRE to entity.notifyToExpire,
EXPIRE_IN_AMOUNT to entity.expireInAmount,
EXPIRE_IN_UNIT to entity.expireInUnit.name
)
)
).setInputData(delegatedWorkData)
.setConstraints(
Constraints.Builder()
.setRequiredNetworkType(NetworkType.NOT_REQUIRED)
Expand All @@ -80,6 +85,7 @@ internal class SentinelWorkManager(
)
.build()
)
}

fun certificatesCheckState(): Flow<Boolean> =
WorkManager.getInstance(context)
Expand Down
2 changes: 2 additions & 0 deletions sentinel/src/main/kotlin/com/infinum/sentinel/ui/shared/Constants.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,7 @@ internal object Constants {
const val NOTIFY_TO_EXPIRE: String = "KEY_NOTIFY_TO_EXPIRE"
const val EXPIRE_IN_AMOUNT: String = "KEY_EXPIRE_IN_AMOUNT"
const val EXPIRE_IN_UNIT: String = "KEY_EXPIRE_IN_UNIT"
const val WORKER_CLASS_NAME = "WORKER_CLASS_NAME"
const val WORKER_ID = "WORKER_ID"
}
}
Loading