Skip to content

Commit

Permalink
Merge pull request #50 from scribd/sid/improve_error_analytics
Browse files Browse the repository at this point in the history
Improve top error analytics
  • Loading branch information
sidkaria authored Oct 18, 2024
2 parents 428dd66 + ca195fa commit 84c83c4
Show file tree
Hide file tree
Showing 12 changed files with 127 additions and 48 deletions.
1 change: 1 addition & 0 deletions Armadillo/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:tools="http://schemas.android.com/tools">

<uses-permission android:name="android.permission.ACCESS_NETWORK_STATE" />
<uses-permission android:name="android.permission.INTERNET"/>
<uses-permission android:name="android.permission.RECEIVE_BOOT_COMPLETED"/>
<uses-permission android:name="android.permission.FOREGROUND_SERVICE"/>
Expand Down
17 changes: 17 additions & 0 deletions Armadillo/src/main/java/com/scribd/armadillo/Util.kt
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package com.scribd.armadillo

import android.content.Context
import android.net.ConnectivityManager
import android.net.NetworkCapabilities
import android.os.Build
import androidx.annotation.ChecksSdkIntAtLeast
import com.scribd.armadillo.models.Chapter
Expand Down Expand Up @@ -47,5 +49,20 @@ fun sanitizeChapters(chapters: List<Chapter>): List<Chapter> {
}
}

fun isInternetAvailable(context: Context): Boolean {
val connectivityManager =
context.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager
val networkCapabilities = connectivityManager.activeNetwork ?: return false
val actNw =
connectivityManager.getNetworkCapabilities(networkCapabilities) ?: return false
return when {
actNw.hasTransport(NetworkCapabilities.TRANSPORT_WIFI) -> true
actNw.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR) -> true
actNw.hasTransport(NetworkCapabilities.TRANSPORT_ETHERNET) -> true
actNw.hasTransport(NetworkCapabilities.TRANSPORT_VPN) -> true
else -> false
}
}

