Throwing GravatarException on noncatching service methods

hamorillo · hamorillo · commit add42d28bf87 · 2024-10-15T13:50:25.000+02:00
We want to wrap all exceptions that can occur inside the Gravatar service methods in our own "GravatarException."

With this approach, we'll provide detailed information about the error, but at the same time, third-party developers know what exceptions can be expected from the Gravatar Services.
diff --git a/gravatar-quickeditor/src/main/java/com/gravatar/quickeditor/ui/avatarpicker/AvatarPickerViewModel.kt b/gravatar-quickeditor/src/main/java/com/gravatar/quickeditor/ui/avatarpicker/AvatarPickerViewModel.kt
@@ -264,7 +264,7 @@ internal class AvatarPickerViewModel(
                 ErrorType.NotFound,
                 ErrorType.RateLimitExceeded,
                 ErrorType.Timeout,
-                ErrorType.Unknown,
+                is ErrorType.Unknown,
                 is ErrorType.InvalidRequest,
                 -> SectionError.Unknown
             }
diff --git a/gravatar-quickeditor/src/test/java/com/gravatar/quickeditor/data/repository/AvatarRepositoryTest.kt b/gravatar-quickeditor/src/test/java/com/gravatar/quickeditor/data/repository/AvatarRepositoryTest.kt
@@ -97,12 +97,12 @@ class AvatarRepositoryTest {
         coEvery { tokenStorage.getToken(any()) } returns "token"
         coEvery {
             avatarService.setAvatarCatching(any(), any(), any())
-        } returns GravatarResult.Failure(ErrorType.Unknown)
+        } returns GravatarResult.Failure(ErrorType.Unknown())
 
         val result = avatarRepository.selectAvatar(email, "avatarId")
 
         assertEquals(
-            GravatarResult.Failure<String, QuickEditorError>(QuickEditorError.Request(ErrorType.Unknown)),
+            GravatarResult.Failure<String, QuickEditorError>(QuickEditorError.Request(ErrorType.Unknown())),
             result,
         )
     }
