Merge pull request #38 from scribd/kabliz/APT-9578-errors

[APT-9578] Improve Exception Handling
scribd · Aug 14, 2024 · e9df9bb · e9df9bb
2 parents f304e58 + 6155a79
commit e9df9bb
Show file tree

Hide file tree

Showing 15 changed files with 105 additions and 62 deletions.
diff --git a/Armadillo/src/main/java/com/scribd/armadillo/ArmadilloPlayerChoreographer.kt b/Armadillo/src/main/java/com/scribd/armadillo/ArmadilloPlayerChoreographer.kt
@@ -241,7 +241,7 @@ internal class ArmadilloPlayerChoreographer : ArmadilloPlayer {
 
     override fun beginDownload(audioPlayable: AudioPlayable) {
         if (!isDownloadEngineInit) {
-            stateModifier.dispatch(ErrorAction(EngineNotInitialized("download engine not init")))
+            stateModifier.dispatch(ErrorAction(EngineNotInitialized("download engine cannot start download.")))
             return
         }
         downloadEngine.download(audioPlayable)
@@ -253,7 +253,7 @@ internal class ArmadilloPlayerChoreographer : ArmadilloPlayer {
 
     override fun removeAllDownloads() {
         if (!isDownloadEngineInit) {
-            stateModifier.dispatch(ErrorAction(EngineNotInitialized("download engine not init")))
+            stateModifier.dispatch(ErrorAction(EngineNotInitialized("Cannot remove all the downloads.")))
             return
         }
         downloadEngine.removeAllDownloads()
@@ -308,7 +308,10 @@ internal class ArmadilloPlayerChoreographer : ArmadilloPlayer {
                 PlaybackState.PLAYING -> controls.pause()
                 PlaybackState.PAUSED -> controls.play()
                 else -> {
-                    stateModifier.dispatch(ErrorAction(UnexpectedException("Unknown state: $playbackState")))
+                    stateModifier.dispatch(ErrorAction(
+                        UnexpectedException(cause = IllegalStateException("Neither playing nor paused"),
+                            actionThatFailedMessage = "Trying to play or pause media."))
+                    )
                 }
             }
         }
@@ -333,15 +336,18 @@ internal class ArmadilloPlayerChoreographer : ArmadilloPlayer {
     override fun seekWithinChapter(percent: Int) {
         val position = stateProvider.currentState.positionFromChapterPercent(percent)
             ?: run {
-                stateModifier.dispatch(ErrorAction(UnexpectedException("No audiobook playing")))
+                stateModifier.dispatch(ErrorAction(
+                    UnexpectedException(cause = KotlinNullPointerException("Current state's position is null"),
+                        actionThatFailedMessage = "seeking within chapter"))
+                )
                 return
             }
         seekTo(position)
     }
 
     override fun removeDownload(audioPlayable: AudioPlayable) {
         if (!isDownloadEngineInit) {
-            stateModifier.dispatch(ErrorAction(EngineNotInitialized("DownloadEngine")))
+            stateModifier.dispatch(ErrorAction(EngineNotInitialized("Cannot remove a download.")))
             return
         }
         downloadEngine.removeDownload(audioPlayable)
@@ -412,18 +418,16 @@ internal class ArmadilloPlayerChoreographer : ArmadilloPlayer {
             lambda.invoke(controls, playbackState)
         } else {
             val error = if (controls == null) {
-                Log.e(TAG, "transportControls null")
-                TransportControlsNull
+                Log.e(TAG, "The transport controls are null. Are the controls initialized?")
+                TransportControlsNull()
             } else if (playbackState == null) {
-                Log.e(TAG, "playbackState is null")
-                NoPlaybackInfo
+                Log.e(TAG, "The playbackState is null.")
+                NoPlaybackInfo()
             } else if (PlaybackState.NONE == playbackState) {
-                Log.e(TAG, "playbackState == PlaybackState.NONE")
-                InvalidPlaybackState
-            } else if (!playbackReady) {
-                EngineNotInitialized("isPlaybackEngineReady == false")
+                Log.e(TAG, "There is no playback State.")
+                InvalidPlaybackState()
             } else {
-                UnexpectedException("unknown error in $TAG.doIfPlaybackReady")
+                EngineNotInitialized("Playback is not ready.")
             }
 
             stateModifier.dispatch(ErrorAction(error))

diff --git a/Armadillo/src/main/java/com/scribd/armadillo/Reducer.kt b/Armadillo/src/main/java/com/scribd/armadillo/Reducer.kt
@@ -282,7 +282,7 @@ internal object Reducer {
     private fun checkBounds(timeToCheck: () -> Interval<Millisecond>): ArmadilloException? {
         return if (maxDurationDiscrepancy >= 0 && timeToCheck.invoke().inSeconds.longValue > maxDurationDiscrepancy) {
             Log.e(TAG, "Content metadata is incorrect")
-            IncorrectChapterMetadataException
+            IncorrectChapterMetadataException()
         } else {
             null
         }

diff --git a/Armadillo/src/main/java/com/scribd/armadillo/StateStore.kt b/Armadillo/src/main/java/com/scribd/armadillo/StateStore.kt
@@ -49,5 +49,5 @@ internal class ArmadilloStateStore(private val reducer: Reducer) :
         get() = armadilloStateObservable
 
     override val currentState: ArmadilloState
-        get() = armadilloStateObservable.value ?: throw MissingDataException("armadilloState should never be null")
+        get() = armadilloStateObservable.value ?: throw MissingDataException("Armadillo's State should never be null")
 }
diff --git a/Armadillo/src/main/java/com/scribd/armadillo/Util.kt b/Armadillo/src/main/java/com/scribd/armadillo/Util.kt
@@ -2,6 +2,7 @@ package com.scribd.armadillo
 
 import android.content.Context
 import android.os.Build
+import androidx.annotation.ChecksSdkIntAtLeast
 import com.scribd.armadillo.models.Chapter
 import com.scribd.armadillo.time.Interval
 import com.scribd.armadillo.time.Millisecond
@@ -46,4 +47,5 @@ fun sanitizeChapters(chapters: List<Chapter>): List<Chapter> {
     }
 }
 
+@ChecksSdkIntAtLeast(api = Build.VERSION_CODES.S)
 fun hasSnowCone() = Build.VERSION.SDK_INT >= Build.VERSION_CODES.S
diff --git a/Armadillo/src/main/java/com/scribd/armadillo/download/DownloadEngine.kt b/Armadillo/src/main/java/com/scribd/armadillo/download/DownloadEngine.kt
@@ -17,6 +17,7 @@ import com.scribd.armadillo.StateStore
 import com.scribd.armadillo.actions.ErrorAction
 import com.scribd.armadillo.download.drm.OfflineDrmManager
 import com.scribd.armadillo.error.ArmadilloException
+import com.scribd.armadillo.error.ArmadilloIOException
 import com.scribd.armadillo.error.DownloadServiceLaunchedInBackground
 import com.scribd.armadillo.error.UnexpectedDownloadException
 import com.scribd.armadillo.extensions.encodeInByteArray
@@ -80,15 +81,15 @@ internal class ExoplayerDownloadEngine @Inject constructor(
                             startDownload(context, request)
                         } catch (e: Exception) {
                             if (hasSnowCone() && e is ForegroundServiceStartNotAllowedException) {
-                                stateModifier.dispatch(ErrorAction(DownloadServiceLaunchedInBackground(audioPlayable.id)))
+                                stateModifier.dispatch(ErrorAction(DownloadServiceLaunchedInBackground(audioPlayable.id, e)))
                             } else {
-                                stateModifier.dispatch(ErrorAction(com.scribd.armadillo.error.ArmadilloIOException(e)))
+                                stateModifier.dispatch(ErrorAction(ArmadilloIOException(cause = e, actionThatFailedMessage = "Can't prepare download.")))
                             }
                         }
                     }
 
                     override fun onPrepareError(helper: DownloadHelper, e: IOException) =
-                        stateModifier.dispatch(ErrorAction(com.scribd.armadillo.error.ArmadilloIOException(e)))
+                        stateModifier.dispatch(ErrorAction(ArmadilloIOException(cause = e, actionThatFailedMessage = "Can't report download error.")))
                 })
             }
         }

diff --git a/Armadillo/src/main/java/com/scribd/armadillo/download/ExoplayerDownloadTracker.kt b/Armadillo/src/main/java/com/scribd/armadillo/download/ExoplayerDownloadTracker.kt
@@ -96,7 +96,7 @@ internal class ExoplayerDownloadTracker @Inject constructor(
             } catch (e: IOException) {
                 Log.e(TAG, "Failed to load downloads", e)
                 withContext(Dispatchers.Main) {
-                    stateModifier.dispatch(ErrorAction(UnableToLoadDownloadInfo))
+                    stateModifier.dispatch(ErrorAction(UnableToLoadDownloadInfo()))
                 }
             }
         }