@ChecksSdkIntAtLeast(api = Build.VERSION_CODES.S)
fun hasSnowCone() = Build.VERSION.SDK_INT >= Build.VERSION_CODES.S
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import com.scribd.armadillo.playback.MediaMetadataCompatBuilderImpl
import com.scribd.armadillo.playback.PlaybackEngineFactoryHolder
import com.scribd.armadillo.playback.PlaybackStateBuilderImpl
import com.scribd.armadillo.playback.PlaybackStateCompatBuilder
import com.scribd.armadillo.playback.PlayerEventListener
import com.scribd.armadillo.playback.mediasource.DrmMediaSourceHelper
import com.scribd.armadillo.playback.mediasource.DrmMediaSourceHelperImpl
import com.scribd.armadillo.playback.mediasource.HeadersMediaSourceFactoryFactory
Expand Down Expand Up @@ -75,4 +76,8 @@ internal class PlaybackModule {
@Provides
@Singleton
fun drmSessionManagerProvider(stateStore: StateStore.Modifier): DrmSessionManagerProvider = ArmadilloDrmSessionManagerProvider(stateStore)

@Provides
@Singleton
fun playerEventListener(context: Context): PlayerEventListener = PlayerEventListener(context)
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ internal data class TestableDownloadState(val id: Int,
val url: String,
val state: Int,
val downloadPercentage: Int,
val downloadedBytes: Long) {
val downloadedBytes: Long,
val failureReason: Int? = null) {
companion object {
const val QUEUED = ExoplayerDownload.STATE_QUEUED
const val COMPLETED = ExoplayerDownload.STATE_COMPLETED
Expand All @@ -24,11 +25,12 @@ internal data class TestableDownloadState(val id: Int,
}

constructor(download: ExoplayerDownload) : this(
download.request.data.decodeToInt(),
download.request.uri.toString(),
download.state,
download.percentDownloaded.toInt(),
download.bytesDownloaded)
download.request.data.decodeToInt(),
download.request.uri.toString(),
download.state,
download.percentDownloaded.toInt(),
download.bytesDownloaded,
download.failureReason)

/**
* This method converts [TestableDownloadState] (a testable wrapper fo exoplayer's [DownloadManager.TaskState])
Expand All @@ -47,13 +49,15 @@ internal data class TestableDownloadState(val id: Int,
}
DownloadState.STARTED(percent, downloadedBytes)
}

QUEUED -> return null
else -> DownloadState.FAILED
else -> DownloadState.FAILED(failureReason)
}

return DownloadProgressInfo(
id = id,
url = url,
downloadState = downloadState)
id = id,
url = url,
downloadState = downloadState,
exoPlayerDownloadState = state)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext
import java.io.IOException
import java.lang.Exception
import java.util.HashMap
import javax.inject.Inject
import javax.inject.Named
import javax.inject.Singleton
Expand Down Expand Up @@ -111,7 +109,7 @@ internal class ExoplayerDownloadTracker @Inject constructor(

override fun updateProgress() {
downloadManager.currentDownloads.forEach { download ->
if(downloads.containsKey(download.request.uri.toString())){
if (downloads.containsKey(download.request.uri.toString())) {
downloads[download.request.uri.toString()] = download //older usage
} else {
downloads[download.request.id] = download
Expand All @@ -129,7 +127,7 @@ internal class ExoplayerDownloadTracker @Inject constructor(

override fun onDownloadChanged(downloadManager: DownloadManager, download: Download, finalException: Exception?) {
Log.v(TAG, "onDownloadChanged")
if(downloads.containsKey(download.request.uri.toString())){
if (downloads.containsKey(download.request.uri.toString())) {
downloads[download.request.uri.toString()] = download //older usage
} else {
downloads[download.request.id] = download
Expand All @@ -155,6 +153,7 @@ internal class ExoplayerDownloadTracker @Inject constructor(
@VisibleForTesting
fun dispatchActionsForProgress(downloadInfo: DownloadProgressInfo) {
val taskFailed = downloadInfo.isFailed()
val exoplayerDownloadState = downloadInfo.exoPlayerDownloadState
val isRemoveDownloadComplete = downloadInfo.downloadState is DownloadState.REMOVED
val isDownloadComplete = downloadInfo.isDownloaded()

Expand All @@ -173,7 +172,13 @@ internal class ExoplayerDownloadTracker @Inject constructor(
}

if (taskFailed) {
actions.add(ErrorAction(DownloadFailed()))
actions.add(ErrorAction(DownloadFailed(
mapOf(
"exo_player_download_state" to exoplayerDownloadState.toString(),
"is_download_complete" to isDownloadComplete.toString(),
"failure_reason" to (downloadInfo.downloadState as? DownloadState.FAILED)?.failureReason.toString()
)
)))
}

actions.forEach {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,12 @@ class InvalidRequest(message: String)
/**
* Playback Errors
*/
data class HttpResponseCodeException(val responseCode: Int, val url: String?, override val cause: Exception)
: ArmadilloException(cause = cause, message = "HTTP Error $responseCode.", isNetworkRelatedError = true) {
data class HttpResponseCodeException(
val responseCode: Int,
val url: String?,
override val cause: Exception,
val extraData: Map<String, String> = emptyMap(),
) : ArmadilloException(cause = cause, message = "HTTP Error $responseCode.", isNetworkRelatedError = true) {
override val errorCode: Int = 200
}

Expand Down Expand Up @@ -96,6 +100,11 @@ class ConnectivityException(cause: Exception)
override val errorCode: Int = 206
}

class ParsingException(cause: Exception)
: ArmadilloException(cause = cause, message = "The content cannot be parsed and it is not playable: ${cause.message}") {
override val errorCode = 207
}

/**
* Download Errors
*/
Expand All @@ -104,7 +113,7 @@ class MissingInfoDownloadException(message: String)
override val errorCode = 301
}

class DownloadFailed
class DownloadFailed(val extraData: Map<String, String>)
: ArmadilloException(message = "The download has failed to finish.", isNetworkRelatedError = true) {
override val errorCode = 302
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,12 @@ data class MediaControlState(
data class DownloadProgressInfo(
val id: Int,
val url: String,
val downloadState: DownloadState) {
val downloadState: DownloadState,
val exoPlayerDownloadState: Int? = null) {

fun isDownloaded(): Boolean = DownloadState.COMPLETED == downloadState

fun isFailed(): Boolean = DownloadState.FAILED == downloadState
fun isFailed(): Boolean = downloadState is DownloadState.FAILED

companion object {
const val PROGRESS_UNSET = C.PERCENTAGE_UNSET
Expand All @@ -129,25 +130,25 @@ sealed class DownloadState {
override fun toString() = "REMOVED"
}

object FAILED : DownloadState() {
data class FAILED(val failureReason: Int? = null) : DownloadState() {
override fun toString() = "FAILED"
}
}

data class InternalState(val isPlaybackEngineReady: Boolean = false)

sealed class DrmState (val drmType: DrmType?, val expireMillis: Milliseconds, val isSessionValid: Boolean) {
sealed class DrmState(val drmType: DrmType?, val expireMillis: Milliseconds, val isSessionValid: Boolean) {
/** This Content is not utilizing DRM protections, or is now first initializing **/
object NoDRM : DrmState(null, 0.milliseconds, true)

/** Attempt to open the license and decrypt */
class LicenseOpening(drmType: DrmType?, expireMillis: Milliseconds = 0.milliseconds): DrmState(drmType, expireMillis, true)
class LicenseOpening(drmType: DrmType?, expireMillis: Milliseconds = 0.milliseconds) : DrmState(drmType, expireMillis, true)

/** A DRM License has been obtained. */
class LicenseAcquired(drmType: DrmType, expireMillis: Milliseconds): DrmState(drmType, expireMillis, true)
class LicenseAcquired(drmType: DrmType, expireMillis: Milliseconds) : DrmState(drmType, expireMillis, true)

/** The player encountered an expiration event */
class LicenseExpired(drmType: DrmType?, expireMillis: Milliseconds): DrmState(drmType, expireMillis, false)
class LicenseExpired(drmType: DrmType?, expireMillis: Milliseconds) : DrmState(drmType, expireMillis, false)

/** A DRM license exists and content is able to be decrypted. */
class LicenseUsable(drmType: DrmType?, expireMillis: Milliseconds) : DrmState(drmType, expireMillis, true)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,38 +1,58 @@
package com.scribd.armadillo.playback

import android.content.Context
import com.google.android.exoplayer2.ExoPlaybackException
import com.google.android.exoplayer2.ExoPlaybackException.TYPE_RENDERER
import com.google.android.exoplayer2.ExoPlaybackException.TYPE_SOURCE
import com.google.android.exoplayer2.ParserException
import com.google.android.exoplayer2.audio.AudioSink
import com.google.android.exoplayer2.drm.MediaDrmCallbackException
import com.google.android.exoplayer2.upstream.DataSpec
import com.google.android.exoplayer2.upstream.HttpDataSource
import com.scribd.armadillo.error.ArmadilloException
import com.scribd.armadillo.error.ArmadilloIOException
import com.scribd.armadillo.error.HttpResponseCodeException
import com.scribd.armadillo.error.ConnectivityException
import com.scribd.armadillo.error.HttpResponseCodeException
import com.scribd.armadillo.error.ParsingException
import com.scribd.armadillo.error.RendererConfigurationException
import com.scribd.armadillo.error.RendererInitializationException
import com.scribd.armadillo.error.RendererWriteException
import com.scribd.armadillo.error.UnexpectedException
import com.scribd.armadillo.error.UnknownRendererException
import com.scribd.armadillo.isInternetAvailable
import java.net.SocketTimeoutException
import java.net.UnknownHostException

internal fun ExoPlaybackException.toArmadilloException(): ArmadilloException {
internal fun ExoPlaybackException.toArmadilloException(context: Context): ArmadilloException {
return if (TYPE_SOURCE == type) {
return this.sourceException.let { source ->
when (source) {
is HttpDataSource.InvalidResponseCodeException ->
HttpResponseCodeException(source.responseCode, source.dataSpec.uri.toString(), source)
HttpResponseCodeException(source.responseCode, source.dataSpec.uri.toString(), source, source.dataSpec.toAnalyticsMap
(context))

is HttpDataSource.HttpDataSourceException ->
HttpResponseCodeException(0, source.dataSpec.uri.toString(), source)
HttpResponseCodeException(source.reason, source.dataSpec.uri.toString(), source, source.dataSpec.toAnalyticsMap(context))

is MediaDrmCallbackException -> {
val httpCause = source.cause as? HttpDataSource.InvalidResponseCodeException
HttpResponseCodeException(httpCause?.responseCode ?: 0, httpCause?.dataSpec?.uri.toString(), source)
HttpResponseCodeException(httpCause?.responseCode
?: 0, httpCause?.dataSpec?.uri.toString(), source, source.dataSpec.toAnalyticsMap(context))
}

is UnknownHostException,
is SocketTimeoutException -> ConnectivityException(source)
else -> ArmadilloIOException(cause = this, actionThatFailedMessage = "Exoplayer error.")

else -> {
var cause: Throwable? = source
while (source.cause != null && cause !is ParserException) {
cause = source.cause
}
when (cause) {
is ParserException -> ParsingException(cause = this)
else -> ArmadilloIOException(cause = this, actionThatFailedMessage = "Exoplayer error.")
}
}
}
}
} else if (TYPE_RENDERER == type) {
Expand All @@ -47,4 +67,18 @@ internal fun ExoPlaybackException.toArmadilloException(): ArmadilloException {
} else {
UnexpectedException(cause = this, actionThatFailedMessage = "Exoplayer error")
}
}

private fun DataSpec.toAnalyticsMap(context: Context): Map<String, String> {
return mapOf(
"uri" to uri.toString(),
"uriPositionOffset" to uriPositionOffset.toString(),
"httpMethod" to httpMethod.toString(),
"position" to position.toString(),
"length" to length.toString(),
"key" to key.toString(),
"flags" to flags.toString(),
"customData" to customData.toString(),
"isInternetConnected" to isInternetAvailable(context).toString(),
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,8 @@ internal class ExoplayerPlaybackEngine(private var audioPlayable: AudioPlayable)
@VisibleForTesting
internal lateinit var exoPlayer: ExoPlayer

private val playerEventListener = PlayerEventListener()
@Inject
internal lateinit var playerEventListener: PlayerEventListener

override val currentChapterIndex: Int
get() = audioPlayable.chapters.indexOf(currentChapter)
Expand Down Expand Up @@ -155,7 +156,7 @@ internal class ExoplayerPlaybackEngine(private var audioPlayable: AudioPlayable)
stateModifier.dispatch(PlaybackEngineReady(true))
stateModifier.dispatch(PlayerStateAction(PlaybackState.PAUSED))
} catch (ex: Exception) {
val armadilloException = if(ex is ArmadilloException) ex else PlaybackStartFailureException(cause = ex)
val armadilloException = if (ex is ArmadilloException) ex else PlaybackStartFailureException(cause = ex)
stateModifier.dispatch(ErrorAction(armadilloException))
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.scribd.armadillo.playback

import android.content.Context
import android.util.Log
import com.google.android.exoplayer2.ExoPlaybackException
import com.google.android.exoplayer2.ExoPlayer
Expand All @@ -22,7 +23,7 @@ import javax.inject.Inject
*
* It communicates changes by sending [Action]s with [StateStore.Modifier].
*/
internal class PlayerEventListener : Player.Listener {
internal class PlayerEventListener @Inject constructor(private val context: Context) : Player.Listener {
init {
Injector.mainComponent.inject(this)
}
Expand All @@ -35,7 +36,7 @@ internal class PlayerEventListener : Player.Listener {
internal lateinit var stateModifier: StateStore.Modifier

override fun onPlayerError(error: PlaybackException) {
val exception = (error as ExoPlaybackException).toArmadilloException()
val exception = (error as ExoPlaybackException).toArmadilloException(context)
stateModifier.dispatch(ErrorAction(exception))
Log.e(TAG, "onPlayerError: $error")
}
Expand Down
Loading

0 comments on commit 84c83c4

Please sign in to comment.