Throwing GravatarException on noncatching service methods #392
Conversation
📲 You can test the changes from this Pull Request in Gravatar Demo by scanning the QR code below to install the corresponding build.
|
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.
hamorillo
force-pushed
the
hamorillo/377-gravatar-exception
branch
from
27acb45
to
add42d2
Compare
| 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() { |
There was a problem hiding this comment.
We'll use this exception only internally to catch HTTP errors.
There was a problem hiding this comment.
We are throwing it and passing as originalException, no? Would it make sense to keep it public for that?
There was a problem hiding this comment.
Yeah, it makes sense. I'll revert the changes in this class.
There was a problem hiding this comment.
I think you can keep the constructor private - it's for internal use indeed.
There was a problem hiding this comment.
I kept it as internal. 👍
| * | ||
| * @property errorMsg The error message, if available. | ||
| */ | ||
| public class Unknown(public val errorMsg: String? = null) : ErrorType() { |
There was a problem hiding this comment.
I think it would be nice to provide some information when it is available.
| @@ -84,6 +84,7 @@ dependencies { | |||
| testImplementation(libs.mockk.android) | |||
| testImplementation(libs.mockk.agent) | |||
| testImplementation(libs.kotlinx.coroutines.test) | |||
| testImplementation(kotlin("test")) | |||
There was a problem hiding this comment.
Testing the inside of the expected exception with JUnit 4 is not very straightforward, so adding this dependency here makes our life easier.
| 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() { |
There was a problem hiding this comment.
We are throwing it and passing as originalException, no? Would it make sense to keep it public for that?
| 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) | ||
| } | ||
| } |
There was a problem hiding this comment.
Is there a reason why we can't use runThrowingExceptionRequest here?
There was a problem hiding this comment.
In this case, the returned value when 404 is null instead of the exception, so we can't use runThrowingExceptionRequest, which requires not null.
Keep HttpException public, as it is returned as the original exception when an HTTP error occurs.
Closes #377
Description
We want to wrap all exceptions that can occur inside the Gravatar service methods in our own
GravatarExceptionto provide theErrorTypeas we do in the catching variants of those methods.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, basically
GravatarException.Testing Steps
nonCatchingmethod there)