@@ -166,7 +166,7 @@ internal class ExoplayerDownloadTracker @Inject constructor(
         }
 
         if (taskFailed) {
-            actions.add(ErrorAction(DownloadFailed))
+            actions.add(ErrorAction(DownloadFailed()))
         }
 
         actions.forEach {

diff --git a/Armadillo/src/main/java/com/scribd/armadillo/error/ArmadilloException.kt b/Armadillo/src/main/java/com/scribd/armadillo/error/ArmadilloException.kt
@@ -2,84 +2,103 @@ package com.scribd.armadillo.error
 
 import com.scribd.armadillo.actions.Action
 
-sealed class ArmadilloException(exception: Exception) : Exception(exception) {
+sealed class ArmadilloException(cause: Throwable? = null,
+                                message: String = "Unhandled Armadillo Error")
+    : Exception(message, cause) {
     abstract val errorCode: Int
 }
 
 /**
  * Errors in Armadillo
  */
-data class EngineNotInitialized(val reason: String) : ArmadilloException(exception = Exception(reason)) {
+class EngineNotInitialized(message: String)
+    : ArmadilloException(message = "Armadillo Engine is not initialized: $message") {
     override val errorCode = 100
 }
 
-object TransportControlsNull : ArmadilloException(Exception()) {
+class TransportControlsNull
+    : ArmadilloException(message = "Internal error, the transport controls are null. Are the controls being initialized?") {
     override val errorCode = 101
 }
 
-data class MissingDataException(val reason: String) : ArmadilloException(exception = Exception(reason)) {
+class MissingDataException(message: String)
+    : ArmadilloException(message = "Internal error, unexpectedly missing internal data: $message") {
     override val errorCode = 102
 }
 
-data class ActionBeforeSetup(val action: Action) : ArmadilloException(exception = Exception(action.name)) {
+data class ActionBeforeSetup(val action: Action)
+    : ArmadilloException(message = "Internal Error, using an Action before setup is finished: ${action.name}") {
     override val errorCode = 103
 }
 
-data class UnrecognizedAction(val action: Action) : ArmadilloException(exception = Exception(action.name)) {
+data class UnrecognizedAction(val action: Action)
+    : ArmadilloException(message = "Internal error, unknown action being taken: ${action.name}") {
     override val errorCode = 104
 }
 
-object NoPlaybackInfo : ArmadilloException(Exception()) {
+class NoPlaybackInfo : ArmadilloException(message = "Internal error, playback info is missing. Is the player initialized?") {
     override val errorCode = 105
 }
 
-object InvalidPlaybackState : ArmadilloException(Exception()) {
+class InvalidPlaybackState
+    : ArmadilloException(message = "Internal Error, playback state is missing. Has the player been initialized or destroyed?") {
     override val errorCode = 106
 }
 
-data class InvalidRequest(val reason: String) : ArmadilloException(exception = Exception(reason)) {
+class InvalidRequest(message: String)
+    : ArmadilloException(message = "Internal Error, certain media requests cannot be taken: $message") {
     override val errorCode: Int = 107
 }
 
 /**
  * Playback Errors
  */
-data class HttpResponseCodeException(val responseCode: Int, val url: String?, val exception: Exception) : ArmadilloException(exception) {
+data class HttpResponseCodeException(val responseCode: Int, val url: String?, override val cause: Exception)
+    : ArmadilloException(cause = cause, message = "HTTP Error $responseCode.") {
     override val errorCode: Int = 200
 }
 
-data class ArmadilloIOException(val exception: Exception) : ArmadilloException(exception) {
+class ArmadilloIOException(cause: Exception, actionThatFailedMessage: String)
+    : ArmadilloException(cause = cause, message = "A critical playback issue occurred: $actionThatFailedMessage") {
     override val errorCode = 201
 }
 
-data class UpdateProgressFailureException(val exception: Exception) : ArmadilloException(exception) {
+class UpdateProgressFailureException(cause: Exception)
+    : ArmadilloException(cause = cause, message = "Progress failed to update suddenly during playback. Download progress or playback " +
+    "progress may not be reporting correctly.") {
     override val errorCode = 202
 }
 
-data class PlaybackStartFailureException(val exception: Exception) : ArmadilloException(exception) {
+class PlaybackStartFailureException(cause: Exception)
+    : ArmadilloException(cause = cause, message = "The player is initialized, but it failed to begin.") {
     override val errorCode = 203
 }
 
-data class ActionListenerException(val exception: Exception) : ArmadilloException(exception) {
+class ActionListenerException(cause: Exception)
+    : ArmadilloException(cause = cause, message = "A problem occurred processing state updates.") {
     override val errorCode = 204
 }
 
-object IncorrectChapterMetadataException : ArmadilloException(Exception()) {
+class IncorrectChapterMetadataException
+    : ArmadilloException(message = "The metadata for chapter durations are wrong and cannot be used for this content.") {
     override val errorCode = 205
 }
 
 /**
  * Download Errors
  */
-data class MissingInfoDownloadException(val reason: String) : ArmadilloException(exception = Exception(reason)) {
+class MissingInfoDownloadException(message: String)
+    : ArmadilloException(message = "Download info is missing: $message") {
     override val errorCode = 301
 }
 
-object DownloadFailed : ArmadilloException(Exception()) {
+class DownloadFailed
+    : ArmadilloException(message = "The download has failed to finish.") {
     override val errorCode = 302
 }
 
-object UnableToLoadDownloadInfo : ArmadilloException(Exception()) {
+class UnableToLoadDownloadInfo
+    : ArmadilloException(message = "Failed to load download metadata, queued downloads may fail to resume.") {
     override val errorCode = 303
 }
 
@@ -89,61 +108,71 @@ object UnableToLoadDownloadInfo : ArmadilloException(Exception()) {
  * the background before DownloadService.sendAddDownload
  */
 
-data class DownloadServiceLaunchedInBackground(val id: Int) : ArmadilloException(Exception()) {
+data class DownloadServiceLaunchedInBackground(val id: Int, override val cause: Exception)
+    : ArmadilloException(cause = cause,
+    message = "Android 12 compliance problem: ${cause.message}") {
     override val errorCode = 304
 }
 
-data class UnexpectedDownloadException(val throwable: Throwable): ArmadilloException(exception = Exception(throwable)){
+class UnexpectedDownloadException(throwable: Throwable)
+    : ArmadilloException(cause = throwable, message = "Unknown problem while downloading."){
     override val errorCode = 305
 }
 
 /**
  * Misc Errors
  */
-data class UnexpectedException(val exception: Exception) : ArmadilloException(exception) {
-    constructor(reason: String) : this(Exception(reason))
-
+class UnexpectedException(cause: Exception, actionThatFailedMessage: String)
+    : ArmadilloException(cause = cause, message = "Unanticipated error: $actionThatFailedMessage.") {
     override val errorCode = 400
 }
 
 /** Media Browse Errors */
-data class BadMediaHierarchyException(val reason: String) : ArmadilloException(exception = Exception(reason)) {
+class BadMediaHierarchyException(cause: Exception?, message: String)
+    : ArmadilloException(cause = cause, message = message) {
     override val errorCode: Int = 501
 }
 
 /**
  * Audio Renderer Errors
  */
 
-data class RendererConfigurationException(val exception: Exception) : ArmadilloException(exception) {
+class RendererConfigurationException(cause: Exception)
+    : ArmadilloException(cause = cause, message = "The audio data buffer (AudioSink) has been misconfigured.") {
     override val errorCode: Int = 601
 }
 
-data class RendererInitializationException(val exception: Exception) : ArmadilloException(exception) {
+class RendererInitializationException(cause: Exception)
+    : ArmadilloException(cause = cause, message = "The audio data buffer (AudioSink) isn't initialized.") {
     override val errorCode: Int = 602
 }
 
-data class RendererWriteException(val exception: Exception) : ArmadilloException(exception) {
+class RendererWriteException(cause: Exception)
+    : ArmadilloException(cause = cause, message = "Cannot write data into the audio data buffer (AudioSink).") {
     override val errorCode: Int = 603
 }
 
-data class UnknownRendererException(val exception: Exception) : ArmadilloException(exception) {
+class UnknownRendererException(cause: Exception)
+    : ArmadilloException(cause = cause, message = "Unknown problem with the audio data buffer (AudioSink).") {
     override val errorCode: Int = 604
 }
 
 /**
  * DRM errors
  */
 
-data class DrmContentTypeUnsupportedException(val contentType: Int) : ArmadilloException(exception = Exception()) {
+data class DrmContentTypeUnsupportedException(val contentType: Int)
+    : ArmadilloException(message = "This DRM content type is not yet supported. Please consider opening a pull request.") {
     override val errorCode = 700
 }
 
-data class DrmDownloadException(val exception: Exception) : ArmadilloException(exception) {
+class DrmDownloadException(cause: Exception)
+    : ArmadilloException(cause = cause, "Failed to process DRM license for downloading.") {
     override val errorCode = 701
 }
 
-data class DrmPlaybackException(val exception: Exception) : ArmadilloException(exception) {
+class DrmPlaybackException(cause: Exception)
+    : ArmadilloException(cause = cause, message = "Cannot play DRM content.") {
     override val errorCode = 702
 }
 
diff --git a/Armadillo/src/main/java/com/scribd/armadillo/models/ArmadilloState.kt b/Armadillo/src/main/java/com/scribd/armadillo/models/ArmadilloState.kt
@@ -31,7 +31,8 @@ data class ArmadilloState(
             return null
         }
         if (percent > 100 || percent < 0) {
-            throw UnexpectedException(Exception("Invalid argument: $percent"))
+            throw UnexpectedException(cause = IllegalArgumentException("Invalid argument: $percent"),
+                actionThatFailedMessage = "Calculating seekbar progress from percentage.")
         }
         val currentChapter = playbackInfo.audioPlayable.chapters[playbackInfo.progress.currentChapterIndex]
         val decimal = percent / 100.toDouble()

diff --git a/Armadillo/src/main/java/com/scribd/armadillo/playback/ExoPlaybackExceptionExt.kt b/Armadillo/src/main/java/com/scribd/armadillo/playback/ExoPlaybackExceptionExt.kt
@@ -27,7 +27,7 @@ internal fun ExoPlaybackException.toArmadilloException(): ArmadilloException {
                 is SocketTimeoutException -> HttpResponseCodeException(0, null, source)
                 is UnknownHostException ->
                     HttpResponseCodeException(0, source.message, source) // Message is supposed to be the host for UnknownHostException
-                else -> ArmadilloIOException(this)
+                else -> ArmadilloIOException(cause = this, actionThatFailedMessage = "Exoplayer error.")
             }
         }
     } else if (TYPE_RENDERER == type) {
@@ -40,6 +40,6 @@ internal fun ExoPlaybackException.toArmadilloException(): ArmadilloException {
             }
         }
     } else {
-        UnexpectedException(this)
+        UnexpectedException(cause = this, actionThatFailedMessage = "Exoplayer error")
     }
 }
diff --git a/Armadillo/src/main/java/com/scribd/armadillo/playback/MediaSessionConnection.kt b/Armadillo/src/main/java/com/scribd/armadillo/playback/MediaSessionConnection.kt
@@ -48,7 +48,7 @@ internal class MediaSessionConnection(
          * completed.
          */
         override fun onConnected() {
-            val mediaBrowserCompat = mediaBrowser ?: throw MissingDataException("media browser should be init")
+            val mediaBrowserCompat = mediaBrowser ?: throw MissingDataException("Media Browser needs to be initialized.")
             val mediaControllerCompat = MediaControllerCompat(context, mediaBrowserCompat.sessionToken)
             mediaController = mediaControllerCompat
             connectionListener.onConnectionCallback(mediaControllerCompat.transportControls)