Automattic / Gravatar-SDK-Android

Gravatar Android library
https://gravatar.com
Mozilla Public License 2.0
38 stars 11 forks source link

API: `uploadImage` (and future private endpoints) onError callback should encapsulate the error message returned from the backend #66

Open maxme opened 7 months ago

maxme commented 7 months ago

While testing the upload component, I realized that we don't read the error message from a response when we get a 4xx error here. When using an invalid token, there is no way to know if the authentication went through or if there is another error (we get and UNKNOWN error).

We should parse the response.errorBody field and at least forward the error message to the API user. I think we don't get any error code or error identifier from the backend (we should add one). I wonder if we can declare the error response in the OpenAPI spec - #65.

eyedol commented 6 months ago

@maxme I'm interested in picking this up of course if it's ready for grabs. I just need a little info on how you want this implemented.

With support for passing the error message along with the error code to client consumers, I foresee it requiring re-modeling the enum ErrorType class. Something along this line:

public sealed interface ErrorType {
    public val code: Int
    public val message: String?

    /** Bad user request occurred */
    public data class BadRequest(
        override val code: Int,
        override val message: String
    ): ErrorType

    /** An unknown error occurred */
    public data class Unknown(
        override val code: Int = -1,
        override val message: String? = null
    ): ErrorType

    ...
}

What did you have in mind?

maxme commented 6 months ago

Hey @eyedol, thank you for looking into that. We'll need to update the backend first to add a new error code field. Currently there is no error code and we can't identify the error by its message. Ideally we will model the error codes with an enum or a sealed class instead of an Int, but we need to define and list all the possible errors first.

eyedol commented 6 months ago

I understand. It needs to be unblocked by backend work then. Thanks for the response.