Combine error callbacks

[msya]

Summary

The errors are now propagated to the user with a single callback method.

authenticator.clientCredentials(object : VimeoCallback<BasicAccessToken> {

      override fun onSuccess(response: VimeoResponse.Success<BasicAccessToken>) {

      }

      override fun onError(error: VimeoResponse.Error) {

      }
})

The sealed class for error enumerates all type of errors that can happen:

sealed class VimeoResponse<out T> {

    data class Success<out T>(val data: T) : VimeoResponse<T>()

    sealed class Error(val message: String): VimeoResponse<Nothing>() {

        data class Api(val reason: ApiError): Error("API error")

        data class Exception(val throwable: Throwable): Error("Exception thrown")

        data class Parsing(val rawResponse: String): Error("Parsing error")

    }

}

• ApiResponse was renamed to VimeoResponse.
• VimeoCallbackAdapter was updated to use the Error sealed class above.

[anthonycr]

Perhaps the Error message could have more information like

"API error: ${reason.errorCode ?: "unknown"}"

[anthonycr]

It seems like this error replaces the generic error - what is the logic behind any error that doesn't have a reason being a parsing error? What if a request is made for a missing resource and we get a 404? Will that be surfaced as a Parsing error, and, if so, is that really the "right" error?

[msya]

A 404 error gives the following response from the API:

{
  "error": "The requested page could not be found"
}

If this error is successfully parsed, it will be surfaced to the client as an API error as follow:

ApiError(
    developerMessage=null, 
     errorMessage=The requested page could not be found, 
     errorCode=null, 
     invalidParameters=null, link=null
)

Suppose this error occurs and the error convert fails to convert it into a ApiError object, the client will be informed that a parsing error occurred. This will be the response to the client:

Parsing(
   rawResponse=Response{ protocol=http/1.1, code=404, message=Not Found, url=https://api.vimeo.com/oauth/authorize/clientttttt})

I could rename Parsing to Generic for a better name.

[anthonycr]

🔥

[anthonycr]

Good for both Java and Kotlin consumers!

[msya]

@anthonycr Back to you.

[anthonycr]

There are certain APIs that require knowledge about the HTTP status code (e.g. 404, 401, 500) of an error. Such APIs include:

• Adding a video to a channel, which performs a check to see if the video is in the channel and a 404 indicates that it is not in the channel.
• Editing video settings, where a 404 indicates that the video was deleted.
• Adding a comment to a video, where a 404 indicates that the video was deleted.
• Playing a DRM protected video, where a 403 indicates that the device limit has been exceeded.

If we don't expose at least the error code here, it will be more difficult for consumers to use certain APIs. Of course in most cases, the specifics of the error are not important to the UI, but our consuming code often makes certain decisions based on that error code, so I would suggest adding the code in addition to the raw response.

[msya]

@anthonycr Back to you.

[anthonycr]

LGTM