diff --git a/gravatar-quickeditor/src/test/java/com/gravatar/quickeditor/ui/avatarpicker/AvatarPickerViewModelTest.kt b/gravatar-quickeditor/src/test/java/com/gravatar/quickeditor/ui/avatarpicker/AvatarPickerViewModelTest.kt
@@ -62,7 +62,7 @@ class AvatarPickerViewModelTest {
 
     @Before
     fun setup() {
-        coEvery { profileService.retrieveCatching(email) } returns GravatarResult.Failure(ErrorType.Unknown)
+        coEvery { profileService.retrieveCatching(email) } returns GravatarResult.Failure(ErrorType.Unknown())
         coEvery { avatarRepository.getAvatars(email) } returns GravatarResult.Success(emailAvatars)
     }
 
diff --git a/gravatar-quickeditor/src/test/java/com/gravatar/quickeditor/ui/oauth/OAuthViewModelTest.kt b/gravatar-quickeditor/src/test/java/com/gravatar/quickeditor/ui/oauth/OAuthViewModelTest.kt
@@ -91,7 +91,7 @@ class OAuthViewModelTest {
     fun `given token when association check failed then UiState_Status updated`() = runTest {
         coEvery {
             profileService.checkAssociatedEmailCatching(token, email)
-        } returns GravatarResult.Failure(ErrorType.Unknown)
+        } returns GravatarResult.Failure(ErrorType.Unknown())
 
         viewModel.uiState.test {
             expectMostRecentItem()
@@ -108,7 +108,7 @@ class OAuthViewModelTest {
     fun `given token when association check failed then token stored`() = runTest {
         coEvery {
             profileService.checkAssociatedEmailCatching(token, email)
-        } returns GravatarResult.Failure(ErrorType.Unknown)
+        } returns GravatarResult.Failure(ErrorType.Unknown())
 
         viewModel.tokenReceived(
             email,
diff --git a/gravatar/api/gravatar.api b/gravatar/api/gravatar.api
@@ -705,12 +705,20 @@ public final class com/gravatar/services/ErrorType$Unauthorized : com/gravatar/s
 }
 
 public final class com/gravatar/services/ErrorType$Unknown : com/gravatar/services/ErrorType {
-	public static final field INSTANCE Lcom/gravatar/services/ErrorType$Unknown;
+	public fun <init> ()V
+	public fun <init> (Ljava/lang/String;)V
+	public synthetic fun <init> (Ljava/lang/String;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
 	public fun equals (Ljava/lang/Object;)Z
+	public final fun getErrorMsg ()Ljava/lang/String;
 	public fun hashCode ()I
 	public fun toString ()Ljava/lang/String;
 }
 
+public final class com/gravatar/services/GravatarException : java/lang/RuntimeException {
+	public final fun getErrorType ()Lcom/gravatar/services/ErrorType;
+	public final fun getOriginalException ()Ljava/lang/Exception;
+}
+
 public abstract interface class com/gravatar/services/GravatarListener {
 	public abstract fun onError (Ljava/lang/Object;)V
 	public abstract fun onSuccess (Ljava/lang/Object;)V
@@ -742,11 +750,6 @@ public final class com/gravatar/services/GravatarResult$Success : com/gravatar/s
 	public fun toString ()Ljava/lang/String;
 }
 
-public final class com/gravatar/services/HttpException : java/lang/RuntimeException {
-	public final fun getCode ()I
-	public fun getMessage ()Ljava/lang/String;
-}
-
 public final class com/gravatar/services/ProfileService {
 	public static final field LOG_TAG Ljava/lang/String;
 	public fun <init> ()V
diff --git a/gravatar/build.gradle.kts b/gravatar/build.gradle.kts
@@ -84,6 +84,7 @@ dependencies {
     testImplementation(libs.mockk.android)
     testImplementation(libs.mockk.agent)
     testImplementation(libs.kotlinx.coroutines.test)
+    testImplementation(kotlin("test"))
 }
 
 project.afterEvaluate {
diff --git a/gravatar/src/main/java/com/gravatar/services/AvatarService.kt b/gravatar/src/main/java/com/gravatar/services/AvatarService.kt
@@ -29,23 +29,25 @@ public class AvatarService(private val okHttpClient: OkHttpClient? = null) {
      * @param file The image file to upload
      * @param oauthToken The OAuth token to use for authentication
      */
-    public suspend fun upload(file: File, oauthToken: String): Avatar = withContext(GravatarSdkDI.dispatcherIO) {
-        val service = instance.getGravatarV3Service(okHttpClient, oauthToken)
+    public suspend fun upload(file: File, oauthToken: String): Avatar = runThrowingExceptionRequest {
+        withContext(GravatarSdkDI.dispatcherIO) {
+            val service = instance.getGravatarV3Service(okHttpClient, oauthToken)
 
-        val filePart =
-            MultipartBody.Part.createFormData("image", file.name, file.asRequestBody())
+            val filePart =
+                MultipartBody.Part.createFormData("image", file.name, file.asRequestBody())
 
-        val response = service.uploadAvatar(filePart)
+            val response = service.uploadAvatar(filePart)
 
-        if (response.isSuccessful && response.body() != null) {
-            response.body()!!
-        } else {
-            // Log the response body for debugging purposes if the response is not successful
-            Logger.w(
-                LOG_TAG,
-                "Network call unsuccessful trying to upload Gravatar: $response.body",
-            )
-            throw HttpException(response)
+            if (response.isSuccessful && response.body() != null) {
+                response.body()!!
+            } else {
+                // Log the response body for debugging purposes if the response is not successful
+                Logger.w(
+                    LOG_TAG,
+                    "Network call unsuccessful trying to upload Gravatar: $response.body",
+                )
+                throw HttpException(response)
+            }
         }
     }
 
@@ -70,7 +72,7 @@ public class AvatarService(private val okHttpClient: OkHttpClient? = null) {
      * @param hash The hash of the email to associate the avatars with
      * @return The list of avatars
      */
-    public suspend fun retrieve(oauthToken: String, hash: Hash): List<Avatar> =
+    public suspend fun retrieve(oauthToken: String, hash: Hash): List<Avatar> = runThrowingExceptionRequest {
         withContext(GravatarSdkDI.dispatcherIO) {
             val service = instance.getGravatarV3Service(okHttpClient, oauthToken)
 
@@ -87,6 +89,7 @@ public class AvatarService(private val okHttpClient: OkHttpClient? = null) {
                 throw HttpException(response)
             }
         }
+    }
 
     /**
      * Retrieves a list of available avatars for the authenticated user.
@@ -110,20 +113,22 @@ public class AvatarService(private val okHttpClient: OkHttpClient? = null) {
      * @param oauthToken The OAuth token to use for authentication
      */
     public suspend fun setAvatar(hash: String, avatarId: String, oauthToken: String): Unit =
-        withContext(GravatarSdkDI.dispatcherIO) {
-            val service = GravatarSdkDI.getGravatarV3Service(okHttpClient, oauthToken)
+        runThrowingExceptionRequest {
+            withContext(GravatarSdkDI.dispatcherIO) {
+                val service = GravatarSdkDI.getGravatarV3Service(okHttpClient, oauthToken)
 
-            val response = service.setEmailAvatar(avatarId, SetEmailAvatarRequest { emailHash = hash })
+                val response = service.setEmailAvatar(avatarId, SetEmailAvatarRequest { emailHash = hash })
 
-            if (response.isSuccessful) {
-                Unit
-            } else {
-                // Log the response body for debugging purposes if the response is not successful
-                Logger.w(
-                    LOG_TAG,
-                    "Network call unsuccessful trying to set Gravatar avatar: $response.body",
-                )
-                throw HttpException(response)
+                if (response.isSuccessful) {
+                    Unit
+                } else {
+                    // Log the response body for debugging purposes if the response is not successful
+                    Logger.w(
+                        LOG_TAG,
+                        "Network call unsuccessful trying to set Gravatar avatar: $response.body",
+                    )
+                    throw HttpException(response)
+                }
             }
         }
 
diff --git a/gravatar/src/main/java/com/gravatar/services/ErrorType.kt b/gravatar/src/main/java/com/gravatar/services/ErrorType.kt
@@ -20,15 +20,15 @@ internal fun HttpException.errorTypeFromHttpCode(moshi: Moshi): ErrorType = when
     }
 
     in HttpResponseCode.SERVER_ERRORS -> ErrorType.Server
-    else -> ErrorType.Unknown
+    else -> ErrorType.Unknown("HTTP Code $code - ErrorBody $rawErrorBody")
 }
 
 internal fun Throwable.errorType(moshi: Moshi): ErrorType {
     return when (this) {
         is SocketTimeoutException -> ErrorType.Timeout
         is UnknownHostException -> ErrorType.Network
         is HttpException -> this.errorTypeFromHttpCode(moshi)
-        else -> ErrorType.Unknown
+        else -> ErrorType.Unknown(message)
     }
 }
 
@@ -54,8 +54,18 @@ public sealed class ErrorType {
     /** User not authorized to perform given action **/
     public data object Unauthorized : ErrorType()
 
-    /** An unknown error occurred */
-    public data object Unknown : ErrorType()
+    /**
+     * An unknown error occurred
+     *
+     * @property errorMsg The error message, if available.
+     */
+    public class Unknown(public val errorMsg: String? = null) : ErrorType() {
+        override fun toString(): String = "Unknown(errorMsg=$errorMsg)"
+
+        override fun equals(other: Any?): Boolean = other is Unknown && errorMsg == other.errorMsg
+
+        override fun hashCode(): Int = Objects.hash(errorMsg)
+    }
 
     /**
      * An error occurred while processing the request.
diff --git a/gravatar/src/main/java/com/gravatar/services/GravatarException.kt b/gravatar/src/main/java/com/gravatar/services/GravatarException.kt
@@ -0,0 +1,12 @@
+package com.gravatar.services
+
+/**
+ * Exception that will be thrown when an error occurs during Gravatar operations.
+ *
+ * @property errorType The type of error that occurred.
+ * @property originalException The original exception that caused this exception.
+ */
+public class GravatarException internal constructor(
+    public val errorType: ErrorType,
+    public val originalException: Exception? = null,
+) : RuntimeException()
diff --git a/gravatar/src/main/java/com/gravatar/services/HttpException.kt b/gravatar/src/main/java/com/gravatar/services/HttpException.kt
@@ -5,11 +5,10 @@ import retrofit2.Response
 /**
  * Exception thrown when an HTTP error occurs in a non-cathing method.
  *
+ * [rawErrorBody] The raw error body of the response.
  * [code] The HTTP status code.
- * [message] The HTTP status message.
  */
-public class HttpException internal constructor(response: Response<*>) : RuntimeException() {
-    internal val rawErrorBody: String? = response.errorBody()?.string()
-    public val code: Int = response.code()
-    public override val message: String = "HTTP ${response.code()} $rawErrorBody"
+internal class HttpException internal constructor(response: Response<*>) : RuntimeException() {
+    val rawErrorBody: String? = response.errorBody()?.string()
+    val code: Int = response.code()
 }
diff --git a/gravatar/src/main/java/com/gravatar/services/ProfileService.kt b/gravatar/src/main/java/com/gravatar/services/ProfileService.kt
@@ -1,6 +1,7 @@
 package com.gravatar.services
 
 import com.gravatar.HttpResponseCode
+import com.gravatar.di.container.GravatarSdkContainer
 import com.gravatar.logger.Logger
 import com.gravatar.restapi.models.Profile
 import com.gravatar.types.Email
@@ -29,23 +30,29 @@ public class ProfileService(private val okHttpClient: OkHttpClient? = null) {
      * @param hashOrUsername The hash or username to fetch the profile for
      * @return The fetched profile or null if profile not found
      */
-    public suspend fun retrieve(hashOrUsername: String): Profile? = withContext(GravatarSdkDI.dispatcherIO) {
-        val response = service.getProfileById(hashOrUsername)
-        if (response.isSuccessful) {
-            response.body() ?: error("Response body is null")
-        } else {
-            // Log the response body for debugging purposes if the response is not successful
-            Logger.w(
-                LOG_TAG,
-                "Network call unsuccessful trying to get Gravatar profile: ${response.code()}",
-            )
-            if (response.code() == HttpResponseCode.HTTP_NOT_FOUND) {
-                return@withContext null
-            } else {
-                throw HttpException(response)
+    public suspend fun retrieve(hashOrUsername: String): Profile? =
+        @Suppress("TooGenericExceptionCaught")
+        try {
+            withContext(GravatarSdkDI.dispatcherIO) {
+                val response = service.getProfileById(hashOrUsername)
+                if (response.isSuccessful) {
+                    response.body() ?: error("Response body is null")
+                } else {
+                    // Log the response body for debugging purposes if the response is not successful
+                    Logger.w(
+                        LOG_TAG,
+                        "Network call unsuccessful trying to get Gravatar profile: ${response.code()}",
+                    )
+                    if (response.code() == HttpResponseCode.HTTP_NOT_FOUND) {
+                        return@withContext null
+                    } else {
+                        throw HttpException(response)
+                    }
+                }
             }
+        } catch (ex: Exception) {
+            throw GravatarException(ex.errorType(GravatarSdkContainer.instance.moshi), ex)
         }
-    }
 
     /**
      * Fetches a Gravatar profile for the given hash or username.
@@ -138,7 +145,7 @@ public class ProfileService(private val okHttpClient: OkHttpClient? = null) {
      * @param email The email address to check
      * @return True if the email is associated with the account, false otherwise
      */
-    public suspend fun checkAssociatedEmail(oauthToken: String, email: Email): Boolean =
+    public suspend fun checkAssociatedEmail(oauthToken: String, email: Email): Boolean = runThrowingExceptionRequest {
         withContext(GravatarSdkDI.dispatcherIO) {
             val service = GravatarSdkDI.getGravatarV3Service(okHttpClient, oauthToken)
 
@@ -154,6 +161,7 @@ public class ProfileService(private val okHttpClient: OkHttpClient? = null) {
                 throw HttpException(response)
             }
         }
+    }
 
     /**
      * Checks if the given email address is associated with the already authorized Gravatar account.
diff --git a/gravatar/src/main/java/com/gravatar/services/ServiceUtils.kt b/gravatar/src/main/java/com/gravatar/services/ServiceUtils.kt
@@ -14,7 +14,21 @@ internal inline fun <T> runCatchingRequest(block: () -> T?): GravatarResult<T, E
         }
     } catch (cancellationException: CancellationException) {
         throw cancellationException
+    } catch (gravatarException: GravatarException) {
+        GravatarResult.Failure(gravatarException.errorType)
     } catch (ex: Exception) {
         GravatarResult.Failure(ex.errorType(GravatarSdkContainer.instance.moshi))
     }
 }
+
+@Suppress("ThrowsCount")
+internal inline fun <T> runThrowingExceptionRequest(block: () -> T?): T {
+    @Suppress("TooGenericExceptionCaught")
+    try {
+        return block() ?: throw GravatarException(ErrorType.NotFound)
+    } catch (cancellationException: CancellationException) {
+        throw cancellationException
+    } catch (ex: Exception) {
+        throw GravatarException(ex.errorType(GravatarSdkContainer.instance.moshi), ex)
+    }
+}
diff --git a/gravatar/src/test/java/com/gravatar/services/AvatarServiceTest.kt b/gravatar/src/test/java/com/gravatar/services/AvatarServiceTest.kt
@@ -62,16 +62,17 @@ class AvatarServiceTest {
         }
     }
 
-    @Test(expected = HttpException::class)
-    fun `given an avatar upload when an error occurs then an exception is thrown`() = runTest {
-        val mockResponse = mockk<Response<Avatar>>(relaxed = true) {
-            every { isSuccessful } returns false
-            every { code() } returns 500
-        }
-        coEvery { containerRule.gravatarApiMock.uploadAvatar(any()) } returns mockResponse
+    @Test
+    fun `given an avatar upload when an error occurs then an exception is thrown`() =
+        runTestExpectingGravatarException(ErrorType.Server, HttpException::class.java) {
+            val mockResponse = mockk<Response<Avatar>>(relaxed = true) {
+                every { isSuccessful } returns false
+                every { code() } returns 500
+            }
+            coEvery { containerRule.gravatarApiMock.uploadAvatar(any()) } returns mockResponse
 
-        avatarService.upload(File("avatarFile"), oauthToken)
-    }
+            avatarService.upload(File("avatarFile"), oauthToken)
+        }
 
     @Test
     fun `given a file when uploadCatching avatar then Gravatar service is invoked`() = runTest {
@@ -136,9 +137,9 @@ class AvatarServiceTest {
         avatarService.setAvatar(hash, avatarId, oauthToken)
     }
 
-    @Test(expected = HttpException::class)
+    @Test
     fun `given a hash and an avatarId when setting an avatar and an error occurs then an exception is thrown`() =
-        runTest {
+        runTestExpectingGravatarException(ErrorType.Server, HttpException::class.java) {
             val hash = "hash"
             val avatarId = "avatarId"
             val mockResponse = mockk<Response<Unit>>(relaxed = true) {
diff --git a/gravatar/src/test/java/com/gravatar/services/ErrorTypeTest.kt b/gravatar/src/test/java/com/gravatar/services/ErrorTypeTest.kt
diff --git a/gravatar/src/test/java/com/gravatar/services/ProfileServiceTests.kt b/gravatar/src/test/java/com/gravatar/services/ProfileServiceTests.kt
diff --git a/gravatar/src/test/java/com/gravatar/services/ServiceTestUtils.kt b/gravatar/src/test/java/com/gravatar/services/ServiceTestUtils.kt

Original file line number	Diff line number	Diff line change
`@@ -264,7 +264,7 @@ internal class AvatarPickerViewModel(`
`264`	`264`	`ErrorType.NotFound,`
`265`	`265`	`ErrorType.RateLimitExceeded,`
`266`	`266`	`ErrorType.Timeout,`
`267`		`- ErrorType.Unknown,`
	`267`	`+ is ErrorType.Unknown,`
`268`	`268`	`is ErrorType.InvalidRequest,`
`269`	`269`	`-> SectionError.Unknown`
`270`	`270`	`}`
Original file line number	Diff line number	Diff line change
`@@ -97,12 +97,12 @@ class AvatarRepositoryTest {`
`97`	`97`	`coEvery { tokenStorage.getToken(any()) } returns "token"`
`98`	`98`	`coEvery {`
`99`	`99`	`avatarService.setAvatarCatching(any(), any(), any())`
`100`		`- } returns GravatarResult.Failure(ErrorType.Unknown)`
	`100`	`+ } returns GravatarResult.Failure(ErrorType.Unknown())`
`101`	`101`
`102`	`102`	`val result = avatarRepository.selectAvatar(email, "avatarId")`
`103`	`103`
`104`	`104`	`assertEquals(`
`105`		`- GravatarResult.Failure<String, QuickEditorError>(QuickEditorError.Request(ErrorType.Unknown)),`
	`105`	`+ GravatarResult.Failure<String, QuickEditorError>(QuickEditorError.Request(ErrorType.Unknown())),`
`106`	`106`	`result,`
`107`	`107`	`)`
`108`	`108`	`}`
Original file line number	Diff line number	Diff line change
`@@ -62,7 +62,7 @@ class AvatarPickerViewModelTest {`
`62`	`62`
`63`	`63`	`@Before`
`64`	`64`	`fun setup() {`
`65`		`- coEvery { profileService.retrieveCatching(email) } returns GravatarResult.Failure(ErrorType.Unknown)`
	`65`	`+ coEvery { profileService.retrieveCatching(email) } returns GravatarResult.Failure(ErrorType.Unknown())`
`66`	`66`	`coEvery { avatarRepository.getAvatars(email) } returns GravatarResult.Success(emailAvatars)`
`67`	`67`	`}`
`68`	`68`
Original file line number	Diff line number	Diff line change
`@@ -84,6 +84,7 @@ dependencies {`
`84`	`84`	`testImplementation(libs.mockk.android)`
`85`	`85`	`testImplementation(libs.mockk.agent)`
`86`	`86`	`testImplementation(libs.kotlinx.coroutines.test)`
	`87`	`+ testImplementation(kotlin("test"))`
`87`	`88`	`}`
`88`	`89`
`89`	`90`	`project.afterEvaluate {`