Skip to content

Commit

Permalink
fix: use startForegroundService only as last resort (#579)
Browse files Browse the repository at this point in the history
* fix: use startForegroundService only as a last resort

* fix: log exception instead of crashing

* fix: try to catch the uncatchable RemoteServiceException when using the unsafe API by resorting to the default uncaught exception handler

* fix: revert. Bad idea. Even when setting a custom default uncaught exception handler, the app is not recoverable at that point, so it's pointless to adopt this technique

* fix: use labels to enhance readability

* fix: more labels
  • Loading branch information
gotev authored Apr 13, 2021
1 parent a7efe50 commit 7744d8a
Showing 1 changed file with 95 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package net.gotev.uploadservice.extensions
import android.app.PendingIntent
import android.content.Context
import android.content.Intent
import android.os.Build
import net.gotev.uploadservice.UploadService
import net.gotev.uploadservice.UploadServiceConfig
import net.gotev.uploadservice.UploadTask
Expand All @@ -11,6 +12,7 @@ import net.gotev.uploadservice.data.UploadTaskParameters
import net.gotev.uploadservice.logger.UploadServiceLogger
import net.gotev.uploadservice.logger.UploadServiceLogger.NA
import net.gotev.uploadservice.observer.task.UploadTaskObserver
import java.lang.IllegalStateException

// constants used in the intent which starts this service
private const val taskParametersKey = "taskParameters"
Expand All @@ -30,7 +32,44 @@ fun Context.startNewUpload(
putExtra(taskNotificationConfig, notificationConfig)
}

startService(intent)
try {
/*
When trying to start a service on API 26+
while the app is in the background, an IllegalStateException will be fired
https://developer.android.com/reference/android/content/Context#startService(android.content.Intent)
Then why not using startForegroundService always on API 26+? Read below
*/
startService(intent)
} catch (exc: Throwable) {
if (Build.VERSION.SDK_INT >= 26 && exc is IllegalStateException) {
/*
this is a bugged Android API and Google is not going to fix it
https://issuetracker.google.com/issues/76112072
Android SDK can not guarantee that the service is going to be started in under 5 seconds
which in turn can cause the non catchable
RemoteServiceException: Context.startForegroundService() did not then call Service.startForeground()
so the library is going to use this bugged API only as a last resort, to be able
to support starting uploads also when the app is in the background, but preventing
non catchable exceptions when you launch uploads while the app is in foreground.
*/
startForegroundService(intent)
} else {
UploadServiceLogger.error(
component = "UploadService",
uploadId = params.id,
exception = exc,
message = {
"Error while starting AndroidUploadService"
}
)
}
}

return params.id
}
Expand All @@ -43,31 +82,63 @@ data class UploadTaskCreationParameters(
@Suppress("UNCHECKED_CAST")
fun Intent?.getUploadTaskCreationParameters(): UploadTaskCreationParameters? {
if (this == null || action != UploadServiceConfig.uploadAction) {
UploadServiceLogger.error(UploadService.TAG, NA) { "Error while instantiating new task. Invalid intent received" }
UploadServiceLogger.error(
component = UploadService.TAG,
uploadId = NA,
message = {
"Error while instantiating new task. Invalid intent received"
}
)
return null
}

val params: UploadTaskParameters = getParcelableExtra(taskParametersKey) ?: run {
UploadServiceLogger.error(UploadService.TAG, NA) { "Error while instantiating new task. Missing task parameters." }
UploadServiceLogger.error(
component = UploadService.TAG,
uploadId = NA,
message = {
"Error while instantiating new task. Missing task parameters."
}
)
return null
}

val taskClass = try {
Class.forName(params.taskClass)
} catch (exc: Throwable) {
UploadServiceLogger.error(UploadService.TAG, NA, exc) { "Error while instantiating new task. ${params.taskClass} does not exist." }
UploadServiceLogger.error(
component = UploadService.TAG,
uploadId = NA,
exception = exc,
message = {
"Error while instantiating new task. ${params.taskClass} does not exist."
}
)
null
} ?: return null

if (!UploadTask::class.java.isAssignableFrom(taskClass)) {
UploadServiceLogger.error(UploadService.TAG, NA) { "Error while instantiating new task. ${params.taskClass} does not extend UploadTask." }
UploadServiceLogger.error(
component = UploadService.TAG,
uploadId = NA,
message = {
"Error while instantiating new task. ${params.taskClass} does not extend UploadTask."
}
)
return null
}

val notificationConfig: UploadNotificationConfig = getParcelableExtra(taskNotificationConfig) ?: run {
UploadServiceLogger.error(UploadService.TAG, NA) { "Error while instantiating new task. Missing notification config." }
return null
}
val notificationConfig: UploadNotificationConfig =
getParcelableExtra(taskNotificationConfig) ?: run {
UploadServiceLogger.error(
component = UploadService.TAG,
uploadId = NA,
message = {
"Error while instantiating new task. Missing notification config."
}
)
return null
}

return UploadTaskCreationParameters(
params = params,
Expand Down Expand Up @@ -98,10 +169,23 @@ fun Context.getUploadTask(
)
}

UploadServiceLogger.debug(UploadService.TAG, NA) { "Successfully created new task with class: ${taskClass.name}" }
UploadServiceLogger.debug(
component = UploadService.TAG,
uploadId = NA,
message = {
"Successfully created new task with class: ${taskClass.name}"
}
)
uploadTask
} catch (exc: Throwable) {
UploadServiceLogger.error(UploadService.TAG, NA, exc) { "Error while instantiating new task" }
UploadServiceLogger.error(
component = UploadService.TAG,
uploadId = NA,
exception = exc,
message = {
"Error while instantiating new task"
}
)
null
}
}
Expand Down

0 comments on commit 7744d8a

Please sign in to comment.