Merge pull request #392 from Automattic/hamorillo/377-gravatar-exception

hamorillo · web-flow · commit 10e422f62701 · 2024-10-16T17:24:01.000+02:00
Throwing GravatarException on noncatching service methods
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
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/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
@@ -32,7 +32,7 @@ class ErrorTypeTest {
         HTTP_CLIENT_TIMEOUT to ErrorType.Timeout,
         HTTP_NOT_FOUND to ErrorType.NotFound,
         HTTP_TOO_MANY_REQUESTS to ErrorType.RateLimitExceeded,
-        600 to ErrorType.Unknown,
+        600 to ErrorType.Unknown("HTTP Code 600 - ErrorBody $errorBody"),
         INVALID_REQUEST to ErrorType.InvalidRequest(
             error = Error {
                 code = "uncropped_image"
@@ -67,7 +67,7 @@ class ErrorTypeTest {
         val exceptionToErrorTypeRelation = mutableListOf(
             SocketTimeoutException() to ErrorType.Timeout,
             UnknownHostException() to ErrorType.Network,
-            Exception() to ErrorType.Unknown,
+            Exception() to ErrorType.Unknown(),
         ).apply {
             httpCodeToErrorTypeRelation.forEach { (code, errorType) ->
                 val exception = mockk<HttpException>(relaxed = true) {
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 {`